Skip to content

[code-infra] Fix StrictMode effects not being called twice in React 19 tests #45812

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

Merged
merged 4 commits into from
Apr 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .mocharc.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,7 @@ module.exports = {
'**/build/**',
'docs/.next/**',
],
// detect-modules doesn't work with @babel/register
// https://github.com/babel/babel/issues/6737
'node-option': ['no-experimental-detect-module'],
Copy link
Member Author

Choose a reason for hiding this comment

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

Cherry-picked from 0f92cc3 (#45630)

Copy link
Member

Choose a reason for hiding this comment

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

This breaks again in Node 23, but ok, let's worry about that later

};
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@
"@pigment-css/react": "0.0.30",
"@playwright/test": "1.51.1",
"@types/babel__core": "^7.20.5",
"@types/babel__register": "^7.17.3",
"@types/fs-extra": "^11.0.4",
"@types/lodash": "^4.17.16",
"@types/mocha": "^10.0.10",
Expand Down
36 changes: 18 additions & 18 deletions packages-internal/test-utils/src/createRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
screen as rtlScreen,
Screen,
render as testingLibraryRender,
RenderOptions as TestingLibraryRenderOptions,
within,
} from '@testing-library/react/pure';
import { userEvent } from '@testing-library/user-event';
Expand Down Expand Up @@ -201,7 +202,7 @@ const customQueries = {
findAllDescriptionsOf,
};

