Skip to content

Upgrade jest to v29 #21575

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

Closed
eps1lon wants to merge 23 commits into from
Closed

Upgrade jest to v29 #21575

Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
676218c
Install jest 27
eps1lon Nov 23, 2021
1c81367
Fix getCacheKey breakage
eps1lon May 27, 2021
07ab585
Fix default testRunner breakage
eps1lon May 27, 2021
dfcec60
Fix fake timers breakage
eps1lon May 27, 2021
86dac47
jest.resetModuleRegistry -> jest.resetModules
eps1lon May 27, 2021
1ee8e96
Fix pretty-format breakage
eps1lon May 27, 2021
125956e
Fix breakage regarding (set|clear)Immediate
eps1lon May 27, 2021
a390ca0
Update jest
eps1lon Nov 23, 2021
5505b1f
Pull in upstream fix of createCacheKeyFunction
eps1lon Nov 23, 2021
a4a0276
ensure react-dom sets the proper dispatcher in the global react
eps1lon Nov 23, 2021
a2a4f47
Resolve breaking changes due to changed export namespace of `jest-diff`
eps1lon Jan 15, 2022
bc7a094
Use actual Jest environment to mix environments
eps1lon Mar 8, 2022
211024e
Fix useId tests
eps1lon Mar 19, 2022
98118ee
Upgrade jest to 27.5.1
eps1lon Apr 4, 2022
229921e
BROKEN Upgrade to Jest 28
eps1lon Sep 10, 2022
e580909
Install Jest 29
eps1lon Sep 27, 2022
6ec6eeb
Greeeen!
eps1lon Sep 27, 2022
67f776f
Fix flow
eps1lon Sep 27, 2022
84286a8
Upgrade to Jest 29.1.1
eps1lon Sep 28, 2022
0a7b29c
Fix experimental tests
eps1lon Sep 28, 2022
d8f4c8a
Fix production tests
eps1lon Sep 28, 2022
6bf603b
We'll get there
eps1lon Oct 2, 2022
8d0a41d
Update float test with server assertions
eps1lon Oct 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
ensure react-dom sets the proper dispatcher in the global react
  • Loading branch information
@eps1lon
eps1lon committed Oct 2, 2022
commit a4a027630c2db02bb8d306754405f703c3edbdbb
8 changes: 4 additions & 4 deletions packages/react-dom/src/__tests__/ReactDOMEventPropagation-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

'use strict';

describe('ReactDOMEventListener', () => {
describe('ReactDOMEventPropagation', () => {
Copy link
Collaborator Author

@eps1lon eps1lon Nov 23, 2021

Choose a reason for hiding this comment

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

Drive-by change. The suite name now matches the file name.

let React;
let OuterReactDOM;
let InnerReactDOM;
Expand All @@ -16,12 +16,12 @@ describe('ReactDOMEventListener', () => {
beforeEach(() => {
window.TextEvent = function() {};
jest.resetModules();
React = require('react');
jest.isolateModules(() => {
OuterReactDOM = require('react-dom');
InnerReactDOM = require('react-dom');
});
jest.isolateModules(() => {
InnerReactDOM = require('react-dom');
React = require('react');
OuterReactDOM = require('react-dom');
Comment on lines -19 to +24
Copy link
Collaborator Author

@eps1lon eps1lon Nov 23, 2021

Choose a reason for hiding this comment

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

Fixes https://app.circleci.com/pipelines/github/facebook/react/21294/workflows/b23b2fbb-9d67-447c-b395-21591b60ebf1/jobs/411166.

Probably required due to jestjs/jest#10963. Though it's unclear why this test passed without this change using scripts/jest/config.source-www.js but failed using scripts/jest/config.build.js.

@SimenB Any idea if this is intended?

Copy link
Contributor

@SimenB SimenB Nov 24, 2021

Choose a reason for hiding this comment

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

Sounds odd that changing config impacts module loading. My only guess is

react/scripts/jest/config.build.js

Lines 38 to 46 in c1220eb

// Map packages to bundles
packages.forEach(name => {
// Root entry point
moduleNameMapper[`^${name}$`] = `<rootDir>/build/${NODE_MODULES_DIR}/${name}`;
// Named entry points
moduleNameMapper[
`^${name}\/([^\/]+)$`
] = `<rootDir>/build/${NODE_MODULES_DIR}/${name}/$1`;
});
makes it resolve to differing modules somehow?

Copy link
Collaborator Author

@eps1lon eps1lon Nov 24, 2021

Choose a reason for hiding this comment

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

The modules were all coming from the mapped paths.
To be clear: It makes sense that the used React needs to be the same one that's used by OuterReactDOM and therefore needs to be in the same isolatedModules callback. What's odd is, that this isn't the case with scripts/jest/config.source-www.js.

So ideally we'd have two isolated copies of react-dom that somehow share the same react copy. This no longer seems possible with Jest 27

Copy link
Contributor

@SimenB SimenB Sep 28, 2022

Choose a reason for hiding this comment

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

Seems you've worked around this, but a solution might be a new API which lets you selectively evict specific modules from the cache? jest.resetModule('react-dom') or something?

Copy link
Collaborator Author

@eps1lon eps1lon Sep 28, 2022

Choose a reason for hiding this comment

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

I haven't thought about this for a while but if I remember correctly, I came to the conclusiong that the old code should never have worked to begin with. Not sure if it's worth revisiting since this testing pattern is very specific.

Copy link
Contributor

@SimenB SimenB Sep 28, 2022

Choose a reason for hiding this comment

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

That sounds very reasonable 👍 It does sound like a weird edge case, but happy to revisit it if proves to actually guard against some regression 🙂

});
expect(OuterReactDOM).not.toBe(InnerReactDOM);
});
Expand Down