Skip to content
This repository was archived by the owner on Jan 10, 2025. It is now read-only.

✨ React testing supports react 17#1958

Merged
marutypes merged 2 commits intomainfrom
react-testing-support-react-17
Jun 25, 2021
Merged

✨ React testing supports react 17#1958
marutypes merged 2 commits intomainfrom
react-testing-support-react-17

Conversation

@marutypes
Copy link
Contributor

@marutypes marutypes commented Jun 23, 2021

closes #1955

Description

This PR adds support to react-17 to @shopify/react-testing

Full breakdown

  • upgrades react-reconciler to latest
  • fixes a breakage with @shopify/react-testing using a renamed internal property by making it work for both React 16 and 17
  • adds tests to ensure we don't break 17.x consumers
  • widens react-testing's stated version support
  • updates docs

Tophatting

The tests should be sufficient, but if you want you can yalc this version of react-testing into another app (eg. Shrink-Ray) and run the test suite there to verify it works with React 16. Next upgrade that repo to React 17 and run the tests again. Things should work swimmingly there too.

You can also use locally upgrade to react-17 and then run yarn test react-testing and ensure that those tests work (though others in the repo will break)

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)

@marutypes marutypes force-pushed the react-testing-support-react-17 branch 3 times, most recently from 4a0fe9d to 78e1065 Compare June 23, 2021 21:26
@marutypes
Copy link
Contributor Author

tests seem to be failing on node 14.x due to document.createEvent not being defined in the test env (and react-dom trying to use it)... oh noes

@marutypes marutypes force-pushed the react-testing-support-react-17 branch from 1aae679 to bb7cfbe Compare June 24, 2021 19:32
@marutypes
Copy link
Contributor Author

update I pulled the react 17 upgrade out of this PR as there were some non react-testing related issues I'd like to sort out separately. I'll be updating the description to be more reflective shortly.

@marutypes marutypes marked this pull request as ready for review June 24, 2021 20:07
@marutypes marutypes requested a review from a team June 24, 2021 20:07
@marutypes marutypes changed the title ⏫ Upgrade React, React testing supports react 17 ✨ React testing supports react 17 Jun 24, 2021
Copy link
Member

@BPScott BPScott left a comment

Choose a reason for hiding this comment

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

Remember to update the react/react-dom peer deps to "^16.8.0 || ^17.0.0".

What's our plan for updating the version of react installed in development? Keep it at 16.whatever for now?

@@ -0,0 +1,11 @@
import {ReactInstance, Fiber} from './types';

export function getInternalFiber(instance: ReactInstance): Fiber {
Copy link
Member

Choose a reason for hiding this comment

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

Silly nitpick - perhaps run with the most recent name getInternals so our terminology matches the most recent react naming


### What versions of React does this support?

The React versions this library supports are spelled out via a [peer dependency in the package.json](https://github.com/Shopify/quilt/blob/main/packages/react-testing/package.json#L47-L47)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we need to update the react/react-dom peer deps to "^16.8.0 || ^17.0.0"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I must have reverted the range expansion when I pulled out the react upgrade, oops

Copy link
Collaborator

@vsumner vsumner left a comment

Choose a reason for hiding this comment

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

🎩 ✅

… to ensure backwards compatibility is maintained
@marutypes marutypes force-pushed the react-testing-support-react-17 branch from b40a3a3 to e8af60a Compare June 25, 2021 16:52
@marutypes
Copy link
Contributor Author

marutypes commented Jun 25, 2021

@BPScott

What's our plan for updating the version of react installed in development? Keep it at 16.whatever for now?

I originally had that rolled into this PR but it broke a couple other things, so my current plan is to fix those as individual PRs and then do one to upgrade.

Copy link
Member

@BPScott BPScott left a comment

Choose a reason for hiding this comment

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

Love how straightforward this was :D

@marutypes marutypes merged commit 30c8600 into main Jun 25, 2021
@marutypes marutypes deleted the react-testing-support-react-17 branch June 25, 2021 17:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for React 17.x to react-testing

3 participants