interface RenderConfiguration {
interface RenderConfiguration extends Pick<TestingLibraryRenderOptions, 'reactStrictMode'> {
/**
* https://testing-library.com/docs/react-testing-library/api#container
*/
Expand Down Expand Up @@ -232,7 +233,7 @@ interface ServerRenderConfiguration extends RenderConfiguration {
container: HTMLElement;
}

export type RenderOptions = Partial<RenderConfiguration>;
export type RenderOptions = Omit<Partial<RenderConfiguration>, 'reactStrictMode'>;

export interface MuiRenderResult extends RenderResult<typeof queries & typeof customQueries> {
user: ReturnType<typeof userEvent.setup>;
Expand All @@ -256,14 +257,15 @@ function render(
element: React.ReactElement<DataAttributes>,
configuration: ClientRenderConfiguration,
): MuiRenderResult {
const { container, hydrate, wrapper } = configuration;
const { container, hydrate, wrapper, reactStrictMode } = configuration;

const testingLibraryRenderResult = traceSync('render', () =>
testingLibraryRender(element, {
container,
hydrate,
queries: { ...queries, ...customQueries },
wrapper,
reactStrictMode,
}),
);
const result: MuiRenderResult = {
Expand Down Expand Up @@ -628,24 +630,16 @@ export function createRenderer(globalOptions: CreateRendererOptions = {}): Rende
serverContainer = null!;
});

function createWrapper(options: Partial<RenderConfiguration>) {
const {
strict = globalStrict,
strictEffects = globalStrictEffects,
wrapper: InnerWrapper = React.Fragment,
} = options;
function createWrapper(options: Pick<RenderOptions, 'wrapper'>) {
const { wrapper: InnerWrapper = React.Fragment } = options;

const usesLegacyRoot = reactMajor < 18;
const Mode = strict && (strictEffects || usesLegacyRoot) ? React.StrictMode : React.Fragment;
return function Wrapper({ children }: { children?: React.ReactNode }) {
return (
<Mode>
<EmotionCacheProvider value={emotionCache}>
<React.Profiler id={profiler.id} onRender={profiler.onRender}>
<InnerWrapper>{children}</InnerWrapper>
</React.Profiler>
</EmotionCacheProvider>
</Mode>
<EmotionCacheProvider value={emotionCache}>
<React.Profiler id={profiler.id} onRender={profiler.onRender}>
<InnerWrapper>{children}</InnerWrapper>
</React.Profiler>
</EmotionCacheProvider>
);
};
}
Expand All @@ -661,8 +655,14 @@ export function createRenderer(globalOptions: CreateRendererOptions = {}): Rende
);
}

const usesLegacyRoot = reactMajor < 18;
const reactStrictMode =
(options.strict ?? globalStrict) &&
((options.strictEffects ?? globalStrictEffects) || usesLegacyRoot);

return render(element, {
...options,
reactStrictMode,
hydrate: false,
wrapper: createWrapper(options),
});
Expand Down
2 changes: 1 addition & 1 deletion packages-internal/test-utils/src/setupBabel.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
require('@babel/register')({
extensions: ['.js', '.mjs', '.ts', '.tsx'],
extensions: ['.js', '.mjs', '.cjs', '.jsx', '.ts', '.tsx'],
});
4 changes: 2 additions & 2 deletions packages/mui-base/src/FocusTrap/FocusTrap.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import * as ReactDOM from 'react-dom';
import { expect } from 'chai';
import { act, createRenderer, screen } from '@mui/internal-test-utils';
import { act, createRenderer, reactMajor, screen } from '@mui/internal-test-utils';
import { FocusTrap } from '@mui/base/FocusTrap';
import { Portal } from '@mui/base/Portal';

Expand Down Expand Up @@ -219,7 +219,7 @@ describe('<FocusTrap />', () => {
</div>
);
}
const { setProps, getByRole } = render(<Test />);
const { setProps, getByRole } = render(<Test />, { strict: reactMajor <= 18 });
expect(screen.getByTestId('root')).toHaveFocus();

act(() => {
Expand Down
6 changes: 3 additions & 3 deletions packages/mui-base/src/Input/Input.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import { createRenderer, fireEvent, screen, act } from '@mui/internal-test-utils';
import { createRenderer, fireEvent, screen, act, reactMajor } from '@mui/internal-test-utils';
import { expect } from 'chai';
import { spy } from 'sinon';
import { Input, inputClasses, InputOwnerState } from '@mui/base/Input';
Expand Down Expand Up @@ -281,8 +281,8 @@ describe('<Input />', () => {
);
}).toErrorDev([
'MUI: You have provided a `slots.input` to the input component\nthat does not correctly handle the `ref` prop.\nMake sure the `ref` prop is called with a HTMLInputElement.',
// React 18 Strict Effects run mount effects twice
React.version.startsWith('18') &&
// React Strict Mode runs mount effects twice
reactMajor >= 18 &&
'MUI: You have provided a `slots.input` to the input component\nthat does not correctly handle the `ref` prop.\nMake sure the `ref` prop is called with a HTMLInputElement.',
]);
});
Expand Down
4 changes: 3 additions & 1 deletion packages/mui-base/src/Portal/Portal.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import { expect } from 'chai';
import { spy } from 'sinon';
import { createRenderer } from '@mui/internal-test-utils';
import { createRenderer, reactMajor } from '@mui/internal-test-utils';
import { Portal, PortalProps } from '@mui/base/Portal';

describe('<Portal />', () => {
Expand Down Expand Up @@ -44,6 +44,7 @@ describe('<Portal />', () => {
<Portal disablePortal ref={refSpy}>
<h1 className="woofPortal">Foo</h1>
</Portal>,
{ strict: reactMajor <= 18 },
);
const mountNode = document.querySelector('.woofPortal');
expect(refSpy.args).to.deep.equal([[mountNode]]);
Expand All @@ -57,6 +58,7 @@ describe('<Portal />', () => {
<Portal disablePortal ref={refSpy}>
<h1 className="woofPortal">Foo</h1>
</Portal>,
{ strict: reactMajor <= 18 },
);
const mountNode = document.querySelector('.woofPortal');
expect(refSpy.args).to.deep.equal([[mountNode]]);
Expand Down
8 changes: 8 additions & 0 deletions packages/mui-base/src/useAutocomplete/useAutocomplete.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,14 @@ describe('useAutocomplete', () => {
aboveErrorTestComponentMessage,
aboveErrorTestComponentMessage,
],
19: [
muiErrorMessage,
muiErrorMessage,
nodeErrorMessage,
nodeErrorMessage,
nodeErrorMessage,
nodeErrorMessage,
],
};

const devErrorMessages = errorMessagesByReactMajor[reactMajor] || defaultErrorMessages;
Expand Down
12 changes: 8 additions & 4 deletions packages/mui-joy/src/Autocomplete/Autocomplete.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1931,7 +1931,8 @@ describe('Joy <Autocomplete />', () => {

await user.click(screen.getByText('Reset'));

const expectedCallCount = reactMajor === 18 ? 4 : 2;
// eslint-disable-next-line no-nested-ternary
const expectedCallCount = reactMajor >= 19 ? 3 : reactMajor === 18 ? 4 : 2;

expect(handleInputChange.callCount).to.equal(expectedCallCount);
expect(handleInputChange.args[expectedCallCount - 1][1]).to.equal(options[1].name);
Expand Down Expand Up @@ -2209,7 +2210,8 @@ describe('Joy <Autocomplete />', () => {

expect(handleHighlightChange.callCount).to.equal(
// FIXME: highlighted index implementation should be implemented using React not the DOM.
reactMajor >= 18 ? 4 : 3,
// eslint-disable-next-line no-nested-ternary
reactMajor >= 19 ? 5 : reactMajor >= 18 ? 4 : 3,
);
if (reactMajor >= 18) {
expect(handleHighlightChange.args[2][0]).to.equal(undefined);
Expand All @@ -2223,7 +2225,8 @@ describe('Joy <Autocomplete />', () => {
fireEvent.keyDown(textbox, { key: 'ArrowDown' });
expect(handleHighlightChange.callCount).to.equal(
// FIXME: highlighted index implementation should be implemented using React not the DOM.
reactMajor >= 18 ? 5 : 4,
// eslint-disable-next-line no-nested-ternary
reactMajor >= 19 ? 6 : reactMajor >= 18 ? 5 : 4,
);
expect(handleHighlightChange.lastCall.args[0]).not.to.equal(undefined);
expect(handleHighlightChange.lastCall.args[1]).to.equal(options[1]);
Expand All @@ -2240,7 +2243,8 @@ describe('Joy <Autocomplete />', () => {
fireEvent.mouseMove(firstOption);
expect(handleHighlightChange.callCount).to.equal(
// FIXME: highlighted index implementation should be implemented using React not the DOM.
reactMajor >= 18 ? 4 : 3,
// eslint-disable-next-line no-nested-ternary
reactMajor >= 19 ? 5 : reactMajor >= 18 ? 4 : 3,
);
if (reactMajor >= 18) {
expect(handleHighlightChange.args[2][0]).to.equal(undefined);
Expand Down
4 changes: 2 additions & 2 deletions packages/mui-material/src/InputBase/InputBase.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ describe('<InputBase />', () => {

let expectedOccurrences = 1;

if (reactMajor === 18) {
if (reactMajor >= 18) {
expectedOccurrences = 2;
}

Expand Down Expand Up @@ -507,7 +507,7 @@ describe('<InputBase />', () => {

let expectedOccurrences = 1;

if (reactMajor === 18) {
if (reactMajor >= 18) {
expectedOccurrences = 2;
}
expect(() => {
Expand Down
4 changes: 3 additions & 1 deletion packages/mui-material/src/Portal/Portal.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import { expect } from 'chai';
import { spy } from 'sinon';
import { createRenderer } from '@mui/internal-test-utils';
import { createRenderer, reactMajor } from '@mui/internal-test-utils';
import Portal, { PortalProps } from '@mui/material/Portal';

describe('<Portal />', () => {
Expand Down Expand Up @@ -44,6 +44,7 @@ describe('<Portal />', () => {
<Portal disablePortal ref={refSpy}>
<h1 className="woofPortal">Foo</h1>
</Portal>,
{ strict: reactMajor <= 18 },
);
const mountNode = document.querySelector('.woofPortal');
expect(refSpy.args).to.deep.equal([[mountNode]]);
Expand All @@ -57,6 +58,7 @@ describe('<Portal />', () => {
<Portal disablePortal ref={refSpy}>
<h1 className="woofPortal">Foo</h1>
</Portal>,
{ strict: reactMajor <= 18 },
);
const mountNode = document.querySelector('.woofPortal');
expect(refSpy.args).to.deep.equal([[mountNode]]);
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-material/src/Select/Select.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ describe('<Select />', () => {

let expectedOccurrences = 2;

if (reactMajor === 18) {
if (reactMajor >= 18) {
expectedOccurrences = 3;
}

Expand Down
8 changes: 4 additions & 4 deletions packages/mui-material/src/Tabs/Tabs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,11 +372,11 @@ describe('<Tabs />', () => {
);
}).toErrorDev([
'You can provide one of the following values: 1, 3',
// React 18 Strict Effects run mount effects twice
reactMajor === 18 && 'You can provide one of the following values: 1, 3',
// React Strict Mode runs mount effects twice
reactMajor >= 18 && 'You can provide one of the following values: 1, 3',
'You can provide one of the following values: 1, 3',
// React 18 Strict Effects run mount effects twice
reactMajor === 18 && 'You can provide one of the following values: 1, 3',
// React Strict Mode runs mount effects twice
reactMajor >= 18 && 'You can provide one of the following values: 1, 3',
'You can provide one of the following values: 1, 3',
'You can provide one of the following values: 1, 3',
]);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import * as ReactDOM from 'react-dom';
import { expect } from 'chai';
import { act, createRenderer, screen } from '@mui/internal-test-utils';
import { act, createRenderer, reactMajor, screen } from '@mui/internal-test-utils';
import FocusTrap from '@mui/material/Unstable_TrapFocus';
import Portal from '@mui/material/Portal';

Expand Down Expand Up @@ -219,7 +219,7 @@ describe('<FocusTrap />', () => {
</div>
);
}
const { setProps, getByRole } = render(<Test />);
const { setProps, getByRole } = render(<Test />, { strict: reactMajor <= 18 });
expect(screen.getByTestId('root')).toHaveFocus();

act(() => {
Expand Down
16 changes: 9 additions & 7 deletions packages/mui-material/src/styles/ThemeProviderWithVars.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { expect } from 'chai';
import { createRenderer, screen, fireEvent } from '@mui/internal-test-utils';
import { createRenderer, screen, fireEvent, reactMajor } from '@mui/internal-test-utils';
import Box from '@mui/material/Box';
import {
CssVarsProvider,
Expand Down Expand Up @@ -470,11 +470,11 @@ describe('[Material UI] ThemeProviderWithVars', () => {
}
const { container } = render(<App />);

expect(container).to.have.text('1 light');
expect(container).to.have.text(`${reactMajor >= 19 ? 2 : 1} light`);

fireEvent.click(screen.getByRole('button'));

expect(container).to.have.text('1 light');
expect(container).to.have.text(`${reactMajor >= 19 ? 2 : 1} light`);
});

it('palette mode should change if not using CSS variables', () => {
Expand Down Expand Up @@ -505,12 +505,14 @@ describe('[Material UI] ThemeProviderWithVars', () => {
}
const { container } = render(<App />);

expect(container).to.have.text(`1 light ${createTheme().palette.primary.main}`);
expect(container).to.have.text(
`${reactMajor >= 19 ? 2 : 1} light ${createTheme().palette.primary.main}`,
);

fireEvent.click(screen.getByRole('button'));

expect(container).to.have.text(
`2 dark ${createTheme({ palette: { mode: 'dark' } }).palette.primary.main}`,
`${reactMajor >= 19 ? 3 : 2} dark ${createTheme({ palette: { mode: 'dark' } }).palette.primary.main}`,
);
});

Expand Down Expand Up @@ -542,10 +544,10 @@ describe('[Material UI] ThemeProviderWithVars', () => {
}
const { container } = render(<App />);

expect(container).to.have.text('1 light');
expect(container).to.have.text(`${reactMajor >= 19 ? 2 : 1} light`);

fireEvent.click(screen.getByRole('button'));

expect(container).to.have.text('2 dark');
expect(container).to.have.text(`${reactMajor >= 19 ? 3 : 2} dark`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,14 @@ describe('useAutocomplete', () => {
aboveErrorTestComponentMessage,
aboveErrorTestComponentMessage,
],
19: [
muiErrorMessage,
muiErrorMessage,
nodeErrorMessage,
nodeErrorMessage,
nodeErrorMessage,
nodeErrorMessage,
],
};

const devErrorMessages = errorMessagesByReactMajor[reactMajor] || defaultErrorMessages;
Expand Down
4 changes: 2 additions & 2 deletions packages/mui-system/src/cssVars/useCurrentColorScheme.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import { expect } from 'chai';
import { spy } from 'sinon';
import { createRenderer, fireEvent, act, screen, reactMajor } from '@mui/internal-test-utils';
import { createRenderer, fireEvent, act, screen } from '@mui/internal-test-utils';
import {
DEFAULT_MODE_STORAGE_KEY,
DEFAULT_COLOR_SCHEME_STORAGE_KEY,
Expand Down Expand Up @@ -111,7 +111,7 @@ describe('useCurrentColorScheme', () => {
const { container } = render(<Data />);

expect(container.firstChild.textContent).to.equal('light');
expect(effectRunCount).to.equal(reactMajor >= 19 ? 2 : 3);
expect(effectRunCount).to.equal(3);
});

it('[noSsr] does not trigger a re-render', () => {
Expand Down
Loading
Loading