diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 23d8037b4ff25..37f1208e0e676 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -34,6 +34,8 @@ import { disableSchedulerTimeoutInWorkLoop, enableDoubleInvokingEffects, skipUnmountedBoundaries, + enableStrictEffectsModeDevWarningForFacebookOnly, + bypassStrictEffectsModeDevWarningForFacebookOnly, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; @@ -2578,6 +2580,8 @@ function flushRenderPhaseStrictModeWarningsInDEV() { } } +let didWarnAboutStrictEffectsMode: boolean = false; + function commitDoubleInvokeEffectsInDEV( fiber: Fiber, hasPassiveEffects: boolean, @@ -2588,6 +2592,42 @@ function commitDoubleInvokeEffectsInDEV( return; } + if ( + __DEV__ && + enableStrictEffectsModeDevWarningForFacebookOnly && + !bypassStrictEffectsModeDevWarningForFacebookOnly + ) { + if (!didWarnAboutStrictEffectsMode) { + didWarnAboutStrictEffectsMode = true; + + // While we roll out strict effects mode within Facebook, + // let's provide a little more context about what it is and why it exists. + // This is primarily intended for people who did not read the internal announcement. + // + // Note we don't check subtreeFlags here to verify there actually are effects + // because it's rare enough for there not to be that it's unnecessary. + // + // The warning includes Facebook specific instructions for how to disable it + // (using a GK blocklist). If we decide to add a similar warning for OSS we'll need + // to revisit both the wording and the opt-out mechansim. + // + // eslint-disable-next-line react-internal/no-production-logging + console.warn( + 'React strict effects mode is enabled for this application. ' + + 'In this mode, React will run effects twice after a component mounts ' + + '(create -> destroy -> recreate) to simulate the component being unmounted ' + + 'and then remounted as may happen with future React APIs like Offscreen. ' + + 'Mount-only effects (ones with an empty dependencies array) should be ' + + 'resilient to being run more than once.' + + '\n\nTo disable this warning, add yourself to the blocklist for the' + + 'react_enable_strict_effects_mode_warning GK here: ' + + 'https://fburl.com/react-disable-strict-effects-mode-warning' + + '\n\nFor more information about strict effects mode, see ' + + 'https://fburl.com/react-strict-effects-mode', + ); + } + } + setCurrentDebugFiberInDEV(fiber); invokeEffectsInDev(fiber, MountLayoutDev, invokeLayoutEffectUnmountInDEV); if (hasPassiveEffects) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index e1431de6431fd..0b55952a8961f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -35,6 +35,8 @@ import { enableDoubleInvokingEffects, skipUnmountedBoundaries, enableDiscreteEventMicroTasks, + enableStrictEffectsModeDevWarningForFacebookOnly, + bypassStrictEffectsModeDevWarningForFacebookOnly, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; @@ -2558,6 +2560,8 @@ function flushRenderPhaseStrictModeWarningsInDEV() { } } +let didWarnAboutStrictEffectsMode: boolean = false; + function commitDoubleInvokeEffectsInDEV( fiber: Fiber, hasPassiveEffects: boolean, @@ -2568,6 +2572,42 @@ function commitDoubleInvokeEffectsInDEV( return; } + if ( + __DEV__ && + enableStrictEffectsModeDevWarningForFacebookOnly && + !bypassStrictEffectsModeDevWarningForFacebookOnly + ) { + if (!didWarnAboutStrictEffectsMode) { + didWarnAboutStrictEffectsMode = true; + + // While we roll out strict effects mode within Facebook, + // let's provide a little more context about what it is and why it exists. + // This is primarily intended for people who did not read the internal announcement. + // + // Note we don't check subtreeFlags here to verify there actually are effects + // because it's rare enough for there not to be that it's unnecessary. + // + // The warning includes Facebook specific instructions for how to disable it + // (using a GK blocklist). If we decide to add a similar warning for OSS we'll need + // to revisit both the wording and the opt-out mechansim. + // + // eslint-disable-next-line react-internal/no-production-logging + console.warn( + 'React strict effects mode is enabled for this application. ' + + 'In this mode, React will run effects twice after a component mounts ' + + '(create -> destroy -> recreate) to simulate the component being unmounted ' + + 'and then remounted as may happen with future React APIs like Offscreen. ' + + 'Mount-only effects (ones with an empty dependencies array) should be ' + + 'resilient to being run more than once.' + + '\n\nTo disable this warning, add yourself to the blocklist for the' + + 'react_enable_strict_effects_mode_warning GK here: ' + + 'https://fburl.com/react-disable-strict-effects-mode-warning' + + '\n\nFor more information about strict effects mode, see ' + + 'https://fburl.com/react-strict-effects-mode', + ); + } + } + setCurrentDebugFiberInDEV(fiber); invokeEffectsInDev(fiber, MountLayoutDev, invokeLayoutEffectUnmountInDEV); if (hasPassiveEffects) { diff --git a/packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.internal.js b/packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.internal.js index b53b7b719c6ba..329a0b4380bd3 100644 --- a/packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.internal.js @@ -31,6 +31,77 @@ describe('ReactDoubleInvokeEvents', () => { ReactFeatureFlags.enableDoubleInvokingEffects = shouldDoubleInvokingEffects(); }); + describe('Facebook strict effects mode warning', () => { + beforeEach(() => { + ReactFeatureFlags.enableStrictEffectsModeDevWarningForFacebookOnly = true; + ReactFeatureFlags.bypassStrictEffectsModeDevWarningForFacebookOnly = false; + }); + + it('should log before double invoking effects the first time', () => { + function Component() { + React.useLayoutEffect(() => {}, []); + return null; + } + + expect(() => { + ReactNoop.act(() => { + ReactNoop.render( + <> + + , + ); + }); + }).toWarnDev( + 'React strict effects mode is enabled for this application.', + { + withoutStack: true, + }, + ); + + // But not on updates, even if new components have mounted. + ReactNoop.act(() => { + ReactNoop.render( + <> + + + , + ); + }); + }); + + it('should not log for legacy roots even if strict mode is enabled', () => { + function Component() { + React.useLayoutEffect(() => {}, []); + return null; + } + + ReactNoop.act(() => { + ReactNoop.renderLegacySyncRoot( + + + , + ); + }); + }); + + it('should not log if bypass flag has been enabled (by GK allowlist)', () => { + ReactFeatureFlags.bypassStrictEffectsModeDevWarningForFacebookOnly = true; + + function Component() { + React.useLayoutEffect(() => {}, []); + return null; + } + + ReactNoop.act(() => { + ReactNoop.render( + + + , + ); + }); + }); + }); + it('should not double invoke effects in legacy mode', () => { function App({text}) { React.useEffect(() => { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 53f9b744cb9c3..42e4a49644cac 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -154,3 +154,18 @@ export const disableSchedulerTimeoutInWorkLoop = false; export const enableTransitionEntanglement = false; export const enableDiscreteEventMicroTasks = false; + +// This flag controls a temporary DEV mode warning that explains strict effects mode. +// This flag should only be enabled for Facebook builds, because the warning includes +// Facebook specific instructions for how to disable it (using a GK blocklist). +// If we decide to add a similar warning for OSS we'll need to revisit both the +// wording and the opt-out mechansim. +// +// This warning is controlled by two flags so that (a) we can enable it for 100% +// of Facebook engineers¹² only and (b) provide a simple opt-out mechanism for engineers +// who know about the mode and no longer want to see the warning. +// +// ¹ This warning will not be logged for legacy applications that aren't in strict effects mode. +// ² GKs within Facebook can't be enabled for 100% of a population. +export const enableStrictEffectsModeDevWarningForFacebookOnly = false; +export const bypassStrictEffectsModeDevWarningForFacebookOnly = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index a230930cb0a6a..3c6a75cb98a33 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -60,6 +60,8 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableTransitionEntanglement = false; export const enableDiscreteEventMicroTasks = false; +export const enableStrictEffectsModeDevWarningForFacebookOnly = false; +export const bypassStrictEffectsModeDevWarningForFacebookOnly = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 0dc9bed575843..7ab4666f64959 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -59,6 +59,8 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableTransitionEntanglement = false; export const enableDiscreteEventMicroTasks = false; +export const enableStrictEffectsModeDevWarningForFacebookOnly = false; +export const bypassStrictEffectsModeDevWarningForFacebookOnly = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 5652b66a0b835..becb1443f404a 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -59,6 +59,8 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableTransitionEntanglement = false; export const enableDiscreteEventMicroTasks = false; +export const enableStrictEffectsModeDevWarningForFacebookOnly = false; +export const bypassStrictEffectsModeDevWarningForFacebookOnly = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index 8962bcce36a41..5c7a50341005e 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -46,6 +46,8 @@ export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; export const disableNativeComponentFrames = false; export const skipUnmountedBoundaries = false; +export const enableStrictEffectsModeDevWarningForFacebookOnly = false; +export const bypassStrictEffectsModeDevWarningForFacebookOnly = false; export const enableNewReconciler = false; export const deferRenderPhaseUpdateToNextBatch = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 7905485bce686..37816a19429ff 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -59,6 +59,8 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableTransitionEntanglement = false; export const enableDiscreteEventMicroTasks = false; +export const enableStrictEffectsModeDevWarningForFacebookOnly = false; +export const bypassStrictEffectsModeDevWarningForFacebookOnly = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index c1a20dd91a324..090c720f10a56 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -59,6 +59,8 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableTransitionEntanglement = false; export const enableDiscreteEventMicroTasks = false; +export const enableStrictEffectsModeDevWarningForFacebookOnly = false; +export const bypassStrictEffectsModeDevWarningForFacebookOnly = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index c274d6a376450..84411cdcdcf3d 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -59,6 +59,8 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableTransitionEntanglement = false; export const enableDiscreteEventMicroTasks = false; +export const enableStrictEffectsModeDevWarningForFacebookOnly = false; +export const bypassStrictEffectsModeDevWarningForFacebookOnly = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 47d8763d039b2..439917bcb9d3c 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -20,6 +20,11 @@ export const enableLegacyFBSupport = __VARIANT__; export const decoupleUpdatePriorityFromScheduler = __VARIANT__; export const skipUnmountedBoundaries = __VARIANT__; +// enableStrictEffectsModeDevWarningForFacebookOnly is true for all Facebook builds by default +// because a GK shouldn't be used to roll a feature to 100% of employees. +// So this flag is disabled by default in the dynamic file to avoid spamming tests. +export const bypassStrictEffectsModeDevWarningForFacebookOnly = true; + // Enable this flag to help with concurrent mode debugging. // It logs information to the console about React scheduling, rendering, and commit phases. // diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 2b1ba37ee8486..4ed22215c1dcf 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -33,6 +33,7 @@ export const { disableSchedulerTimeoutInWorkLoop, enableTransitionEntanglement, enableDiscreteEventMicroTasks, + bypassStrictEffectsModeDevWarningForFacebookOnly, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build. @@ -87,6 +88,8 @@ export const warnUnstableRenderSubtreeIntoContainer = false; export const enableDiscreteEventFlushingChange = true; +export const enableStrictEffectsModeDevWarningForFacebookOnly = __DEV__; + // Enable forked reconciler. Piggy-backing on the "variant" global so that we // don't have to add another test dimension. The build system will compile this // to the correct value.