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.

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

Already on GitHub? Sign in to your account

chore: upgrade jest and jsdom #10453

Merged
merged 2 commits into from May 25, 2023
Merged

Conversation

jenseng
Copy link
Contributor

@jenseng jenseng commented May 5, 2023

This lets us remove a polyfill and some other hacks, and brings along some additional jsdom features which will facilitate testing fixes to submitter serialization (see #9865).

Note:

  • jsdom is making it harder to stub things, so we inject a jest-friendly window into the router. I'm not a huge fan of DI when used solely for the sake of testing, but this seems less terrible than other approaches (e.g. patch-package-ing jsdom, or monkey-patching Object.defineProperty) 馃檭
  • We use a yarn resolution to get the latest jsdom, despite the latest jest pinning it at ^20.0.0. Per the maintainers, this is fine as jest doesn't need any code changes to work with new jsdom, and there are no concrete plans yet to ship a new version.

@changeset-bot
Copy link

changeset-bot bot commented May 5, 2023

馃 Changeset detected

Latest commit: 0f16b2d

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

This PR includes changesets to release 5 packages
Name Type
@remix-run/router Patch
react-router Patch
react-router-dom Patch
react-router-dom-v5-compat Patch
react-router-native Patch

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

@jenseng jenseng force-pushed the upgrade-jest-and-jsdom branch 3 times, most recently from 7a768ac to 730dbb2 Compare May 5, 2023 19:39
This lets us remove a polyfill and some other hacks, and brings along some
additional jsdom features which will facilitate testing fixes to submitter
serialization.

Note:
 * jsdom is making it harder to stub things, so we inject a jest-friendly
   window into the router. I'm not a huge fan of DI when used solely for the
   sake of testing, but this seems less terrible than other approaches (e.g.
   (patch-package-ing jsdom, or monkey-patching Object.defineProperty) 馃檭
 * We use a yarn resolution to get the latest jsdom, despite the latest
   jest pinning it at ^20.0.0. Per the maintainers, this is fine as jest
   doesn't need any code changes to work with new jsdom, and there are no
   concrete plans yet to ship a major version.
@jenseng jenseng changed the base branch from main to dev May 6, 2023 06:45
@jenseng
Copy link
Contributor Author

jenseng commented May 25, 2023

馃憢 @brophdawg11 :)

@brophdawg11
Copy link
Contributor

Thanks for the ping @jenseng - I kept meaning to come back to this and then, well, didn't 馃槄

Let me know if the minor updates look ok to you and I'll get this merged. Agree on DI being better than the alternatives, and it also follows the pattern we use in react-router-dom as well.

@jenseng
Copy link
Contributor Author

jenseng commented May 25, 2023

Awesome, thanks @brophdawg11!

@brophdawg11 brophdawg11 merged commit e2789c1 into remix-run:dev May 25, 2023
2 checks passed
@jenseng jenseng deleted the upgrade-jest-and-jsdom branch May 25, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants