Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

React 18 - @shopify/react-testing #2248

Merged
merged 13 commits into from Jun 8, 2022
Merged

React 18 - @shopify/react-testing #2248

merged 13 commits into from Jun 8, 2022

Conversation

ryanwilsonperkin
Copy link
Member

@ryanwilsonperkin ryanwilsonperkin commented Apr 27, 2022 •
edited

Description

Base: #2241
Fixes https://github.com/Shopify/web/issues/63209

Fixes #2252

We're upgrading @shopify/react-testing to support React 18 while maintaining backwards compatibility with React 17. This takes a couple of steps:

  1. We need to introduce a test matrix and namespaced npm packages so that we can test against both 17 & 18 and ensure we maintain backwards compatibility
  2. We need to update to the new createRoot API, and provide a shim for it for React 17
  3. We need to use the flushSync API within our custom act wrapper in order to help ease the transition to React 18 for shopify/web.

On flushSync

Our Web project has a lot of code that relies on some very specific ordering of promise resolution and state updates and thousands of tests break as a result of the new batching of state updates in React 18. Wrapping these act()s in flushSync helps make that a much easier update.

In the future, what we'd like to do is to transition web over to better testing and component structuring practices which removes the need for flushSync. To do that, we'll probably make an option to opt-out of flushSync on every act and run that as a test suite for Web that we know will be failing and then incrementally fix the tests that are failing because of it.

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)
  • react-testing Minor: New feature (non-breaking change which adds functionality)
  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above (Documentation fix and Test update does not need a changelog as we do not publish new version)

@ryanwilsonperkin ryanwilsonperkin requested a review from a team as a code owner April 27, 2022 14:04
@ryanwilsonperkin ryanwilsonperkin requested review from egjiri and removed request for a team April 27, 2022 14:04
@ryanwilsonperkin ryanwilsonperkin mentioned this pull request Apr 27, 2022
Closed
@ryanwilsonperkin ryanwilsonperkin removed the request for review from egjiri April 27, 2022 14:10
@ryanwilsonperkin ryanwilsonperkin marked this pull request as draft April 27, 2022 14:10
@shopify-shipit shopify-shipit bot temporarily deployed to react-18-react-testing May 2, 2022 20:20 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to beta-js May 3, 2022 19:49 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to beta-js May 3, 2022 20:21 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to beta-js May 18, 2022 18:30 Inactive
@ryanwilsonperkin ryanwilsonperkin force-pushed the react-18-react-testing branch 2 times, most recently from 642b054 to 1707e20 Compare June 1, 2022 15:42
@shopify-shipit shopify-shipit bot temporarily deployed to beta-js June 1, 2022 16:44 Inactive
@ryanwilsonperkin ryanwilsonperkin force-pushed the react-18-react-testing branch 2 times, most recently from e66a9b4 to a1c8a6a Compare June 3, 2022 15:49
@shopify-shipit shopify-shipit bot temporarily deployed to beta-js June 3, 2022 18:08 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to beta-js June 3, 2022 19:48 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to beta-js June 6, 2022 19:29 Inactive
@ryanwilsonperkin ryanwilsonperkin force-pushed the react-18 branch 3 times, most recently from 187f52c to cdbb3aa Compare June 7, 2022 18:17
@ryanwilsonperkin
Enable IS_REACT_ACT_ENVIRONMENT
edd0954
@ryanwilsonperkin
Configure loom to pickup test environment
9ff515f 
Loom typically looks for setupFiles under tests/setup/environment.* but
we have ours under the additional sub-directory of src/ because of the
way that react-testing lays out its own tests.
@ryanwilsonperkin
Update react-testing to use new createRoot API
e210732 
The previous ReactDOM.render method is deprecated and instead tests
should be making use of createRoot to generate a "Root" element and then
calling render/unmount on that.

This causes some confusion here because we already have a concept of a
"Root" in react-testing, and so we use the name "reactRoot" to
differentiate here.
@ryanwilsonperkin
Improvements to the way we await the act scope
f71f134 
Ensure that we're awaiting the promise within the scope of the act block
and use the flushSync API to mimic the behaviour of React 17 to
synchronously flush state to the render rather than trying to batch it.
Without the use of flushSync, we were seeing issues in shopify/web tests
where a previously tested component that left a promise in the act queue
could cause subsequent mounts in other tests to fail.
@ryanwilsonperkin
Only return result to act if it's a promise
ea6d57b 
In React < 18 returning a value that isn't a promise to act will result
in a console warning, we need to match the previous behaviour of
react-testing for applications that are still using React 17 to avoid
introducing a warning.
@ryanwilsonperkin
Install React & ReactDOM versions both 17 & 18
b1b11c9 
Both versions are already functionally installed because react-server
is still using React 17. This installs them both under namespaced
packages as react17 and react18. We'll use these values to allow testing
under particular react versions using the REACT_VERSION env var.
@ryanwilsonperkin
Run tests against either React 17 or 18 with REACT_VERSION env var
08328d1
@ryanwilsonperkin
Backwards compatible support for React 17
d979a51 
React 17 doesn't have a react-dom/client directory, nor does it have a
createRoot or Root API. We provide backwards compatibility for this in
the compat module so that our root.tsx code can still use "createRoot"
just in React < 18 it'll be our own version of createRoot which wraps
ReactDOM.render and ReactDOM.unmountComponentAtNode
@ryanwilsonperkin
Run GitHub actions test suite against React 17&18
17f3975
@ryanwilsonperkin
Add name to test runner
7de68f3 
Makes it clearer what that 14/16/17/18 numbers correspond to
@ryanwilsonperkin
Copy link
Member Author

Tested here against web running React 17 using a beta release of this PR in order to show that this version could be installed into web without error prior to upgrading to React 18: https://buildkite.com/shopify/web-ci-builder/builds/487393

@ryanwilsonperkin ryanwilsonperkin marked this pull request as ready for review June 8, 2022 16:56
melodyhabbouche
melodyhabbouche approved these changes Jun 8, 2022
View reviewed changes
Copy link
Contributor

@melodyhabbouche melodyhabbouche left a comment

Choose a reason for hiding this comment

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

PR looks good to me! 💯 🚀 small nit comment about using block comment instead

@ryanwilsonperkin ryanwilsonperkin merged commit 27bc14b into react-18 Jun 8, 2022
@ryanwilsonperkin ryanwilsonperkin deleted the react-18-react-testing branch June 8, 2022 18:27
@samjrdn samjrdn mentioned this pull request May 5, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@melodyhabbouche melodyhabbouche melodyhabbouche approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ryanwilsonperkin @melodyhabbouche