-
Notifications
You must be signed in to change notification settings - Fork 9
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade Jest to v28 #169
Upgrade Jest to v28 #169
Conversation
Pull Request Test Coverage Report for Build 2455245851
💛 - Coveralls |
@@ -0,0 +1,31 @@ | |||
// This code is from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to use moduleNameMapper instead like suggested here? uuidjs/uuid#616 (comment)
It might be a little easier to find and remove later on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ridiculous they haven't closed out uuidjs/uuid#616 for 4 months
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is possible to use moduleNameMapper
, but it seemed like that might have some consequences, discussed here: uuidjs/uuid#616 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gotcha, sgtm!
This PR upgrades Jest from v27 to v28. Most of the breaking changes don't affect us, except for two: - To use the JSDOM test environment, `jest-environment-jsdom` now has to be installed separately - Jest now has full support for package exports, which can cause problems where file imports are not resolved correctly. Specifically, this is a problem for `uuid`. There is a discussion and proposed solution in this [GitHub issue](microsoft/accessibility-insights-web#5421 (comment)), which I used in this PR. To summarize, the fix is to add a custom resolver that forces Jest to use the CommonJS+node version of `uuid`, but leaves all other resolutions the same J=SLAP-2123 TEST=auto See that Jest tests pass.
Upgrade Jest from v27 to v29. We had [previously](#169) upgraded Jest to v28, but had to [downgrade](#275) to v27 because at the time, `@storybook/test-runner` only supported up to v27. In v0.7 of `@storybook/test-runner`, they changed Jest to be an internal dependency instead of a peer dependency, allowing us to use whatever version of Jest we want. There was only one breaking change in v29 that affected our code, besides those that already needed to be made when upgrading from v27 to v28 (see previous PR [description](#169 (comment))). This was the `jsdom` upgrade from v19 to v20 in `jest-environment-jsdom`, which requires the `typescript` version to be 4.5 or higher. With the new support added in v29 and `uuid` v9, we are now able to remove the manual resolving needed in `tests/__setup__/resolver.ts`. For more context on why this was initially needed with Jest v28, see #169. J=SLAP-2376 TEST=manual See that existing tests still run properly.
This PR upgrades Jest from v27 to v28. Most of the breaking changes don't affect us, except for two:
jest-environment-jsdom
now has to be installed separatelyuuid
. There is a discussion and proposed solution in this GitHub issue, which I used in this PR. To summarize, the fix is to add a custom resolver that forces Jest to use the CommonJS+node version ofuuid
, but leaves all other resolutions the sameJ=SLAP-2123
TEST=auto
See that Jest tests pass.