Skip to content

Conversation

jcfilben
Copy link
Collaborator

@jcfilben jcfilben commented Apr 13, 2022

What does this PR do?

Upgrades to React v18

  • Anyone consuming Grommet should be able to use React v16, v17, or v18. The breaking changes in react 18 are within our testing files.

Important changes in React 18:
ReactDOM.render is no longer supported
You are still able to use ReactDOM.render but you will get a warning that it is deprecated. createRoot should be used instead of render. Render was being used in the following places in grommet:

  • In the unit tests we are using render from @testing-library/react. As of v13.0.0 of @testing-library/react when you use render from testing-library it is actually using createRoot under the hood.
  • Storybook is loading stories with render. There is a pre-release branch of storybook that supports createRoot but it looks like they still need to iron out some issues so we are sticking with the official release version of storybook for now. This means that when you run storybook everything should work as before but there will be warnings in the console on each story. We will need to make a note of this in the release notes if it is not fixed before then.
  • RoutedAnchor and RoutedButton use render. This means if these components are used in a React 18 application they will generate a warning but still function. Since these components are deprecated I felt this was fine.

Updates to TypeScript definitions
The most notable change is that the children prop now needs to be explicitly defined. A list of the typescript changes can be seen here: DefinitelyTyped/DefinitelyTyped#56210. I have updated the relevant type definitions to include children. It is possible that I may have missed updating some of the types so something we will want to keep an eye on as we move closer to the release this month.

Other Notes:

  • I removed the userAgent test from Grommet-test.js. With the testing library update this test's document.body's client width was always returning 0 so the ResponsiveContext would always receive a size of small. I ensured that the userAgent code in Grommet was working as expected and it was. I played around with the test but in the end felt that there wasn't a good way to write the test that was effectively testing the userAgent. I think if we want to have this kind of test it is better suited to an end-to-end type of testing environment instead of testing with jsdom (which acknowledges that it shouldn't be used for this kind of testing).

Helpful links:
How to upgrade to react 18 - https://reactjs.org/blog/2022/03/08/react-18-upgrade-guide.html
React's changelog - https://github.com/facebook/react/blob/main/CHANGELOG.md

Where should the reviewer start?

What testing has been done on this PR?

yarn test
yarn storybook
yarn build

How should this be manually tested?

Do Jest tests follow these best practices?

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • userEvent is used in place of fireEvent.
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

Closes #5947
Closes #6062

Screenshots (if appropriate)

Do the grommet docs need to be updated?

Yes

Should this PR be mentioned in the release notes?

Yes

Is this change backwards compatible or is it a breaking change?

Backwards compatible

Copy link
Contributor

@amandadupell amandadupell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes make sense based on looking into react 18 upgrades and explanations, tests look good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Breaking change from @types/react@18 React 18
4 participants