Skip to content

Conversation

@joshblack
Copy link
Member

@joshblack joshblack commented Dec 9, 2022

Update react dependencies to v18 in devDependencies. The project still supports both v17 and v18.

Notes

  • In order to get snapshots to work in both v17 and v18, the internal useId hook will always use the useSSRSafeId path in test en vironments
  • There doesn't seem to be a way to conditionally bring in react-dom/client in order to replace ReactDOM.render() usage, this may be something that we'll want to save for our next major release or find a way to replace ReactDOM.render() usage with something else

Changelog

New

Changed

  • Flip CI step for installing dependencies for v17 and v18
  • Update package.json to use React 18 deps as default
  • Update useSSRSafeId usage to an internal useId hook, under the hood it toggles between useId and useSSRSafeId depending on React environment
  • Update our eslint rule to catch usage of useSSRSafeId and replace it with the internal useId() hook

Removed

@joshblack joshblack added the skip changeset This change does not need a changelog label Dec 9, 2022
@changeset-bot
Copy link

changeset-bot bot commented Dec 9, 2022

🦋 Changeset detected

Latest commit: 3a4d393

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2022

size-limit report 📦

Path Size
dist/browser.esm.js 82.79 KB (+0.05% 🔺)
dist/browser.umd.js 83.47 KB (+0.06% 🔺)

@joshblack joshblack temporarily deployed to github-pages December 9, 2022 20:49 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2675 December 9, 2022 20:49 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2675 December 9, 2022 21:16 Inactive
@joshblack joshblack temporarily deployed to github-pages December 9, 2022 21:18 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2675 December 9, 2022 21:18 Inactive
@joshblack joshblack force-pushed the feat/update-deps-for-react-18 branch from a9080f4 to 6a319fc Compare December 12, 2022 22:28
@joshblack
Copy link
Member Author

cc @broccolinisoup was running into some weird storybook test errors with UnderlineNav and was curious if you had any thoughts on what I was doing wrong here 🤔

@joshblack joshblack force-pushed the feat/enable-react-18-in-dev branch from 88cd325 to b84bced Compare December 12, 2022 23:07
@joshblack joshblack temporarily deployed to github-pages December 12, 2022 23:14 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2675 December 12, 2022 23:15 Inactive
@broccolinisoup
Copy link
Member

broccolinisoup commented Dec 13, 2022

cc @broccolinisoup was running into some weird storybook test errors with UnderlineNav and was curious if you had any thoughts on what I was doing wrong here 🤔

I don't think there is anything you are doing wrong! Seems like storybook play function has some issues with React 18 already and I have a feeling it is something to do with it. There are 2 issues I see on UnderlineNav interaction tests on this branch

  1. Async userEvent functions seems to cause some issues. For example on the Keyboard Interaction story fires a userEvent.click() on the "More" menu and then userEvent.tab() and it is supposed to tab through on the overflow menu but it fires the userEvent.tab() before opening the overflow menu so it fails. If I add extra await delay(1000) function after firing the userEvent on the More menu it continues as expected.
  2. userEvent.click and userEvent.keyboard('{Enter}') don't seem to respect event.preventDefault 😢 I cannot really understand why this is happening and it opens the story on a new canvas and kind of gets messed up after this. Let me know if you have any thoughts on why this might be happening, I'll be looking into trying some other solution to solve this.

@joshblack
Copy link
Member Author

@broccolinisoup thanks so much for taking a look! 🙌

Just pushed a change that sets target="_self" on UnderlineNav.Item which seems to do the trick 🤞 Let me know what you think!

@joshblack joshblack marked this pull request as ready for review December 13, 2022 17:56
@joshblack joshblack requested review from a team and josepmartins December 13, 2022 17:56
@joshblack joshblack temporarily deployed to github-pages December 13, 2022 17:56 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2675 December 13, 2022 17:56 Inactive
@joshblack joshblack mentioned this pull request Dec 13, 2022
22 tasks
@joshblack joshblack temporarily deployed to github-pages December 13, 2022 21:48 — with GitHub Actions Inactive
@broccolinisoup
Copy link
Member

@broccolinisoup thanks so much for taking a look! 🙌

Just pushed a change that sets target="_self" on UnderlineNav.Item which seems to do the trick 🤞 Let me know what you think!

Sounds good to me! Thanks for adding up the delay functions as well!

@joshblack joshblack merged commit 68ce453 into feat/update-deps-for-react-18 Dec 15, 2022
@joshblack joshblack deleted the feat/enable-react-18-in-dev branch December 15, 2022 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip changeset This change does not need a changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants