From 3b551c82844bcfde51f0febb8e42c1a0d777df2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 22 Apr 2024 12:39:56 -0400 Subject: [PATCH] Rename the react.element symbol to react.transitional.element (#28813) We have changed the shape (and the runtime) of React Elements. To help avoid precompiled or inlined JSX having subtle breakages or deopting hidden classes, I renamed the symbol so that we can early error if private implementation details are used or mismatching versions are used. Why "transitional"? Well, because this is not the last time we'll change the shape. This is just a stepping stone to removing the `ref` field on the elements in the next version so we'll likely have to do it again. --- .../__tests__/legacy/inspectElement-test.js | 20 +- .../src/backend/ReactSymbols.js | 5 +- .../src/__tests__/ReactComponent-test.js | 26 + .../src/__tests__/ReactDOMOption-test.js | 1 + packages/react-dom/src/__tests__/refs-test.js | 2 +- .../react-reconciler/src/ReactChildFiber.js | 11 + packages/react/src/jsx/ReactJSXElement.js | 2 +- packages/shared/ReactFeatureFlags.js | 3 + packages/shared/ReactSymbols.js | 7 +- .../__tests__/ReactSymbols-test.internal.js | 1 + .../forks/ReactFeatureFlags.native-fb.js | 2 + .../forks/ReactFeatureFlags.native-oss.js | 2 + .../forks/ReactFeatureFlags.test-renderer.js | 2 + ...actFeatureFlags.test-renderer.native-fb.js | 2 + .../ReactFeatureFlags.test-renderer.www.js | 2 + .../shared/forks/ReactFeatureFlags.www.js | 2 + .../useSyncExternalStoreShared-test.js | 479 ++++++++++-------- scripts/error-codes/codes.json | 3 +- 18 files changed, 345 insertions(+), 227 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js index 10a355d0430f3..f05c832c56c15 100644 --- a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js @@ -290,9 +290,23 @@ describe('InspectedElementContext', () => { "preview_long": {boolean: true, number: 123, string: "abc"}, }, }, - "react_element": Dehydrated { - "preview_short": , - "preview_long": , + "react_element": { + "$$typeof": Dehydrated { + "preview_short": Symbol(react.element), + "preview_long": Symbol(react.element), + }, + "_owner": null, + "_store": Dehydrated { + "preview_short": {…}, + "preview_long": {}, + }, + "key": null, + "props": Dehydrated { + "preview_short": {…}, + "preview_long": {}, + }, + "ref": null, + "type": "span", }, "regexp": Dehydrated { "preview_short": /abc/giu, diff --git a/packages/react-devtools-shared/src/backend/ReactSymbols.js b/packages/react-devtools-shared/src/backend/ReactSymbols.js index 35e64abbeded0..7a7a9c107e93f 100644 --- a/packages/react-devtools-shared/src/backend/ReactSymbols.js +++ b/packages/react-devtools-shared/src/backend/ReactSymbols.js @@ -23,8 +23,9 @@ export const SERVER_CONTEXT_SYMBOL_STRING = 'Symbol(react.server_context)'; export const DEPRECATED_ASYNC_MODE_SYMBOL_STRING = 'Symbol(react.async_mode)'; -export const ELEMENT_NUMBER = 0xeac7; -export const ELEMENT_SYMBOL_STRING = 'Symbol(react.element)'; +export const ELEMENT_SYMBOL_STRING = 'Symbol(react.transitional.element)'; +export const LEGACY_ELEMENT_NUMBER = 0xeac7; +export const LEGACY_ELEMENT_SYMBOL_STRING = 'Symbol(react.element)'; export const DEBUG_TRACING_MODE_NUMBER = 0xeae1; export const DEBUG_TRACING_MODE_SYMBOL_STRING = diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index 678a7ce4f6db9..61538fed69492 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -612,6 +612,32 @@ describe('ReactComponent', () => { ); }); + // @gate renameElementSymbol + it('throws if a legacy element is used as a child', async () => { + const inlinedElement = { + $$typeof: Symbol.for('react.element'), + type: 'div', + key: null, + ref: null, + props: {}, + _owner: null, + }; + const element =
{[inlinedElement]}
; + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await expect( + act(() => { + root.render(element); + }), + ).rejects.toThrowError( + 'A React Element from an older version of React was rendered. ' + + 'This is not supported. It can happen if:\n' + + '- Multiple copies of the "react" package is used.\n' + + '- A library pre-bundled an old copy of "react" or "react/jsx-runtime".\n' + + '- A compiler tries to "inline" JSX instead of using the runtime.', + ); + }); + it('throws if a plain object even if it is in an owner', async () => { class Foo extends React.Component { render() { diff --git a/packages/react-dom/src/__tests__/ReactDOMOption-test.js b/packages/react-dom/src/__tests__/ReactDOMOption-test.js index 1661a98b1a694..4b431a85bbdee 100644 --- a/packages/react-dom/src/__tests__/ReactDOMOption-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMOption-test.js @@ -134,6 +134,7 @@ describe('ReactDOMOption', () => { }).rejects.toThrow('Objects are not valid as a React child'); }); + // @gate www it('should support element-ish child', async () => { // This is similar to . // We don't toString it because you must instead provide a value prop. diff --git a/packages/react-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js index 577d91f38f368..0c47e1728b270 100644 --- a/packages/react-dom/src/__tests__/refs-test.js +++ b/packages/react-dom/src/__tests__/refs-test.js @@ -382,7 +382,7 @@ describe('ref swapping', () => { }).rejects.toThrow('Expected ref to be a function'); }); - // @gate !enableRefAsProp + // @gate !enableRefAsProp && www it('undefined ref on manually inlined React element triggers error', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index f0fe8883b9eb4..b11e8b59ad1eb 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -32,6 +32,7 @@ import { REACT_PORTAL_TYPE, REACT_LAZY_TYPE, REACT_CONTEXT_TYPE, + REACT_LEGACY_ELEMENT_TYPE, } from 'shared/ReactSymbols'; import { HostRoot, @@ -166,6 +167,16 @@ function coerceRef( } function throwOnInvalidObjectType(returnFiber: Fiber, newChild: Object) { + if (newChild.$$typeof === REACT_LEGACY_ELEMENT_TYPE) { + throw new Error( + 'A React Element from an older version of React was rendered. ' + + 'This is not supported. It can happen if:\n' + + '- Multiple copies of the "react" package is used.\n' + + '- A library pre-bundled an old copy of "react" or "react/jsx-runtime".\n' + + '- A compiler tries to "inline" JSX instead of using the runtime.', + ); + } + // $FlowFixMe[method-unbinding] const childString = Object.prototype.toString.call(newChild); diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js index 36185639e67f0..9fea14ae5cf7b 100644 --- a/packages/react/src/jsx/ReactJSXElement.js +++ b/packages/react/src/jsx/ReactJSXElement.js @@ -162,7 +162,7 @@ function elementRefGetterWithDeprecationWarning() { /** * Factory method to create a new React element. This no longer adheres to * the class pattern, so do not use new to call it. Also, instanceof check - * will not work. Instead test $$typeof field against Symbol.for('react.element') to check + * will not work. Instead test $$typeof field against Symbol.for('react.transitional.element') to check * if something is a React Element. * * @param {*} type diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 5e987aec180e3..545afd1cdcc32 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -143,6 +143,9 @@ export const transitionLaneExpirationMs = 5000; // const __NEXT_MAJOR__ = __EXPERIMENTAL__; +// Renames the internal symbol for elements since they have changed signature/constructor +export const renameElementSymbol = true; + // Removes legacy style context export const disableLegacyContext = true; diff --git a/packages/shared/ReactSymbols.js b/packages/shared/ReactSymbols.js index 26662aa325e7c..002870896f00f 100644 --- a/packages/shared/ReactSymbols.js +++ b/packages/shared/ReactSymbols.js @@ -7,12 +7,17 @@ * @flow */ +import {renameElementSymbol} from 'shared/ReactFeatureFlags'; + // ATTENTION // When adding new symbols to this file, // Please consider also adding to 'react-devtools-shared/src/backend/ReactSymbols' // The Symbol used to tag the ReactElement-like types. -export const REACT_ELEMENT_TYPE: symbol = Symbol.for('react.element'); +export const REACT_LEGACY_ELEMENT_TYPE: symbol = Symbol.for('react.element'); +export const REACT_ELEMENT_TYPE: symbol = renameElementSymbol + ? Symbol.for('react.transitional.element') + : REACT_LEGACY_ELEMENT_TYPE; export const REACT_PORTAL_TYPE: symbol = Symbol.for('react.portal'); export const REACT_FRAGMENT_TYPE: symbol = Symbol.for('react.fragment'); export const REACT_STRICT_MODE_TYPE: symbol = Symbol.for('react.strict_mode'); diff --git a/packages/shared/__tests__/ReactSymbols-test.internal.js b/packages/shared/__tests__/ReactSymbols-test.internal.js index c651f53075a12..2ee93336ea7ee 100644 --- a/packages/shared/__tests__/ReactSymbols-test.internal.js +++ b/packages/shared/__tests__/ReactSymbols-test.internal.js @@ -23,6 +23,7 @@ describe('ReactSymbols', () => { }); }; + // @gate renameElementSymbol it('Symbol values should be unique', () => { expectToBeUnique(Object.entries(require('shared/ReactSymbols'))); }); diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index b0df95c2d6bb3..a4d0434f50469 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -69,6 +69,8 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = true; export const enableGetInspectorDataForInstanceInProduction = true; +export const renameElementSymbol = false; + export const enableRetryLaneExpiration = false; export const retryLaneExpirationMs = 5000; export const syncLaneExpirationMs = 250; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index defe3d6e0fe88..723a6a69c376b 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -104,6 +104,8 @@ export const enableDO_NOT_USE_disableStrictPassiveEffect = false; export const passChildrenWhenCloningPersistedNodes = false; export const enableEarlyReturnForPropDiffing = false; +export const renameElementSymbol = true; + // Profiling Only export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 26b4086ca19fe..2271d9b2a25db 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -79,6 +79,8 @@ export const enableServerComponentLogs = true; export const enableInfiniteRenderLoopDetection = false; export const enableEarlyReturnForPropDiffing = false; +export const renameElementSymbol = true; + // TODO: This must be in sync with the main ReactFeatureFlags file because // the Test Renderer's value must be the same as the one used by the // react package. diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index f39974ab98c98..e6b299035a99e 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -90,5 +90,7 @@ export const disableDOMTestUtils = false; export const disableDefaultPropsExceptForClasses = false; export const enableEarlyReturnForPropDiffing = false; +export const renameElementSymbol = false; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index fdb85b0be0e67..44972804eb2de 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -90,5 +90,7 @@ export const disableDOMTestUtils = false; export const disableDefaultPropsExceptForClasses = false; export const enableEarlyReturnForPropDiffing = false; +export const renameElementSymbol = false; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 753d2f27b67d5..cf634cb168700 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -65,6 +65,8 @@ export const enableSchedulingProfiler: boolean = export const disableLegacyContext = __EXPERIMENTAL__; export const enableGetInspectorDataForInstanceInProduction = false; +export const renameElementSymbol = false; + export const enableCache = true; export const enableLegacyCache = true; export const enableFetchInstrumentation = false; diff --git a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js index e01ef57c10a64..8f8fb8e9a9a5a 100644 --- a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js +++ b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js @@ -20,7 +20,6 @@ let useState; let useEffect; let useLayoutEffect; let assertLog; - let originalError; // This tests shared behavior between the built-in and shim implementations of @@ -28,7 +27,6 @@ let originalError; describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { beforeEach(() => { jest.resetModules(); - if (gate(flags => flags.enableUseSyncExternalStoreShim)) { // Test the shim against React 17. jest.mock('react', () => { @@ -49,7 +47,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { originalError = console.error; console.error = jest.fn(); } - React = require('react'); ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); @@ -57,17 +54,14 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { useState = React.useState; useEffect = React.useEffect; useLayoutEffect = React.useLayoutEffect; - const InternalTestUtils = require('internal-test-utils'); assertLog = InternalTestUtils.assertLog; - const internalAct = require('internal-test-utils').act; // The internal act implementation doesn't batch updates by default, since // it's mostly used to test concurrent mode. But since these tests run // in both concurrent and legacy mode, I'm adding batching here. act = cb => internalAct(() => ReactDOM.unstable_batchedUpdates(cb)); - if (gate(flags => flags.source)) { // The `shim/with-selector` module composes the main // `use-sync-external-store` entrypoint. In the compiled artifacts, this @@ -84,18 +78,15 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { useSyncExternalStoreWithSelector = require('use-sync-external-store/shim/with-selector').useSyncExternalStoreWithSelector; }); - afterEach(() => { if (gate(flags => flags.enableUseSyncExternalStoreShim)) { console.error = originalError; } }); - function Text({text}) { Scheduler.log(text); return text; } - function createRoot(container) { // This wrapper function exists so we can test both legacy roots and // concurrent roots. @@ -117,7 +108,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }; } } - function createExternalStore(initialState) { const listeners = new Set(); let currentState = initialState; @@ -140,41 +130,36 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }, }; } - it('basic usage', async () => { const store = createExternalStore('Initial'); - function App() { const text = useSyncExternalStore(store.subscribe, store.getState); - return ; + return React.createElement(Text, { + text: text, + }); } - const container = document.createElement('div'); const root = createRoot(container); - await act(() => root.render()); - + await act(() => root.render(React.createElement(App, null))); assertLog(['Initial']); expect(container.textContent).toEqual('Initial'); - await act(() => { store.set('Updated'); }); assertLog(['Updated']); expect(container.textContent).toEqual('Updated'); }); - it('skips re-rendering if nothing changes', async () => { const store = createExternalStore('Initial'); - function App() { const text = useSyncExternalStore(store.subscribe, store.getState); - return ; + return React.createElement(Text, { + text: text, + }); } - const container = document.createElement('div'); const root = createRoot(container); - await act(() => root.render()); - + await act(() => root.render(React.createElement(App, null))); assertLog(['Initial']); expect(container.textContent).toEqual('Initial'); @@ -186,26 +171,23 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { assertLog([]); expect(container.textContent).toEqual('Initial'); }); - it('switch to a different store', async () => { const storeA = createExternalStore(0); const storeB = createExternalStore(0); - let setStore; function App() { const [store, _setStore] = useState(storeA); setStore = _setStore; const value = useSyncExternalStore(store.subscribe, store.getState); - return ; + return React.createElement(Text, { + text: value, + }); } - const container = document.createElement('div'); const root = createRoot(container); - await act(() => root.render()); - + await act(() => root.render(React.createElement(App, null))); assertLog([0]); expect(container.textContent).toEqual('0'); - await act(() => { storeA.set(1); }); @@ -239,38 +221,43 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { assertLog([1]); expect(container.textContent).toEqual('1'); }); - it('selecting a specific value inside getSnapshot', async () => { - const store = createExternalStore({a: 0, b: 0}); - + const store = createExternalStore({ + a: 0, + b: 0, + }); function A() { const a = useSyncExternalStore(store.subscribe, () => store.getState().a); - return ; + return React.createElement(Text, { + text: 'A' + a, + }); } function B() { const b = useSyncExternalStore(store.subscribe, () => store.getState().b); - return ; + return React.createElement(Text, { + text: 'B' + b, + }); } - function App() { - return ( - <> - - - + return React.createElement( + React.Fragment, + null, + React.createElement(A, null), + React.createElement(B, null), ); } - const container = document.createElement('div'); const root = createRoot(container); - await act(() => root.render()); - + await act(() => root.render(React.createElement(App, null))); assertLog(['A0', 'B0']); expect(container.textContent).toEqual('A0B0'); // Update b but not a await act(() => { - store.set({a: 0, b: 1}); + store.set({ + a: 0, + b: 1, + }); }); // Only b re-renders assertLog(['B1']); @@ -278,7 +265,10 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { // Update a but not b await act(() => { - store.set({a: 1, b: 1}); + store.set({ + a: 1, + b: 1, + }); }); // Only a re-renders assertLog(['A1']); @@ -293,18 +283,18 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { 'mutation in between the sync and passive effects', async () => { const store = createExternalStore(0); - function App() { const value = useSyncExternalStore(store.subscribe, store.getState); useEffect(() => { Scheduler.log('Passive effect: ' + value); }, [value]); - return ; + return React.createElement(Text, { + text: value, + }); } - const container = document.createElement('div'); const root = createRoot(container); - await act(() => root.render()); + await act(() => root.render(React.createElement(App, null))); assertLog([0, 'Passive effect: 0']); // Schedule an update. We'll intentionally not use `act` so that we can @@ -331,13 +321,13 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { expect(container.textContent).toEqual('0'); }, ); - it('mutating the store in between render and commit when getSnapshot has changed', async () => { - const store = createExternalStore({a: 1, b: 1}); - + const store = createExternalStore({ + a: 1, + b: 1, + }); const getSnapshotA = () => store.getState().a; const getSnapshotB = () => store.getState().b; - function Child1({step}) { const value = useSyncExternalStore(store.subscribe, store.getState); useLayoutEffect(() => { @@ -347,37 +337,42 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { // fired yet, so it doesn't have access to the latest getSnapshot. So // it can't use the getSnapshot to bail out. Scheduler.log('Update B in commit phase'); - store.set({a: value.a, b: 2}); + store.set({ + a: value.a, + b: 2, + }); } }, [step]); return null; } - function Child2({step}) { const label = step === 0 ? 'A' : 'B'; const getSnapshot = step === 0 ? getSnapshotA : getSnapshotB; const value = useSyncExternalStore(store.subscribe, getSnapshot); - return ; + return React.createElement(Text, { + text: label + value, + }); } - let setStep; function App() { const [step, _setStep] = useState(0); setStep = _setStep; - return ( - <> - - - + return React.createElement( + React.Fragment, + null, + React.createElement(Child1, { + step: step, + }), + React.createElement(Child2, { + step: step, + }), ); } - const container = document.createElement('div'); const root = createRoot(container); - await act(() => root.render()); + await act(() => root.render(React.createElement(App, null))); assertLog(['A1']); expect(container.textContent).toEqual('A1'); - await act(() => { // Change getSnapshot and update the store in the same batch setStep(1); @@ -391,13 +386,13 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ]); expect(container.textContent).toEqual('B2'); }); - it('mutating the store in between render and commit when getSnapshot has _not_ changed', async () => { // Same as previous test, but `getSnapshot` does not change - const store = createExternalStore({a: 1, b: 1}); - + const store = createExternalStore({ + a: 1, + b: 1, + }); const getSnapshotA = () => store.getState().a; - function Child1({step}) { const value = useSyncExternalStore(store.subscribe, store.getState); useLayoutEffect(() => { @@ -407,32 +402,38 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { // fired yet, so it doesn't have access to the latest getSnapshot. So // it can't use the getSnapshot to bail out. Scheduler.log('Update B in commit phase'); - store.set({a: value.a, b: 2}); + store.set({ + a: value.a, + b: 2, + }); } }, [step]); return null; } - function Child2({step}) { const value = useSyncExternalStore(store.subscribe, getSnapshotA); - return ; + return React.createElement(Text, { + text: 'A' + value, + }); } - let setStep; function App() { const [step, _setStep] = useState(0); setStep = _setStep; - return ( - <> - - - + return React.createElement( + React.Fragment, + null, + React.createElement(Child1, { + step: step, + }), + React.createElement(Child2, { + step: step, + }), ); } - const container = document.createElement('div'); const root = createRoot(container); - await act(() => root.render()); + await act(() => root.render(React.createElement(App, null))); assertLog(['A1']); expect(container.textContent).toEqual('A1'); @@ -450,10 +451,8 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ]); expect(container.textContent).toEqual('A1'); }); - it("does not bail out if the previous update hasn't finished yet", async () => { const store = createExternalStore(0); - function Child1() { const value = useSyncExternalStore(store.subscribe, store.getState); useLayoutEffect(() => { @@ -462,85 +461,95 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { store.set(0); } }, [value]); - return ; + return React.createElement(Text, { + text: value, + }); } - function Child2() { const value = useSyncExternalStore(store.subscribe, store.getState); - return ; + return React.createElement(Text, { + text: value, + }); } - const container = document.createElement('div'); const root = createRoot(container); await act(() => root.render( - <> - - - , + React.createElement( + React.Fragment, + null, + React.createElement(Child1, null), + React.createElement(Child2, null), + ), ), ); assertLog([0, 0]); expect(container.textContent).toEqual('00'); - await act(() => { store.set(1); }); assertLog([1, 1, 'Reset back to 0', 0, 0]); expect(container.textContent).toEqual('00'); }); - it('uses the latest getSnapshot, even if it changed in the same batch as a store update', async () => { - const store = createExternalStore({a: 0, b: 0}); - + const store = createExternalStore({ + a: 0, + b: 0, + }); const getSnapshotA = () => store.getState().a; const getSnapshotB = () => store.getState().b; - let setGetSnapshot; function App() { const [getSnapshot, _setGetSnapshot] = useState(() => getSnapshotA); setGetSnapshot = _setGetSnapshot; const text = useSyncExternalStore(store.subscribe, getSnapshot); - return ; + return React.createElement(Text, { + text: text, + }); } - const container = document.createElement('div'); const root = createRoot(container); - await act(() => root.render()); + await act(() => root.render(React.createElement(App, null))); assertLog([0]); // Update the store and getSnapshot at the same time await act(() => { ReactDOM.flushSync(() => { setGetSnapshot(() => getSnapshotB); - store.set({a: 1, b: 2}); + store.set({ + a: 1, + b: 2, + }); }); }); // It should read from B instead of A assertLog([2]); expect(container.textContent).toEqual('2'); }); - it('handles errors thrown by getSnapshot', async () => { class ErrorBoundary extends React.Component { - state = {error: null}; + state = { + error: null, + }; static getDerivedStateFromError(error) { - return {error}; + return { + error, + }; } render() { if (this.state.error) { - return ; + return React.createElement(Text, { + text: this.state.error.message, + }); } return this.props.children; } } - const store = createExternalStore({ value: 0, throwInGetSnapshot: false, throwInIsEqual: false, }); - function App() { const {value} = useSyncExternalStore(store.subscribe, () => { const state = store.getState(); @@ -549,17 +558,22 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { } return state; }); - return ; + return React.createElement(Text, { + text: value, + }); } - const errorBoundary = React.createRef(null); const container = document.createElement('div'); const root = createRoot(container); await act(() => root.render( - - - , + React.createElement( + ErrorBoundary, + { + ref: errorBoundary, + }, + React.createElement(App, null), + ), ), ); assertLog([0]); @@ -586,7 +600,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }); }); } - assertLog( gate(flags => flags.enableUseSyncExternalStoreShim) ? ['Error in getSnapshot'] @@ -599,22 +612,22 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ); expect(container.textContent).toEqual('Error in getSnapshot'); }); - it('Infinite loop if getSnapshot keeps returning new reference', async () => { const store = createExternalStore({}); - function App() { const text = useSyncExternalStore(store.subscribe, () => ({})); - return ; + return React.createElement(Text, { + text: JSON.stringify(text), + }); } - const container = document.createElement('div'); const root = createRoot(container); - await expect(async () => { await expect(async () => { await act(() => { - ReactDOM.flushSync(async () => root.render()); + ReactDOM.flushSync(async () => + root.render(React.createElement(App, null)), + ); }); }).rejects.toThrow( 'Maximum update depth exceeded. This can happen when a component repeatedly ' + @@ -642,23 +655,22 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { }, ); }); - it('getSnapshot can return NaN without infinite loop warning', async () => { const store = createExternalStore('not a number'); - function App() { const value = useSyncExternalStore(store.subscribe, () => parseInt(store.getState(), 10), ); - return ; + return React.createElement(Text, { + text: value, + }); } - const container = document.createElement('div'); const root = createRoot(container); // Initial render that reads a snapshot of NaN. This is OK because we use // Object.is algorithm to compare values. - await act(() => root.render()); + await act(() => root.render(React.createElement(App, null))); expect(container.textContent).toEqual('NaN'); assertLog([NaN]); @@ -672,16 +684,16 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { expect(container.textContent).toEqual('NaN'); assertLog([NaN]); }); - describe('extra features implemented in user-space', () => { it('memoized selectors are only called once per update', async () => { - const store = createExternalStore({a: 0, b: 0}); - + const store = createExternalStore({ + a: 0, + b: 0, + }); function selector(state) { Scheduler.log('Selector'); return state.a; } - function App() { Scheduler.log('App'); const a = useSyncExternalStoreWithSelector( @@ -690,19 +702,22 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { null, selector, ); - return ; + return React.createElement(Text, { + text: 'A' + a, + }); } - const container = document.createElement('div'); const root = createRoot(container); - await act(() => root.render()); - + await act(() => root.render(React.createElement(App, null))); assertLog(['App', 'Selector', 'A0']); expect(container.textContent).toEqual('A0'); // Update the store await act(() => { - store.set({a: 1, b: 0}); + store.set({ + a: 1, + b: 0, + }); }); assertLog([ // The selector runs before React starts rendering @@ -714,19 +729,24 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { ]); expect(container.textContent).toEqual('A1'); }); - it('Using isEqual to bailout', async () => { - const store = createExternalStore({a: 0, b: 0}); - + const store = createExternalStore({ + a: 0, + b: 0, + }); function A() { const {a} = useSyncExternalStoreWithSelector( store.subscribe, store.getState, null, - state => ({a: state.a}), + state => ({ + a: state.a, + }), (state1, state2) => state1.a === state2.a, ); - return ; + return React.createElement(Text, { + text: 'A' + a, + }); } function B() { const {b} = useSyncExternalStoreWithSelector( @@ -734,32 +754,36 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { store.getState, null, state => { - return {b: state.b}; + return { + b: state.b, + }; }, (state1, state2) => state1.b === state2.b, ); - return ; + return React.createElement(Text, { + text: 'B' + b, + }); } - function App() { - return ( - <> - - - + return React.createElement( + React.Fragment, + null, + React.createElement(A, null), + React.createElement(B, null), ); } - const container = document.createElement('div'); const root = createRoot(container); - await act(() => root.render()); - + await act(() => root.render(React.createElement(App, null))); assertLog(['A0', 'B0']); expect(container.textContent).toEqual('A0B0'); // Update b but not a await act(() => { - store.set({a: 0, b: 1}); + store.set({ + a: 0, + b: 1, + }); }); // Only b re-renders assertLog(['B1']); @@ -767,16 +791,17 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { // Update a but not b await act(() => { - store.set({a: 1, b: 1}); + store.set({ + a: 1, + b: 1, + }); }); // Only a re-renders assertLog(['A1']); expect(container.textContent).toEqual('A1B1'); }); - it('basic server hydration', async () => { const store = createExternalStore('client'); - const ref = React.createRef(); function App() { const text = useSyncExternalStore( @@ -787,20 +812,22 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { useEffect(() => { Scheduler.log('Passive effect: ' + text); }, [text]); - return ( -
- -
+ return React.createElement( + 'div', + { + ref: ref, + }, + React.createElement(Text, { + text: text, + }), ); } - const container = document.createElement('div'); container.innerHTML = '
server
'; const serverRenderedDiv = container.getElementsByTagName('div')[0]; - if (gate(flags => !flags.enableUseSyncExternalStoreShim)) { await act(() => { - ReactDOMClient.hydrateRoot(container, ); + ReactDOMClient.hydrateRoot(container, React.createElement(App, null)); }); assertLog([ // First it hydrates the server rendered HTML @@ -816,9 +843,9 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { // client. To avoid this server mismatch warning, user must account for // this themselves and return the correct value inside `getSnapshot`. await act(() => { - expect(() => ReactDOM.hydrate(, container)).toErrorDev( - 'Text content did not match', - ); + expect(() => + ReactDOM.hydrate(React.createElement(App, null), container), + ).toErrorDev('Text content did not match'); }); assertLog(['client', 'Passive effect: client']); } @@ -826,10 +853,8 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { expect(ref.current).toEqual(serverRenderedDiv); }); }); - it('regression test for #23150', async () => { const store = createExternalStore('Initial'); - function App() { const text = useSyncExternalStore(store.subscribe, store.getState); const [derivedText, setDerivedText] = useState(text); @@ -837,26 +862,25 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { if (derivedText !== text.toUpperCase()) { setDerivedText(text.toUpperCase()); } - return ; + return React.createElement(Text, { + text: derivedText, + }); } - const container = document.createElement('div'); const root = createRoot(container); - await act(() => root.render()); - + await act(() => root.render(React.createElement(App, null))); assertLog(['INITIAL']); expect(container.textContent).toEqual('INITIAL'); - await act(() => { store.set('Updated'); }); assertLog(['UPDATED']); expect(container.textContent).toEqual('UPDATED'); }); - it('compares selection to rendered selection even if selector changes', async () => { - const store = createExternalStore({items: ['A', 'B']}); - + const store = createExternalStore({ + items: ['A', 'B'], + }); const shallowEqualArray = (a, b) => { if (a.length !== b.length) { return false; @@ -868,19 +892,24 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { } return true; }; - const List = React.memo(({items}) => { - return ( -
    - {items.map(text => ( -
  • - -
    • - ))} -
+ return React.createElement( + 'ul', + null, + items.map(text => + React.createElement( + 'li', + { + key: text, + }, + React.createElement(Text, { + key: text, + text: text, + }), + ), + ), ); }); - function App({step}) { const inlineSelector = state => { Scheduler.log('Inline selector'); @@ -893,28 +922,37 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { inlineSelector, shallowEqualArray, ); - return ( - <> - - - + return React.createElement( + React.Fragment, + null, + React.createElement(List, { + items: items, + }), + React.createElement(Text, { + text: 'Sibling: ' + step, + }), ); } - const container = document.createElement('div'); const root = createRoot(container); await act(() => { - root.render(); + root.render( + React.createElement(App, { + step: 0, + }), + ); }); assertLog(['Inline selector', 'A', 'B', 'C', 'Sibling: 0']); - await act(() => { - root.render(); + root.render( + React.createElement(App, { + step: 1, + }), + ); }); assertLog([ // We had to call the selector again because it's not memoized 'Inline selector', - // But because the result was the same (according to isEqual) we can // bail out of rendering the memoized list. These are skipped: // 'A', @@ -924,33 +962,38 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { 'Sibling: 1', ]); }); - describe('selector and isEqual error handling in extra', () => { let ErrorBoundary; beforeEach(() => { ErrorBoundary = class extends React.Component { - state = {error: null}; + state = { + error: null, + }; static getDerivedStateFromError(error) { - return {error}; + return { + error, + }; } render() { if (this.state.error) { - return ; + return React.createElement(Text, { + text: this.state.error.message, + }); } return this.props.children; } }; }); - it('selector can throw on update', async () => { - const store = createExternalStore({a: 'a'}); + const store = createExternalStore({ + a: 'a', + }); const selector = state => { if (typeof state.a !== 'string') { throw new TypeError('Malformed state'); } return state.a.toUpperCase(); }; - function App() { const a = useSyncExternalStoreWithSelector( store.subscribe, @@ -958,22 +1001,23 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { null, selector, ); - return ; + return React.createElement(Text, { + text: a, + }); } - const container = document.createElement('div'); const root = createRoot(container); await act(() => root.render( - - - , + React.createElement( + ErrorBoundary, + null, + React.createElement(App, null), + ), ), ); - assertLog(['A']); expect(container.textContent).toEqual('A'); - if (__DEV__ && gate(flags => flags.enableUseSyncExternalStoreShim)) { // In 17, the error is re-thrown in DEV. await expect(async () => { @@ -986,12 +1030,12 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { store.set({}); }); } - expect(container.textContent).toEqual('Malformed state'); }); - it('isEqual can throw on update', async () => { - const store = createExternalStore({a: 'A'}); + const store = createExternalStore({ + a: 'A', + }); const selector = state => state.a; const isEqual = (left, right) => { if (typeof left.a !== 'string' || typeof right.a !== 'string') { @@ -999,7 +1043,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { } return left.a.trim() === right.a.trim(); }; - function App() { const a = useSyncExternalStoreWithSelector( store.subscribe, @@ -1008,22 +1051,23 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { selector, isEqual, ); - return ; + return React.createElement(Text, { + text: a, + }); } - const container = document.createElement('div'); const root = createRoot(container); await act(() => root.render( - - - , + React.createElement( + ErrorBoundary, + null, + React.createElement(App, null), + ), ), ); - assertLog(['A']); expect(container.textContent).toEqual('A'); - if (__DEV__ && gate(flags => flags.enableUseSyncExternalStoreShim)) { // In 17, the error is re-thrown in DEV. await expect(async () => { @@ -1036,7 +1080,6 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { store.set({}); }); } - expect(container.textContent).toEqual('Malformed state'); }); }); diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 1fef8eb97413f..858c0519854b7 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -509,5 +509,6 @@ "521": "flushSyncWork should not be called from builds that support legacy mode. This is a bug in React.", "522": "Invalid form element. requestFormReset must be passed a form that was rendered by React.", "523": "The render was aborted due to being postponed.", - "524": "Values cannot be passed to next() of AsyncIterables passed to Client Components." + "524": "Values cannot be passed to next() of AsyncIterables passed to Client Components.", + "525": "A React Element from an older version of React was rendered. This is not supported. It can happen if:\n- Multiple copies of the \"react\" package is used.\n- A library pre-bundled an old copy of \"react\" or \"react/jsx-runtime\".\n- A compiler tries to \"inline\" JSX instead of using the runtime." }