diff --git a/packages/react-call-return/src/__tests__/ReactCallReturn-test.js b/packages/react-call-return/src/__tests__/ReactCallReturn-test.internal.js similarity index 97% rename from packages/react-call-return/src/__tests__/ReactCallReturn-test.js rename to packages/react-call-return/src/__tests__/ReactCallReturn-test.internal.js index 2726557f62..84018bac30 100644 --- a/packages/react-call-return/src/__tests__/ReactCallReturn-test.js +++ b/packages/react-call-return/src/__tests__/ReactCallReturn-test.internal.js @@ -10,12 +10,15 @@ 'use strict'; let React; +let ReactFeatureFlags; let ReactNoop; let ReactCallReturn; describe('ReactCallReturn', () => { beforeEach(() => { jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; React = require('react'); ReactNoop = require('react-noop-renderer'); ReactCallReturn = require('react-call-return'); diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index ab616e4d34..bfa6f80a28 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -40,7 +40,10 @@ import { Ref, } from 'shared/ReactTypeOfSideEffect'; import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState'; -import {debugRenderPhaseSideEffects} from 'shared/ReactFeatureFlags'; +import { + debugRenderPhaseSideEffects, + debugRenderPhaseSideEffectsForStrictMode, +} from 'shared/ReactFeatureFlags'; import invariant from 'fbjs/lib/invariant'; import getComponentName from 'shared/getComponentName'; import warning from 'fbjs/lib/warning'; @@ -64,7 +67,7 @@ import { } from './ReactFiberContext'; import {pushProvider} from './ReactFiberNewContext'; import {NoWork, Never} from './ReactFiberExpirationTime'; -import {AsyncUpdates} from './ReactTypeOfInternalContext'; +import {AsyncUpdates, StrictMode} from './ReactTypeOfInternalContext'; import MAX_SIGNED_31_BIT_INT from './maxSigned31BitInt'; let didWarnAboutBadClass; @@ -287,12 +290,20 @@ export default function( if (__DEV__) { ReactDebugCurrentFiber.setCurrentPhase('render'); nextChildren = instance.render(); - if (debugRenderPhaseSideEffects) { + if ( + debugRenderPhaseSideEffects || + (debugRenderPhaseSideEffectsForStrictMode && + workInProgress.internalContextTag & StrictMode) + ) { instance.render(); } ReactDebugCurrentFiber.setCurrentPhase(null); } else { - if (debugRenderPhaseSideEffects) { + if ( + debugRenderPhaseSideEffects || + (debugRenderPhaseSideEffectsForStrictMode && + workInProgress.internalContextTag & StrictMode) + ) { instance.render(); } nextChildren = instance.render(); diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index ddd5a9cde2..90ca86a992 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -13,6 +13,7 @@ import type {ExpirationTime} from './ReactFiberExpirationTime'; import {Update} from 'shared/ReactTypeOfSideEffect'; import { debugRenderPhaseSideEffects, + debugRenderPhaseSideEffectsForStrictMode, enableAsyncSubtreeAPI, warnAboutDeprecatedLifecycles, } from 'shared/ReactFeatureFlags'; @@ -391,7 +392,11 @@ export default function( : emptyObject; // Instantiate twice to help detect side-effects. - if (debugRenderPhaseSideEffects) { + if ( + debugRenderPhaseSideEffects || + (debugRenderPhaseSideEffectsForStrictMode && + workInProgress.internalContextTag & StrictMode) + ) { new ctor(props, context); // eslint-disable-line no-new } @@ -537,7 +542,11 @@ export default function( } } - if (debugRenderPhaseSideEffects) { + if ( + debugRenderPhaseSideEffects || + (debugRenderPhaseSideEffectsForStrictMode && + workInProgress.internalContextTag & StrictMode) + ) { // Invoke method an extra time to help detect side-effects. type.getDerivedStateFromProps.call( null, diff --git a/packages/react-reconciler/src/ReactFiberUpdateQueue.js b/packages/react-reconciler/src/ReactFiberUpdateQueue.js index a036c5d39e..24fae562e6 100644 --- a/packages/react-reconciler/src/ReactFiberUpdateQueue.js +++ b/packages/react-reconciler/src/ReactFiberUpdateQueue.js @@ -10,11 +10,15 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; -import {debugRenderPhaseSideEffects} from 'shared/ReactFeatureFlags'; +import { + debugRenderPhaseSideEffects, + debugRenderPhaseSideEffectsForStrictMode, +} from 'shared/ReactFeatureFlags'; import {Callback as CallbackEffect} from 'shared/ReactTypeOfSideEffect'; import {ClassComponent, HostRoot} from 'shared/ReactTypeOfWork'; import invariant from 'fbjs/lib/invariant'; import warning from 'fbjs/lib/warning'; +import {StrictMode} from './ReactTypeOfInternalContext'; import {NoWork} from './ReactFiberExpirationTime'; @@ -183,14 +187,7 @@ export function getUpdateExpirationTime(fiber: Fiber): ExpirationTime { function getStateFromUpdate(update, instance, prevState, props) { const partialState = update.partialState; if (typeof partialState === 'function') { - const updateFn = partialState; - - // Invoke setState callback an extra time to help detect side-effects. - if (debugRenderPhaseSideEffects) { - updateFn.call(instance, prevState, props); - } - - return updateFn.call(instance, prevState, props); + return partialState.call(instance, prevState, props); } else { return partialState; } @@ -276,6 +273,16 @@ export function processUpdateQueue( } } + // Invoke setState callback an extra time to help detect side-effects. + // Ignore the return value in this case. + if ( + debugRenderPhaseSideEffects || + (debugRenderPhaseSideEffectsForStrictMode && + workInProgress.internalContextTag & StrictMode) + ) { + getStateFromUpdate(update, instance, state, props); + } + // Process the update let partialState; if (update.isReplace) { diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js similarity index 99% rename from packages/react-reconciler/src/__tests__/ReactIncremental-test.js rename to packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js index 6f5c640a2a..c5f3aa1370 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncremental-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js @@ -11,12 +11,15 @@ 'use strict'; let React; +let ReactFeatureFlags; let ReactNoop; let PropTypes; describe('ReactIncremental', () => { beforeEach(() => { jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; React = require('react'); ReactNoop = require('react-noop-renderer'); PropTypes = require('prop-types'); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js similarity index 99% rename from packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js rename to packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index ae3bf926ae..056885903a 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -11,12 +11,15 @@ 'use strict'; let PropTypes; +let ReactFeatureFlags; let React; let ReactNoop; describe('ReactIncrementalErrorHandling', () => { beforeEach(() => { jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; PropTypes = require('prop-types'); React = require('react'); ReactNoop = require('react-noop-renderer'); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js index b6ad8bb135..24b45a6f3b 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js @@ -11,11 +11,14 @@ 'use strict'; let React; +let ReactFeatureFlags; let ReactNoop; describe('ReactIncrementalErrorLogging', () => { beforeEach(() => { jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; React = require('react'); ReactNoop = require('react-noop-renderer'); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.internal.js similarity index 98% rename from packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.js rename to packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.internal.js index e9ef0336a9..64decb120a 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.internal.js @@ -11,11 +11,14 @@ 'use strict'; let React; +let ReactFeatureFlags; let ReactNoop; describe('ReactIncrementalReflection', () => { beforeEach(() => { jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; React = require('react'); ReactNoop = require('react-noop-renderer'); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js similarity index 98% rename from packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js rename to packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js index 5f46ef4db2..752a21c30d 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.internal.js @@ -11,11 +11,14 @@ 'use strict'; let React; +let ReactFeatureFlags; let ReactNoop; describe('ReactIncrementalScheduling', () => { beforeEach(() => { jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; React = require('react'); ReactNoop = require('react-noop-renderer'); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js similarity index 99% rename from packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js rename to packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js index 5a2bdb022f..d093ebbb16 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalSideEffects-test.internal.js @@ -11,11 +11,14 @@ 'use strict'; let React; +let ReactFeatureFlags; let ReactNoop; describe('ReactIncrementalSideEffects', () => { beforeEach(() => { jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; React = require('react'); ReactNoop = require('react-noop-renderer'); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalTriangle-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalTriangle-test.internal.js similarity index 98% rename from packages/react-reconciler/src/__tests__/ReactIncrementalTriangle-test.js rename to packages/react-reconciler/src/__tests__/ReactIncrementalTriangle-test.internal.js index fae9bc1646..ab40f3a395 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalTriangle-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalTriangle-test.internal.js @@ -11,11 +11,14 @@ 'use strict'; let React; +let ReactFeatureFlags; let ReactNoop; describe('ReactIncrementalTriangle', () => { beforeEach(() => { jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; React = require('react'); ReactNoop = require('react-noop-renderer'); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js similarity index 98% rename from packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js rename to packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js index ff4a35611f..99591b7c18 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js @@ -11,11 +11,14 @@ 'use strict'; let React; +let ReactFeatureFlags; let ReactNoop; describe('ReactIncrementalUpdates', () => { beforeEach(() => { jest.resetModuleRegistry(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; React = require('react'); ReactNoop = require('react-noop-renderer'); }); diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js index cf366cf05a..ed1a96d5e7 100644 --- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js @@ -20,6 +20,7 @@ describe('ReactNewContext', () => { beforeEach(() => { jest.resetModules(); ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; ReactFeatureFlags.enableNewContextAPI = true; React = require('react'); ReactNoop = require('react-noop-renderer'); diff --git a/packages/react/src/__tests__/ReactStrictMode-test.internal.js b/packages/react/src/__tests__/ReactStrictMode-test.internal.js index 15f339a66a..5bab8df2dd 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.internal.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.internal.js @@ -140,6 +140,158 @@ describe('ReactStrictMode', () => { }); }); + [true, false].forEach(debugRenderPhaseSideEffectsForStrictMode => { + describe(`StrictMode (${debugRenderPhaseSideEffectsForStrictMode})`, () => { + beforeEach(() => { + jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = debugRenderPhaseSideEffectsForStrictMode; + React = require('react'); + ReactTestRenderer = require('react-test-renderer'); + }); + + it('should invoke precommit lifecycle methods twice in DEV', () => { + const {StrictMode} = React; + + let log = []; + let shouldComponentUpdate = false; + + function Root() { + return ( + + + + ); + } + + class ClassComponent extends React.Component { + state = {}; + static getDerivedStateFromProps() { + log.push('getDerivedStateFromProps'); + return null; + } + constructor(props) { + super(props); + log.push('constructor'); + } + componentDidMount() { + log.push('componentDidMount'); + } + componentDidUpdate() { + log.push('componentDidUpdate'); + } + componentWillUnmount() { + log.push('componentWillUnmount'); + } + shouldComponentUpdate() { + log.push('shouldComponentUpdate'); + return shouldComponentUpdate; + } + render() { + log.push('render'); + return null; + } + } + + const component = ReactTestRenderer.create(); + + if (debugRenderPhaseSideEffectsForStrictMode) { + expect(log).toEqual([ + 'constructor', + 'constructor', + 'getDerivedStateFromProps', + 'getDerivedStateFromProps', + 'render', + 'render', + 'componentDidMount', + ]); + } else { + expect(log).toEqual([ + 'constructor', + 'getDerivedStateFromProps', + 'render', + 'componentDidMount', + ]); + } + + log = []; + shouldComponentUpdate = true; + + component.update(); + if (debugRenderPhaseSideEffectsForStrictMode) { + expect(log).toEqual([ + 'getDerivedStateFromProps', + 'getDerivedStateFromProps', + 'shouldComponentUpdate', + 'render', + 'render', + 'componentDidUpdate', + ]); + } else { + expect(log).toEqual([ + 'getDerivedStateFromProps', + 'shouldComponentUpdate', + 'render', + 'componentDidUpdate', + ]); + } + + log = []; + shouldComponentUpdate = false; + + component.update(); + if (debugRenderPhaseSideEffectsForStrictMode) { + expect(log).toEqual([ + 'getDerivedStateFromProps', + 'getDerivedStateFromProps', + 'shouldComponentUpdate', + ]); + } else { + expect(log).toEqual([ + 'getDerivedStateFromProps', + 'shouldComponentUpdate', + ]); + } + }); + + it('should invoke setState callbacks twice in DEV', () => { + const {StrictMode} = React; + + let instance; + class ClassComponent extends React.Component { + state = { + count: 1, + }; + render() { + instance = this; + return null; + } + } + + let setStateCount = 0; + + ReactTestRenderer.create( + + + , + ); + instance.setState(state => { + setStateCount++; + return { + count: state.count + 1, + }; + }); + + // Callback should be invoked twice (in DEV) + expect(setStateCount).toBe( + debugRenderPhaseSideEffectsForStrictMode ? 2 : 1, + ); + // But each time `state` should be the previous value + expect(instance.state.count).toBe(2); + }); + }); + }); + describe('async subtree', () => { beforeEach(() => { jest.resetModules(); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 83b07a03f8..7ed2c183c5 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -26,6 +26,12 @@ export const enableNewContextAPI = false; // Helps identify side effects in begin-phase lifecycle hooks and setState reducers: export const debugRenderPhaseSideEffects = false; +// In some cases, StrictMode should also double-render lifecycles. +// This can be confusing for tests though, +// And it can be bad for performance in production. +// This feature flag can be used to control the behavior: +export const debugRenderPhaseSideEffectsForStrictMode = __DEV__; + // Warn about deprecated, async-unsafe lifecycles; relates to RFC #6: export const warnAboutDeprecatedLifecycles = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fabric.js b/packages/shared/forks/ReactFeatureFlags.native-fabric.js index 689df2c13c..3176779501 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fabric.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fabric.js @@ -13,6 +13,7 @@ import typeof * as FeatureFlagsType from 'shared/ReactFeatureFlags'; import typeof * as FabricFeatureFlagsType from './ReactFeatureFlags.native-fabric'; export const debugRenderPhaseSideEffects = false; +export const debugRenderPhaseSideEffectsForStrictMode = false; export const enableAsyncSubtreeAPI = true; export const enableCreateRoot = false; export const enableUserTimingAPI = __DEV__; diff --git a/packages/shared/forks/ReactFeatureFlags.native.js b/packages/shared/forks/ReactFeatureFlags.native.js index 38c6675c95..d43c381a1c 100644 --- a/packages/shared/forks/ReactFeatureFlags.native.js +++ b/packages/shared/forks/ReactFeatureFlags.native.js @@ -15,6 +15,7 @@ import typeof * as FeatureFlagsShimType from './ReactFeatureFlags.native'; // Re-export dynamic flags from the fbsource version. export const { debugRenderPhaseSideEffects, + debugRenderPhaseSideEffectsForStrictMode, warnAboutDeprecatedLifecycles, } = require('ReactFeatureFlags'); diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 23eb86bfab..a080504f4c 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -13,6 +13,7 @@ import typeof * as FeatureFlagsShimType from './ReactFeatureFlags.www'; // Re-export dynamic flags from the www version. export const { debugRenderPhaseSideEffects, + debugRenderPhaseSideEffectsForStrictMode, warnAboutDeprecatedLifecycles, } = require('ReactFeatureFlags');