From f7b698d0f1089a984a9dfc80d1a82a15398a050e Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 24 Nov 2021 12:56:30 +0100 Subject: [PATCH 1/4] Revert "[useMediaQuery] Fix crash in Safari < 14 and IE 11 (#29776)" This reverts commit fb9808c62e610ddd082a151827c46db022f6f0fe. --- .../src/useMediaQuery/useMediaQuery.test.js | 5 ++--- .../mui-material/src/useMediaQuery/useMediaQuery.ts | 10 ++++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/mui-material/src/useMediaQuery/useMediaQuery.test.js b/packages/mui-material/src/useMediaQuery/useMediaQuery.test.js index 082f4ea33dc847..3ed1a94f5d762d 100644 --- a/packages/mui-material/src/useMediaQuery/useMediaQuery.test.js +++ b/packages/mui-material/src/useMediaQuery/useMediaQuery.test.js @@ -22,11 +22,10 @@ function createMatchMedia(width, ref) { matches: mediaQuery.match(query, { width, }), - // Mocking matchMedia in Safari < 14 where MediaQueryList doesn't inherit from EventTarget - addListener: (listener) => { + addEventListener: (type, listener) => { listeners.push(listener); }, - removeListener: (listener) => { + removeEventListener: (type, listener) => { const index = listeners.indexOf(listener); if (index > -1) { listeners.splice(index, 1); diff --git a/packages/mui-material/src/useMediaQuery/useMediaQuery.ts b/packages/mui-material/src/useMediaQuery/useMediaQuery.ts index bd77ff672f70cb..eefd057dbdcf0a 100644 --- a/packages/mui-material/src/useMediaQuery/useMediaQuery.ts +++ b/packages/mui-material/src/useMediaQuery/useMediaQuery.ts @@ -75,11 +75,10 @@ function useMediaQueryOld( } }; updateMatch(); - // TODO: Use `addEventListener` once support for Safari < 14 is dropped - queryList.addListener(updateMatch); + queryList.addEventListener('change', updateMatch); return () => { active = false; - queryList.removeListener(updateMatch); + queryList.removeEventListener('change', updateMatch); }; }, [query, matchMedia, supportMatchMedia]); @@ -110,10 +109,9 @@ function useMediaQueryNew( return [ () => mediaQueryList.matches, (notify: () => void) => { - // TODO: Use `addEventListener` once support for Safari < 14 is dropped - mediaQueryList.addListener(notify); + mediaQueryList.addEventListener('change', notify); return () => { - mediaQueryList.removeListener(notify); + mediaQueryList.removeEventListener('change', notify); }; }, ]; From 1eb6f65ac526c98b90acf136fee340545a5597ae Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 24 Nov 2021 12:56:46 +0100 Subject: [PATCH 2/4] Revert "[useMediaQuery] Ensure no tearing in React 18 (#28491)" This reverts commit e5471f38b4de723a648059faa1c0ea2b23b37839. --- .../src/useMediaQuery/useMediaQuery.test.js | 20 ++- .../src/useMediaQuery/useMediaQuery.ts | 127 +++++------------- scripts/use-react-dist-tag.js | 9 +- 3 files changed, 41 insertions(+), 115 deletions(-) diff --git a/packages/mui-material/src/useMediaQuery/useMediaQuery.test.js b/packages/mui-material/src/useMediaQuery/useMediaQuery.test.js index 3ed1a94f5d762d..0e9e12a5776639 100644 --- a/packages/mui-material/src/useMediaQuery/useMediaQuery.test.js +++ b/packages/mui-material/src/useMediaQuery/useMediaQuery.test.js @@ -13,8 +13,6 @@ import mediaQuery from 'css-mediaquery'; import { expect } from 'chai'; import { stub } from 'sinon'; -const usesUseSyncExternalStore = React.useSyncExternalStore !== undefined; - function createMatchMedia(width, ref) { const listeners = []; return (query) => { @@ -22,10 +20,10 @@ function createMatchMedia(width, ref) { matches: mediaQuery.match(query, { width, }), - addEventListener: (type, listener) => { + addListener: (listener) => { listeners.push(listener); }, - removeEventListener: (type, listener) => { + removeListener: (listener) => { const index = listeners.indexOf(listener); if (index > -1) { listeners.splice(index, 1); @@ -119,7 +117,7 @@ describe('useMediaQuery', () => { render(); expect(screen.getByTestId('matches').textContent).to.equal('false'); - expect(getRenderCountRef.current()).to.equal(usesUseSyncExternalStore ? 1 : 2); + expect(getRenderCountRef.current()).to.equal(2); }); }); @@ -159,10 +157,10 @@ describe('useMediaQuery', () => { render(); expect(screen.getByTestId('matches').textContent).to.equal('false'); - expect(getRenderCountRef.current()).to.equal(usesUseSyncExternalStore ? 1 : 2); + expect(getRenderCountRef.current()).to.equal(2); }); - it('should render once if the default value does not match the expectation but `noSsr` is enabled', () => { + it('should render once if the default value does not match the expectation', () => { const getRenderCountRef = React.createRef(); const Test = () => { const matches = useMediaQuery('(min-width:2000px)', { @@ -199,13 +197,13 @@ describe('useMediaQuery', () => { const { unmount } = render(); expect(screen.getByTestId('matches').textContent).to.equal('false'); - expect(getRenderCountRef.current()).to.equal(usesUseSyncExternalStore ? 1 : 2); + expect(getRenderCountRef.current()).to.equal(2); unmount(); render(); expect(screen.getByTestId('matches').textContent).to.equal('false'); - expect(getRenderCountRef.current()).to.equal(usesUseSyncExternalStore ? 1 : 2); + expect(getRenderCountRef.current()).to.equal(2); }); it('should be able to change the query dynamically', () => { @@ -227,10 +225,10 @@ describe('useMediaQuery', () => { const { setProps } = render(); expect(screen.getByTestId('matches').textContent).to.equal('false'); - expect(getRenderCountRef.current()).to.equal(usesUseSyncExternalStore ? 1 : 2); + expect(getRenderCountRef.current()).to.equal(2); setProps({ query: '(min-width:100px)' }); expect(screen.getByTestId('matches').textContent).to.equal('true'); - expect(getRenderCountRef.current()).to.equal(usesUseSyncExternalStore ? 2 : 4); + expect(getRenderCountRef.current()).to.equal(4); }); it('should observe the media query', () => { diff --git a/packages/mui-material/src/useMediaQuery/useMediaQuery.ts b/packages/mui-material/src/useMediaQuery/useMediaQuery.ts index eefd057dbdcf0a..60038013f960a7 100644 --- a/packages/mui-material/src/useMediaQuery/useMediaQuery.ts +++ b/packages/mui-material/src/useMediaQuery/useMediaQuery.ts @@ -26,24 +26,42 @@ export type MuiMediaQueryListListener = (event: MuiMediaQueryListEvent) => void; export interface Options { defaultMatches?: boolean; matchMedia?: typeof window.matchMedia; - /** - * This option is kept for backwards compatibility and has no longer any effect. - * It's previous behavior is now handled automatically. - */ - // TODO: Deprecate for v6 noSsr?: boolean; ssrMatchMedia?: (query: string) => { matches: boolean }; } -function useMediaQueryOld( - query: string, - defaultMatches: boolean, - matchMedia: typeof window.matchMedia | null, - ssrMatchMedia: ((query: string) => { matches: boolean }) | null, - noSsr: boolean | undefined, +export default function useMediaQuery( + queryInput: string | ((theme: Theme) => string), + options: Options = {}, ): boolean { + const theme = useTheme(); + // Wait for jsdom to support the match media feature. + // All the browsers MUI support have this built-in. + // This defensive check is here for simplicity. + // Most of the time, the match media logic isn't central to people tests. const supportMatchMedia = typeof window !== 'undefined' && typeof window.matchMedia !== 'undefined'; + const { + defaultMatches = false, + matchMedia = supportMatchMedia ? window.matchMedia : null, + noSsr = false, + ssrMatchMedia = null, + } = getThemeProps({ name: 'MuiUseMediaQuery', props: options, theme }); + + if (process.env.NODE_ENV !== 'production') { + if (typeof queryInput === 'function' && theme === null) { + console.error( + [ + 'MUI: The `query` argument provided is invalid.', + 'You are providing a function without a theme in the context.', + 'One of the parent elements needs to use a ThemeProvider.', + ].join('\n'), + ); + } + } + + let query = typeof queryInput === 'function' ? queryInput(theme) : queryInput; + query = query.replace(/^@media( ?)/m, ''); const [match, setMatch] = React.useState(() => { if (noSsr && supportMatchMedia) { @@ -75,96 +93,13 @@ function useMediaQueryOld( } }; updateMatch(); - queryList.addEventListener('change', updateMatch); + queryList.addListener(updateMatch); return () => { active = false; - queryList.removeEventListener('change', updateMatch); + queryList.removeListener(updateMatch); }; }, [query, matchMedia, supportMatchMedia]); - return match; -} - -function useMediaQueryNew( - query: string, - defaultMatches: boolean, - matchMedia: typeof window.matchMedia | null, - ssrMatchMedia: ((query: string) => { matches: boolean }) | null, -): boolean { - const getDefaultSnapshot = React.useCallback(() => defaultMatches, [defaultMatches]); - const getServerSnapshot = React.useMemo(() => { - if (ssrMatchMedia !== null) { - const { matches } = ssrMatchMedia(query); - return () => matches; - } - return getDefaultSnapshot; - }, [getDefaultSnapshot, query, ssrMatchMedia]); - const [getSnapshot, subscribe] = React.useMemo(() => { - if (matchMedia === null) { - return [getDefaultSnapshot, () => () => {}]; - } - - const mediaQueryList = matchMedia(query); - - return [ - () => mediaQueryList.matches, - (notify: () => void) => { - mediaQueryList.addEventListener('change', notify); - return () => { - mediaQueryList.removeEventListener('change', notify); - }; - }, - ]; - }, [getDefaultSnapshot, matchMedia, query]); - const match = (React as any).useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot); - - return match; -} - -export default function useMediaQuery( - queryInput: string | ((theme: Theme) => string), - options: Options = {}, -): boolean { - const theme = useTheme(); - // Wait for jsdom to support the match media feature. - // All the browsers MUI support have this built-in. - // This defensive check is here for simplicity. - // Most of the time, the match media logic isn't central to people tests. - const supportMatchMedia = - typeof window !== 'undefined' && typeof window.matchMedia !== 'undefined'; - const { - defaultMatches = false, - matchMedia = supportMatchMedia ? window.matchMedia : null, - ssrMatchMedia = null, - noSsr, - } = getThemeProps({ name: 'MuiUseMediaQuery', props: options, theme }); - - if (process.env.NODE_ENV !== 'production') { - if (typeof queryInput === 'function' && theme === null) { - console.error( - [ - 'MUI: The `query` argument provided is invalid.', - 'You are providing a function without a theme in the context.', - 'One of the parent elements needs to use a ThemeProvider.', - ].join('\n'), - ); - } - } - - let query = typeof queryInput === 'function' ? queryInput(theme) : queryInput; - query = query.replace(/^@media( ?)/m, ''); - - // TODO: Drop `useMediaQueryOld` and use `use-sync-external-store` shim in `useMediaQueryNew` once the package is stable - const useMediaQueryImplementation = - (React as any).useSyncExternalStore !== undefined ? useMediaQueryNew : useMediaQueryOld; - const match = useMediaQueryImplementation( - query, - defaultMatches, - matchMedia, - ssrMatchMedia, - noSsr, - ); - if (process.env.NODE_ENV !== 'production') { // eslint-disable-next-line react-hooks/rules-of-hooks React.useDebugValue({ query, match }); diff --git a/scripts/use-react-dist-tag.js b/scripts/use-react-dist-tag.js index 0a5c0d94cda847..405d4de4831de0 100644 --- a/scripts/use-react-dist-tag.js +++ b/scripts/use-react-dist-tag.js @@ -16,14 +16,7 @@ const { promisify } = require('util'); const exec = promisify(childProcess.exec); // packages published from the react monorepo using the same version -const reactPackageNames = [ - 'react', - 'react-dom', - 'react-is', - 'react-test-renderer', - 'scheduler', - 'use-sync-external-store', -]; +const reactPackageNames = ['react', 'react-dom', 'react-is', 'react-test-renderer', 'scheduler']; async function main(options) { const { distTag } = options; From e8db2479bc4428b893f90d613b04da2336c94c00 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 24 Nov 2021 12:57:00 +0100 Subject: [PATCH 3/4] Revert "[utils] Use built-in hook when available for useId (#26489)" This reverts commit 3323b23d8c5b6c03627f9c46634db3b595795433. --- .../src/Autocomplete/Autocomplete.test.js | 5 + .../src/RadioGroup/RadioGroup.test.js | 49 ++++------ packages/mui-utils/src/useId.test.js | 97 ++++--------------- packages/mui-utils/src/useId.ts | 18 +--- 4 files changed, 42 insertions(+), 127 deletions(-) diff --git a/packages/mui-material/src/Autocomplete/Autocomplete.test.js b/packages/mui-material/src/Autocomplete/Autocomplete.test.js index 6b857e753a9e5a..26b9f892d58016 100644 --- a/packages/mui-material/src/Autocomplete/Autocomplete.test.js +++ b/packages/mui-material/src/Autocomplete/Autocomplete.test.js @@ -1384,6 +1384,11 @@ describe('', () => { }).toWarnDev([ 'returns duplicated headers', !strictModeDoubleLoggingSupressed && 'returns duplicated headers', + // React 18 Strict Effects run mount effects twice which lead to a cascading update + React.version.startsWith('18') && 'returns duplicated headers', + React.version.startsWith('18') && + !strictModeDoubleLoggingSupressed && + 'returns duplicated headers', ]); const options = screen.getAllByRole('option').map((el) => el.textContent); expect(options).to.have.length(7); diff --git a/packages/mui-material/src/RadioGroup/RadioGroup.test.js b/packages/mui-material/src/RadioGroup/RadioGroup.test.js index cbbd5ada49d98f..573f4bc2f82350 100644 --- a/packages/mui-material/src/RadioGroup/RadioGroup.test.js +++ b/packages/mui-material/src/RadioGroup/RadioGroup.test.js @@ -91,11 +91,10 @@ describe('', () => { , ); - const [arbitraryRadio, ...radios] = getAllByRole('radio'); - // `name` **property** will always be a string even if the **attribute** is omitted - expect(arbitraryRadio.name).not.to.equal(''); - // all input[type="radio"] have the same name - expect(new Set(radios.map((radio) => radio.name))).to.have.length(1); + const radios = getAllByRole('radio'); + + expect(radios[0].name).to.match(/^mui-[0-9]+/); + expect(radios[1].name).to.match(/^mui-[0-9]+/); }); it('should support number value', () => { @@ -300,20 +299,21 @@ describe('', () => { }); describe('useRadioGroup', () => { - describe('from props', () => { - const MinimalRadio = React.forwardRef(function MinimalRadio(_, ref) { - const radioGroup = useRadioGroup(); - return ; - }); + const RadioGroupController = React.forwardRef((_, ref) => { + const radioGroup = useRadioGroup(); + React.useImperativeHandle(ref, () => radioGroup, [radioGroup]); + return null; + }); - const RadioGroupControlled = React.forwardRef(function RadioGroupControlled(props, ref) { - return ( - - - - ); - }); + const RadioGroupControlled = React.forwardRef(function RadioGroupControlled(props, ref) { + return ( + + + + ); + }); + describe('from props', () => { it('should have the name prop from the instance', () => { const radioGroupRef = React.createRef(); const { setProps } = render(); @@ -338,7 +338,7 @@ describe('', () => { const radioGroupRef = React.createRef(); const { setProps } = render(); - expect(radioGroupRef.current.name).not.to.equal(''); + expect(radioGroupRef.current.name).to.match(/^mui-[0-9]+/); setProps({ name: 'anotherGroup' }); expect(radioGroupRef.current).to.have.property('name', 'anotherGroup'); @@ -346,19 +346,6 @@ describe('', () => { }); describe('callbacks', () => { - const RadioGroupController = React.forwardRef((_, ref) => { - const radioGroup = useRadioGroup(); - React.useImperativeHandle(ref, () => radioGroup, [radioGroup]); - return null; - }); - - const RadioGroupControlled = React.forwardRef(function RadioGroupControlled(props, ref) { - return ( - - - - ); - }); describe('onChange', () => { it('should set the value state', () => { const radioGroupRef = React.createRef(); diff --git a/packages/mui-utils/src/useId.test.js b/packages/mui-utils/src/useId.test.js index a69bd9b5228f75..66f22745cb5ee3 100644 --- a/packages/mui-utils/src/useId.test.js +++ b/packages/mui-utils/src/useId.test.js @@ -1,97 +1,36 @@ import * as React from 'react'; +import PropTypes from 'prop-types'; import { expect } from 'chai'; -import { createRenderer, screen } from 'test/utils'; +import { createRenderer } from 'test/utils'; import useId from './useId'; +const TestComponent = ({ id: idProp }) => { + const id = useId(idProp); + return {id}; +}; + +TestComponent.propTypes = { + id: PropTypes.string, +}; + describe('useId', () => { - const { render, renderToString } = createRenderer(); + const { render } = createRenderer(); it('returns the provided ID', () => { - const TestComponent = ({ id: idProp }) => { - const id = useId(idProp); - return ; - }; - const { hydrate } = renderToString(); - const { setProps } = hydrate(); + const { getByText, setProps } = render(); - expect(screen.getByTestId('target')).to.have.property('id', 'some-id'); + expect(getByText('some-id')).not.to.equal(null); setProps({ id: 'another-id' }); - - expect(screen.getByTestId('target')).to.have.property('id', 'another-id'); + expect(getByText('another-id')).not.to.equal(null); }); it("generates an ID if one isn't provided", () => { - const TestComponent = ({ id: idProp }) => { - const id = useId(idProp); - return ; - }; - const { hydrate } = renderToString(); - const { setProps } = hydrate(); + const { getByText, setProps } = render(); - expect(screen.getByTestId('target').id).not.to.equal(''); + expect(getByText(/^mui-[0-9]+$/)).not.to.equal(null); setProps({ id: 'another-id' }); - expect(screen.getByTestId('target')).to.have.property('id', 'another-id'); - }); - - it('can be suffixed', () => { - function Widget() { - const id = useId(); - const labelId = `${id}-label`; - - return ( - - - - Label - - - ); - } - render(); - - expect(screen.getByTestId('labelable')).to.have.attr( - 'aria-labelledby', - screen.getByTestId('label').id, - ); - }); - - it('can be used in in IDREF attributes', () => { - function Widget() { - const labelPartA = useId(); - const labelPartB = useId(); - - return ( - - - - A - - - B - - - ); - } - render(); - - expect(screen.getByTestId('labelable')).to.have.attr( - 'aria-labelledby', - `${screen.getByTestId('labelA').id} ${screen.getByTestId('labelB').id}`, - ); - }); - - it('provides an ID on server in React 18', function test() { - if (React.useId === undefined) { - this.skip(); - } - const TestComponent = () => { - const id = useId(); - return ; - }; - renderToString(); - - expect(screen.getByTestId('target').id).not.to.equal(''); + expect(getByText('another-id')).not.to.equal(null); }); }); diff --git a/packages/mui-utils/src/useId.ts b/packages/mui-utils/src/useId.ts index 95ca13a8604fc4..a756b4df1fb5b5 100644 --- a/packages/mui-utils/src/useId.ts +++ b/packages/mui-utils/src/useId.ts @@ -1,6 +1,6 @@ import * as React from 'react'; -function useRandomId(idOverride?: string): string | undefined { +export default function useId(idOverride?: string): string | undefined { const [defaultId, setDefaultId] = React.useState(idOverride); const id = idOverride || defaultId; React.useEffect(() => { @@ -13,19 +13,3 @@ function useRandomId(idOverride?: string): string | undefined { }, [defaultId]); return id; } - -/** - * - * @example
- * @param idOverride - * @returns {string} - */ -export default function useReactId(idOverride?: string): string | undefined { - // TODO: Remove `React as any` once `useId` is part of stable types. - if ((React as any).useId !== undefined) { - const reactId = (React as any).useId(); - return idOverride ?? reactId; - } - // eslint-disable-next-line react-hooks/rules-of-hooks -- `React.useId` is invariant at runtime. - return useRandomId(idOverride); -} From 26025e372e96a615f0b057c28511b46fad363b9c Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 24 Nov 2021 13:21:31 +0100 Subject: [PATCH 4/4] Keep test --- .../src/RadioGroup/RadioGroup.test.js | 49 ++++++++++++------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/packages/mui-material/src/RadioGroup/RadioGroup.test.js b/packages/mui-material/src/RadioGroup/RadioGroup.test.js index 573f4bc2f82350..cbbd5ada49d98f 100644 --- a/packages/mui-material/src/RadioGroup/RadioGroup.test.js +++ b/packages/mui-material/src/RadioGroup/RadioGroup.test.js @@ -91,10 +91,11 @@ describe('', () => { , ); - const radios = getAllByRole('radio'); - - expect(radios[0].name).to.match(/^mui-[0-9]+/); - expect(radios[1].name).to.match(/^mui-[0-9]+/); + const [arbitraryRadio, ...radios] = getAllByRole('radio'); + // `name` **property** will always be a string even if the **attribute** is omitted + expect(arbitraryRadio.name).not.to.equal(''); + // all input[type="radio"] have the same name + expect(new Set(radios.map((radio) => radio.name))).to.have.length(1); }); it('should support number value', () => { @@ -299,21 +300,20 @@ describe('', () => { }); describe('useRadioGroup', () => { - const RadioGroupController = React.forwardRef((_, ref) => { - const radioGroup = useRadioGroup(); - React.useImperativeHandle(ref, () => radioGroup, [radioGroup]); - return null; - }); + describe('from props', () => { + const MinimalRadio = React.forwardRef(function MinimalRadio(_, ref) { + const radioGroup = useRadioGroup(); + return ; + }); - const RadioGroupControlled = React.forwardRef(function RadioGroupControlled(props, ref) { - return ( - - - - ); - }); + const RadioGroupControlled = React.forwardRef(function RadioGroupControlled(props, ref) { + return ( + + + + ); + }); - describe('from props', () => { it('should have the name prop from the instance', () => { const radioGroupRef = React.createRef(); const { setProps } = render(); @@ -338,7 +338,7 @@ describe('', () => { const radioGroupRef = React.createRef(); const { setProps } = render(); - expect(radioGroupRef.current.name).to.match(/^mui-[0-9]+/); + expect(radioGroupRef.current.name).not.to.equal(''); setProps({ name: 'anotherGroup' }); expect(radioGroupRef.current).to.have.property('name', 'anotherGroup'); @@ -346,6 +346,19 @@ describe('', () => { }); describe('callbacks', () => { + const RadioGroupController = React.forwardRef((_, ref) => { + const radioGroup = useRadioGroup(); + React.useImperativeHandle(ref, () => radioGroup, [radioGroup]); + return null; + }); + + const RadioGroupControlled = React.forwardRef(function RadioGroupControlled(props, ref) { + return ( + + + + ); + }); describe('onChange', () => { it('should set the value state', () => { const radioGroupRef = React.createRef();