From 6010e15c33437b03fbf3741b3e9398cb24910cf3 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 19 Feb 2021 13:35:26 -0500 Subject: [PATCH] Add StrictMode 'level' prop and createRoot 'strictModeLevel' option New StrictMode 'level' prop allows specifying which level of strict mode to use. If no level attribute is specified, StrictLegacyMode will be used to maintain backwards compatibility. Otherwise the following is true: * level 0 does nothing * level 1 selects StrictLegacyMode * level 2 selects StrictEffectsMode (which includes StrictLegacyMode) Levels can be increased with nesting (0 -> 1 -> 2) but not decreased. This commit also adds a new createRoot option ('unstable_strictModeLevel') to the createRoot and createBatchedRoot APIs. This option can be used to override default behavior to increase or decrease the StrictMode level of the root. A subsequent commit will add additional DEV warnings: * If a nested StrictMode tag attempts to explicitly decrease the level * If a level attribute changes in an update --- packages/react-dom/src/client/ReactDOMRoot.js | 14 +- .../src/createReactNoop.js | 5 +- .../react-reconciler/src/ReactFiber.new.js | 41 ++- .../react-reconciler/src/ReactFiber.old.js | 41 ++- .../src/ReactFiberReconciler.new.js | 9 +- .../src/ReactFiberReconciler.old.js | 9 +- .../src/ReactFiberRoot.new.js | 3 +- .../src/ReactFiberRoot.old.js | 3 +- .../src/ReactFiberWorkLoop.new.js | 8 +- .../src/ReactFiberWorkLoop.old.js | 8 +- .../src/ReactTestRenderer.js | 1 + .../ReactStrictMode-test.internal.js | 245 ++++++++++++++++++ 12 files changed, 357 insertions(+), 30 deletions(-) create mode 100644 packages/react/src/__tests__/ReactStrictMode-test.internal.js diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index 66a31328540e5..62a72dc229618 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_strictModeLevel?: number, ... }; @@ -128,7 +129,18 @@ function createRootImpl( options.hydrationOptions != null && options.hydrationOptions.mutableSources) || null; - const root = createContainer(container, tag, hydrate, hydrationCallbacks); + const strictModeLevelOverride = + options != null && options.unstable_strictModeLevel != null + ? options.unstable_strictModeLevel + : null; + + const root = createContainer( + container, + tag, + hydrate, + hydrationCallbacks, + strictModeLevelOverride, + ); markContainerAsRoot(root.current, container); const rootContainerElement = diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 26c58ffd8445c..743470966b3f0 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) { BlockingRoot, false, null, + null, ); return { _Scheduler: Scheduler, @@ -792,6 +794,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 d1a4837fdba01..ac4319b676a68 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -421,14 +421,29 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { return workInProgress; } -export function createHostRootFiber(tag: RootTag): Fiber { +export function createHostRootFiber( + tag: RootTag, + strictModeLevelOverride: null | number, +): Fiber { let mode; if (tag === ConcurrentRoot) { - if (enableStrictEffects && createRootStrictEffectsByDefault) { - mode = - ConcurrentMode | BlockingMode | StrictLegacyMode | StrictEffectsMode; + mode = ConcurrentMode | BlockingMode; + + if (strictModeLevelOverride !== null) { + if (strictModeLevelOverride >= 1) { + mode |= StrictLegacyMode; + } + if (enableStrictEffects) { + if (strictModeLevelOverride >= 2) { + mode |= StrictEffectsMode; + } + } } else { - mode = ConcurrentMode | BlockingMode | StrictLegacyMode; + if (enableStrictEffects && createRootStrictEffectsByDefault) { + mode |= StrictLegacyMode | StrictEffectsMode; + } else { + mode |= StrictLegacyMode; + } } } else if (tag === BlockingRoot) { if (enableStrictEffects && createRootStrictEffectsByDefault) { @@ -484,8 +499,20 @@ export function createFiberFromTypeAndProps( break; case REACT_STRICT_MODE_TYPE: fiberTag = Mode; - // TODO (StrictEffectsMode) Add support for new strict mode "level" attribute - mode |= StrictLegacyMode; + + // Legacy strict mode ( without any level prop) defaults to level 1. + const level = pendingProps.level == null ? 1 : pendingProps.level; + + // Levels cascade; higher levels inherit all lower level modes. + // It is explicitly not supported to lower a mode with nesting, only to increase it. + if (level >= 1) { + mode |= StrictLegacyMode; + } + if (enableStrictEffects) { + if (level >= 2) { + mode |= StrictEffectsMode; + } + } break; case REACT_PROFILER_TYPE: return createFiberFromProfiler(pendingProps, mode, lanes, key); diff --git a/packages/react-reconciler/src/ReactFiber.old.js b/packages/react-reconciler/src/ReactFiber.old.js index 6f79f0331d348..b06e6ac104b8a 100644 --- a/packages/react-reconciler/src/ReactFiber.old.js +++ b/packages/react-reconciler/src/ReactFiber.old.js @@ -421,14 +421,29 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { return workInProgress; } -export function createHostRootFiber(tag: RootTag): Fiber { +export function createHostRootFiber( + tag: RootTag, + strictModeLevelOverride: null | number, +): Fiber { let mode; if (tag === ConcurrentRoot) { - if (enableStrictEffects && createRootStrictEffectsByDefault) { - mode = - ConcurrentMode | BlockingMode | StrictLegacyMode | StrictEffectsMode; + mode = ConcurrentMode | BlockingMode; + + if (strictModeLevelOverride !== null) { + if (strictModeLevelOverride >= 1) { + mode |= StrictLegacyMode; + } + if (enableStrictEffects) { + if (strictModeLevelOverride >= 2) { + mode |= StrictEffectsMode; + } + } } else { - mode = ConcurrentMode | BlockingMode | StrictLegacyMode; + if (enableStrictEffects && createRootStrictEffectsByDefault) { + mode |= StrictLegacyMode | StrictEffectsMode; + } else { + mode |= StrictLegacyMode; + } } } else if (tag === BlockingRoot) { if (enableStrictEffects && createRootStrictEffectsByDefault) { @@ -484,8 +499,20 @@ export function createFiberFromTypeAndProps( break; case REACT_STRICT_MODE_TYPE: fiberTag = Mode; - // TODO (StrictEffectsMode) Add support for new strict mode "level" attribute - mode |= StrictLegacyMode; + + // Legacy strict mode ( without any level prop) defaults to level 1. + const level = pendingProps.level == null ? 1 : pendingProps.level; + + // Levels cascade; higher levels inherit all lower level modes. + // It is explicitly not supported to lower a mode with nesting, only to increase it. + if (level >= 1) { + mode |= StrictLegacyMode; + } + if (enableStrictEffects) { + if (level >= 2) { + mode |= StrictEffectsMode; + } + } break; case REACT_PROFILER_TYPE: return createFiberFromProfiler(pendingProps, mode, lanes, key); diff --git a/packages/react-reconciler/src/ReactFiberReconciler.new.js b/packages/react-reconciler/src/ReactFiberReconciler.new.js index 3b4edc51e28e8..cc639fa0865f6 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.new.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.new.js @@ -256,8 +256,15 @@ export function createContainer( tag: RootTag, hydrate: boolean, hydrationCallbacks: null | SuspenseHydrationCallbacks, + strictModeLevelOverride: null | number, ): OpaqueRoot { - return createFiberRoot(containerInfo, tag, hydrate, hydrationCallbacks); + return createFiberRoot( + containerInfo, + tag, + hydrate, + hydrationCallbacks, + strictModeLevelOverride, + ); } export function updateContainer( diff --git a/packages/react-reconciler/src/ReactFiberReconciler.old.js b/packages/react-reconciler/src/ReactFiberReconciler.old.js index ecbcbd8a9e5e3..3044bf934daf8 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.old.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.old.js @@ -256,8 +256,15 @@ export function createContainer( tag: RootTag, hydrate: boolean, hydrationCallbacks: null | SuspenseHydrationCallbacks, + strictModeLevelOverride: null | number, ): OpaqueRoot { - return createFiberRoot(containerInfo, tag, hydrate, hydrationCallbacks); + return createFiberRoot( + containerInfo, + tag, + hydrate, + hydrationCallbacks, + strictModeLevelOverride, + ); } export function updateContainer( diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js index 9057137ec61a8..ae6a5bdb596f6 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.new.js +++ b/packages/react-reconciler/src/ReactFiberRoot.new.js @@ -91,6 +91,7 @@ export function createFiberRoot( tag: RootTag, hydrate: boolean, hydrationCallbacks: null | SuspenseHydrationCallbacks, + strictModeLevelOverride: null | number, ): FiberRoot { const root: FiberRoot = (new FiberRootNode(containerInfo, tag, hydrate): any); if (enableSuspenseCallback) { @@ -99,7 +100,7 @@ export function createFiberRoot( // Cyclic construction. This cheats the type system right now because // stateNode is any. - const uninitializedFiber = createHostRootFiber(tag); + const uninitializedFiber = createHostRootFiber(tag, strictModeLevelOverride); root.current = uninitializedFiber; uninitializedFiber.stateNode = root; diff --git a/packages/react-reconciler/src/ReactFiberRoot.old.js b/packages/react-reconciler/src/ReactFiberRoot.old.js index 92ec811dd5589..0c0d45c098720 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.old.js +++ b/packages/react-reconciler/src/ReactFiberRoot.old.js @@ -91,6 +91,7 @@ export function createFiberRoot( tag: RootTag, hydrate: boolean, hydrationCallbacks: null | SuspenseHydrationCallbacks, + strictModeLevelOverride: null | number, ): FiberRoot { const root: FiberRoot = (new FiberRootNode(containerInfo, tag, hydrate): any); if (enableSuspenseCallback) { @@ -99,7 +100,7 @@ export function createFiberRoot( // Cyclic construction. This cheats the type system right now because // stateNode is any. - const uninitializedFiber = createHostRootFiber(tag); + const uninitializedFiber = createHostRootFiber(tag, strictModeLevelOverride); root.current = uninitializedFiber; uninitializedFiber.stateNode = root; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index ebc9fb8f17605..7caee07872517 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -105,7 +105,6 @@ import { import { NoMode, StrictLegacyMode, - StrictEffectsMode, ProfileMode, BlockingMode, ConcurrentMode, @@ -2562,10 +2561,9 @@ function commitDoubleInvokeEffectsInDEV( hasPassiveEffects: boolean, ) { if (__DEV__ && enableStrictEffects) { - // Never double-invoke effects outside of StrictEffectsMode. - if ((fiber.mode & StrictEffectsMode) === NoMode) { - return; - } + // TODO (StrictEffects) Should we set a marker on the root if it contains strict effects + // so we don't traverse unnecessarily? similar to subtreeFlags but just at the root level. + // Maybe not a big deal since this is DEV only behavior. setCurrentDebugFiberInDEV(fiber); invokeEffectsInDev(fiber, MountLayoutDev, invokeLayoutEffectUnmountInDEV); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 25b77701d8699..1467929ce4597 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -105,7 +105,6 @@ import { import { NoMode, StrictLegacyMode, - StrictEffectsMode, ProfileMode, BlockingMode, ConcurrentMode, @@ -2562,10 +2561,9 @@ function commitDoubleInvokeEffectsInDEV( hasPassiveEffects: boolean, ) { if (__DEV__ && enableStrictEffects) { - // Never double-invoke effects outside of StrictEffectsMode. - if ((fiber.mode & StrictEffectsMode) === NoMode) { - return; - } + // TODO (StrictEffects) Should we set a marker on the root if it contains strict effects + // so we don't traverse unnecessarily? similar to subtreeFlags but just at the root level. + // Maybe not a big deal since this is DEV only behavior. setCurrentDebugFiberInDEV(fiber); invokeEffectsInDev(fiber, MountLayoutDev, invokeLayoutEffectUnmountInDEV); diff --git a/packages/react-test-renderer/src/ReactTestRenderer.js b/packages/react-test-renderer/src/ReactTestRenderer.js index c4eea12e8a550..ff80440379f14 100644 --- a/packages/react-test-renderer/src/ReactTestRenderer.js +++ b/packages/react-test-renderer/src/ReactTestRenderer.js @@ -451,6 +451,7 @@ function create(element: React$Element, options: TestRendererOptions) { isConcurrent ? ConcurrentRoot : LegacyRoot, false, null, + null, ); invariant(root != null, 'something went wrong'); updateContainer(element, root, null, null); diff --git a/packages/react/src/__tests__/ReactStrictMode-test.internal.js b/packages/react/src/__tests__/ReactStrictMode-test.internal.js new file mode 100644 index 0000000000000..48fe7884b6187 --- /dev/null +++ b/packages/react/src/__tests__/ReactStrictMode-test.internal.js @@ -0,0 +1,245 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +describe('ReactStrictMode', () => { + let React; + let ReactDOM; + let act; + + beforeEach(() => { + jest.resetModules(); + React = require('react'); + ReactDOM = require('react-dom'); + + const TestUtils = require('react-dom/test-utils'); + act = TestUtils.unstable_concurrentAct; + + const ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.enableStrictEffects = true; + }); + + describe('levels', () => { + let log; + + beforeEach(() => { + log = []; + }); + + function Component({label}) { + React.useEffect(() => { + log.push(`${label}: useEffect mount`); + return () => log.push(`${label}: useEffect unmount`); + }); + + React.useLayoutEffect(() => { + log.push(`${label}: useLayoutEffect mount`); + return () => log.push(`${label}: useLayoutEffect unmount`); + }); + + log.push(`${label}: render`); + + return null; + } + + // @gate experimental + it('should default to level 1 (legacy mode)', () => { + act(() => { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + root.render( + + + , + ); + }); + + expect(log).toEqual([ + 'A: render', + 'A: render', + 'A: useLayoutEffect mount', + 'A: useEffect mount', + ]); + }); + + // @gate experimental + it('should support overriding default via createRoot option', () => { + act(() => { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container, { + unstable_strictModeLevel: 0, + }); + root.render(); + }); + + expect(log).toEqual([ + 'A: render', + 'A: useLayoutEffect mount', + 'A: useEffect mount', + ]); + }); + + // @gate experimental + it('should disable strict mode if level 0 is specified', () => { + act(() => { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container, { + unstable_strictModeLevel: 0, + }); + root.render( + + + , + ); + }); + + expect(log).toEqual([ + 'A: render', + 'A: useLayoutEffect mount', + 'A: useEffect mount', + ]); + }); + + // @gate experimental + it('should support level 1 (legacy mode)', () => { + act(() => { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + root.render( + + + , + ); + }); + + expect(log).toEqual([ + 'A: render', + 'A: render', + 'A: useLayoutEffect mount', + 'A: useEffect mount', + ]); + }); + + // @gate experimental + it('should support level 2 (legacy + strict effects mode)', () => { + act(() => { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + 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 allow level to be increased with nesting', () => { + act(() => { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container, { + unstable_strictModeLevel: 0, + }); + root.render( + <> + + + + + + + , + + , + , + ); + }); + + expect(log).toEqual([ + 'A: render', + 'B: render', + 'B: render', + 'C: render', + 'C: render', + 'A: useLayoutEffect mount', + 'B: useLayoutEffect mount', + 'C: useLayoutEffect mount', + 'A: useEffect mount', + 'B: useEffect mount', + 'C: useEffect mount', + 'C: useLayoutEffect unmount', + 'C: useEffect unmount', + 'C: useLayoutEffect mount', + 'C: useEffect mount', + ]); + }); + + // @gate experimental + it('should not allow level to be decreased with nesting', () => { + act(() => { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container, { + unstable_strictModeLevel: 2, + }); + root.render( + <> + + + + + + + , + + , + , + ); + }); + + expect(log).toEqual([ + 'A: render', + 'A: render', + 'B: render', + 'B: render', + 'C: render', + 'C: render', + 'A: useLayoutEffect mount', + 'B: useLayoutEffect mount', + 'C: useLayoutEffect mount', + 'A: useEffect mount', + 'B: useEffect mount', + 'C: useEffect mount', + 'A: useLayoutEffect unmount', + 'B: useLayoutEffect unmount', + 'C: useLayoutEffect unmount', + 'A: useEffect unmount', + 'B: useEffect unmount', + 'C: useEffect unmount', + 'A: useLayoutEffect mount', + 'B: useLayoutEffect mount', + 'C: useLayoutEffect mount', + 'A: useEffect mount', + 'B: useEffect mount', + 'C: useEffect mount', + ]); + }); + }); +});