From e9a4a44aae675e1b164cf2ae509e438c324d424a Mon Sep 17 00:00:00 2001 From: Ricky Date: Tue, 4 May 2021 15:42:48 -0400 Subject: [PATCH] Add back root override for strict mode (#21428) * Add back root override for strict mode * Switch flag to boolean * Fix flow --- .../src/__tests__/ReactTestUtilsAct-test.js | 14 ++++++++++++ packages/react-dom/src/client/ReactDOMRoot.js | 3 +++ .../react-native-renderer/src/ReactFabric.js | 2 +- .../src/ReactNativeRenderer.js | 2 +- .../src/createReactNoop.js | 4 +++- .../react-reconciler/src/ReactFiber.new.js | 9 +++++++- .../react-reconciler/src/ReactFiber.old.js | 9 +++++++- .../src/ReactFiberReconciler.new.js | 2 ++ .../src/ReactFiberReconciler.old.js | 2 ++ .../src/ReactFiberRoot.new.js | 2 ++ .../src/ReactFiberRoot.old.js | 2 ++ .../src/ReactTestRenderer.js | 6 +++++ .../ReactStrictMode-test.internal.js | 22 +++++++++++++++++++ 13 files changed, 74 insertions(+), 5 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js index 8419c93c38c41..dee4b7852a837 100644 --- a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js +++ b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js @@ -102,6 +102,20 @@ describe('ReactTestUtils.act()', () => { root.render(); Scheduler.unstable_flushAll(); }); + + // @gate experimental + it('warns in concurrent mode if root is strict', () => { + expect(() => { + const root = ReactDOM.unstable_createRoot( + document.createElement('div'), + {unstable_strictMode: true}, + ); + root.render(); + Scheduler.unstable_flushAll(); + }).toErrorDev([ + 'An update to App ran an effect, but was not wrapped in act(...)', + ]); + }); }); }); diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index 73e96eb086bbc..dc4c95a41ef30 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -27,6 +27,7 @@ export type RootOptions = { mutableSources?: Array>, ... }, + unstable_strictMode?: boolean, unstable_concurrentUpdatesByDefault?: boolean, ... }; @@ -122,6 +123,7 @@ function createRootImpl( options.hydrationOptions != null && options.hydrationOptions.mutableSources) || null; + const isStrictMode = options != null && options.unstable_strictMode === true; let concurrentUpdatesByDefaultOverride = null; if (allowConcurrentByDefault) { @@ -136,6 +138,7 @@ function createRootImpl( tag, hydrate, hydrationCallbacks, + isStrictMode, concurrentUpdatesByDefaultOverride, ); markContainerAsRoot(root.current, container); diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index 9f1f44f51cf49..c516452a17661 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -207,7 +207,7 @@ function render( if (!root) { // TODO (bvaughn): If we decide to keep the wrapper component, // We could create a wrapper for containerTag as well to reduce special casing. - root = createContainer(containerTag, LegacyRoot, false, null, null); + root = createContainer(containerTag, LegacyRoot, false, null, false, null); roots.set(containerTag, root); } updateContainer(element, root, null, callback); diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index de42c6e41a58c..6f12fd5d8d445 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -203,7 +203,7 @@ function render( if (!root) { // TODO (bvaughn): If we decide to keep the wrapper component, // We could create a wrapper for containerTag as well to reduce special casing. - root = createContainer(containerTag, LegacyRoot, false, null, null); + root = createContainer(containerTag, LegacyRoot, false, null, false, null); roots.set(containerTag, root); } updateContainer(element, root, null, callback); diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 659f4d63fcc03..9d2c87621bcef 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -722,7 +722,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { if (!root) { const container = {rootID: rootID, pendingChildren: [], children: []}; rootContainers.set(rootID, container); - root = NoopRenderer.createContainer(container, tag, false, null); + root = NoopRenderer.createContainer(container, tag, false, null, null); roots.set(rootID, root); } return root.current.stateNode.containerInfo; @@ -740,6 +740,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { ConcurrentRoot, false, null, + null, ); return { _Scheduler: Scheduler, @@ -766,6 +767,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { LegacyRoot, false, null, + null, ); return { _Scheduler: Scheduler, diff --git a/packages/react-reconciler/src/ReactFiber.new.js b/packages/react-reconciler/src/ReactFiber.new.js index 13cbe26b5acc4..701c2926797da 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -422,12 +422,19 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { export function createHostRootFiber( tag: RootTag, + isStrictMode: boolean, concurrentUpdatesByDefaultOverride: null | boolean, ): Fiber { let mode; if (tag === ConcurrentRoot) { mode = ConcurrentMode; - if (enableStrictEffects && createRootStrictEffectsByDefault) { + if (isStrictMode === true) { + mode |= StrictLegacyMode; + + if (enableStrictEffects) { + mode |= StrictEffectsMode; + } + } else if (enableStrictEffects && createRootStrictEffectsByDefault) { mode |= StrictLegacyMode | StrictEffectsMode; } if ( diff --git a/packages/react-reconciler/src/ReactFiber.old.js b/packages/react-reconciler/src/ReactFiber.old.js index 40a09820beae4..fd2428b59779e 100644 --- a/packages/react-reconciler/src/ReactFiber.old.js +++ b/packages/react-reconciler/src/ReactFiber.old.js @@ -422,12 +422,19 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { export function createHostRootFiber( tag: RootTag, + isStrictMode: boolean, concurrentUpdatesByDefaultOverride: null | boolean, ): Fiber { let mode; if (tag === ConcurrentRoot) { mode = ConcurrentMode; - if (enableStrictEffects && createRootStrictEffectsByDefault) { + if (isStrictMode === true) { + mode |= StrictLegacyMode; + + if (enableStrictEffects) { + mode |= StrictEffectsMode; + } + } else if (enableStrictEffects && createRootStrictEffectsByDefault) { mode |= StrictLegacyMode | StrictEffectsMode; } if ( diff --git a/packages/react-reconciler/src/ReactFiberReconciler.new.js b/packages/react-reconciler/src/ReactFiberReconciler.new.js index b19223db377a4..c1e9778160a7c 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.new.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.new.js @@ -248,6 +248,7 @@ export function createContainer( tag: RootTag, hydrate: boolean, hydrationCallbacks: null | SuspenseHydrationCallbacks, + isStrictMode: boolean, concurrentUpdatesByDefaultOverride: null | boolean, ): OpaqueRoot { return createFiberRoot( @@ -255,6 +256,7 @@ export function createContainer( tag, hydrate, hydrationCallbacks, + isStrictMode, concurrentUpdatesByDefaultOverride, ); } diff --git a/packages/react-reconciler/src/ReactFiberReconciler.old.js b/packages/react-reconciler/src/ReactFiberReconciler.old.js index fe528b8316c82..47ced00130310 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.old.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.old.js @@ -248,6 +248,7 @@ export function createContainer( tag: RootTag, hydrate: boolean, hydrationCallbacks: null | SuspenseHydrationCallbacks, + isStrictMode: boolean, concurrentUpdatesByDefaultOverride: null | boolean, ): OpaqueRoot { return createFiberRoot( @@ -255,6 +256,7 @@ export function createContainer( tag, hydrate, hydrationCallbacks, + isStrictMode, concurrentUpdatesByDefaultOverride, ); } diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js index ae62134de1bc1..5e1f8275b694f 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.new.js +++ b/packages/react-reconciler/src/ReactFiberRoot.new.js @@ -98,6 +98,7 @@ export function createFiberRoot( tag: RootTag, hydrate: boolean, hydrationCallbacks: null | SuspenseHydrationCallbacks, + isStrictMode: boolean, concurrentUpdatesByDefaultOverride: null | boolean, ): FiberRoot { const root: FiberRoot = (new FiberRootNode(containerInfo, tag, hydrate): any); @@ -109,6 +110,7 @@ export function createFiberRoot( // stateNode is any. const uninitializedFiber = createHostRootFiber( tag, + isStrictMode, concurrentUpdatesByDefaultOverride, ); root.current = uninitializedFiber; diff --git a/packages/react-reconciler/src/ReactFiberRoot.old.js b/packages/react-reconciler/src/ReactFiberRoot.old.js index 83d4de95b6a8b..413a87fbcffc8 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.old.js +++ b/packages/react-reconciler/src/ReactFiberRoot.old.js @@ -98,6 +98,7 @@ export function createFiberRoot( tag: RootTag, hydrate: boolean, hydrationCallbacks: null | SuspenseHydrationCallbacks, + isStrictMode: boolean, concurrentUpdatesByDefaultOverride: null | boolean, ): FiberRoot { const root: FiberRoot = (new FiberRootNode(containerInfo, tag, hydrate): any); @@ -109,6 +110,7 @@ export function createFiberRoot( // stateNode is any. const uninitializedFiber = createHostRootFiber( tag, + isStrictMode, concurrentUpdatesByDefaultOverride, ); root.current = uninitializedFiber; diff --git a/packages/react-test-renderer/src/ReactTestRenderer.js b/packages/react-test-renderer/src/ReactTestRenderer.js index bc46f1d6d182f..246790ead3a46 100644 --- a/packages/react-test-renderer/src/ReactTestRenderer.js +++ b/packages/react-test-renderer/src/ReactTestRenderer.js @@ -58,6 +58,7 @@ const {IsSomeRendererActing} = ReactSharedInternals; type TestRendererOptions = { createNodeMock: (element: React$Element) => any, unstable_isConcurrent: boolean, + unstable_strictMode: boolean, unstable_concurrentUpdatesByDefault: boolean, ... }; @@ -436,6 +437,7 @@ function propsMatch(props: Object, filter: Object): boolean { function create(element: React$Element, options: TestRendererOptions) { let createNodeMock = defaultTestOptions.createNodeMock; let isConcurrent = false; + let isStrictMode = false; let concurrentUpdatesByDefault = null; if (typeof options === 'object' && options !== null) { if (typeof options.createNodeMock === 'function') { @@ -444,6 +446,9 @@ function create(element: React$Element, options: TestRendererOptions) { if (options.unstable_isConcurrent === true) { isConcurrent = true; } + if (options.unstable_strictMode === true) { + isStrictMode = true; + } if (allowConcurrentByDefault) { if (options.unstable_concurrentUpdatesByDefault !== undefined) { concurrentUpdatesByDefault = @@ -461,6 +466,7 @@ function create(element: React$Element, options: TestRendererOptions) { isConcurrent ? ConcurrentRoot : LegacyRoot, false, null, + isStrictMode, concurrentUpdatesByDefault, ); invariant(root != null, 'something went wrong'); diff --git a/packages/react/src/__tests__/ReactStrictMode-test.internal.js b/packages/react/src/__tests__/ReactStrictMode-test.internal.js index ca651fdefaf29..b1fb0bcc270ae 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.internal.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.internal.js @@ -65,6 +65,28 @@ describe('ReactStrictMode', () => { }); if (__DEV__) { + // @gate experimental + it('should support enabling strict mode via createRoot option', () => { + act(() => { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container, { + unstable_strictMode: true, + }); + root.render(); + }); + + expect(log).toEqual([ + 'A: render', + 'A: render', + 'A: useLayoutEffect mount', + 'A: useEffect mount', + 'A: useLayoutEffect unmount', + 'A: useEffect unmount', + 'A: useLayoutEffect mount', + 'A: useEffect mount', + ]); + }); + // @gate experimental it('should include legacy + strict effects mode', () => { act(() => {