From ef8c6e68118a8e6f6d29e1010f9e4eda927064ce Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Thu, 8 Sep 2022 23:00:55 -0700 Subject: [PATCH 1/4] Use dispatcheronupdate for concurrent mode --- .../src/ReactFiberHooks.new.js | 33 +++-- .../src/ReactFiberHooks.old.js | 33 +++-- .../ReactHooksWithNoopRenderer-test.js | 134 +++++++++++++++++- 3 files changed, 183 insertions(+), 17 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 87a65254b40f..3f3845948b7d 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -232,7 +232,6 @@ let ignorePreviousDependencies: boolean = false; function mountHookTypesDev() { if (__DEV__) { const hookName = ((currentHookNameInDev: any): HookType); - if (hookTypesDev === null) { hookTypesDev = [hookName]; } else { @@ -269,7 +268,7 @@ function checkDepsAreArrayDev(deps: mixed) { } } -function warnOnHookMismatchInDev(currentHookName: HookType) { +function warnOnHookMismatchInDev(currentHookName: ?HookType) { if (__DEV__) { const componentName = getComponentNameFromFiber(currentlyRenderingFiber); if (!didWarnAboutMismatchedHooksForComponent.has(componentName)) { @@ -280,12 +279,18 @@ function warnOnHookMismatchInDev(currentHookName: HookType) { const secondColumnStart = 30; - for (let i = 0; i <= ((hookTypesUpdateIndexDev: any): number); i++) { + const endIndex = + hookTypesUpdateIndexDev < 0 + ? hookTypesDev.length - 1 + : hookTypesUpdateIndexDev; + for (let i = 0; i <= endIndex; i++) { const oldHookName = hookTypesDev[i]; const newHookName = - i === ((hookTypesUpdateIndexDev: any): number) + (hookTypesUpdateIndexDev < 0 + ? null + : i === ((hookTypesUpdateIndexDev: any): number) ? currentHookName - : oldHookName; + : oldHookName) ?? undefined; let row = `${i + 1}. ${oldHookName}`; @@ -416,7 +421,10 @@ export function renderWithHooks( // Non-stateful hooks (e.g. context) don't get added to memoizedState, // so memoizedState would be null during updates and mounts. if (__DEV__) { - if (current !== null && current.memoizedState !== null) { + if ( + current !== null && + (current.mode & ConcurrentMode || current.memoizedState !== null) + ) { ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV; } else if (hookTypesDev !== null) { // This dispatcher handles an edge case where a component is updating, @@ -430,7 +438,9 @@ export function renderWithHooks( } } else { ReactCurrentDispatcher.current = - current === null || current.memoizedState === null + current === null || + ((current.mode & ConcurrentMode) === NoMode && + current.memoizedState === null) ? HooksDispatcherOnMount : HooksDispatcherOnUpdate; } @@ -485,7 +495,14 @@ export function renderWithHooks( ReactCurrentDispatcher.current = ContextOnlyDispatcher; if (__DEV__) { - workInProgress._debugHookTypes = hookTypesDev; + workInProgress._debugHookTypes = hookTypesDev ?? []; + + if (hookTypesDev !== null) { + if (currentHookNameInDev == null && hookTypesDev.length) { + // Renders no hook from some hooks + warnOnHookMismatchInDev(null); + } + } } // This check uses currentHook so that it works the same in DEV and prod bundles. diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index fd081fe5b0a6..b930bad8d715 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -232,7 +232,6 @@ let ignorePreviousDependencies: boolean = false; function mountHookTypesDev() { if (__DEV__) { const hookName = ((currentHookNameInDev: any): HookType); - if (hookTypesDev === null) { hookTypesDev = [hookName]; } else { @@ -269,7 +268,7 @@ function checkDepsAreArrayDev(deps: mixed) { } } -function warnOnHookMismatchInDev(currentHookName: HookType) { +function warnOnHookMismatchInDev(currentHookName: ?HookType) { if (__DEV__) { const componentName = getComponentNameFromFiber(currentlyRenderingFiber); if (!didWarnAboutMismatchedHooksForComponent.has(componentName)) { @@ -280,12 +279,18 @@ function warnOnHookMismatchInDev(currentHookName: HookType) { const secondColumnStart = 30; - for (let i = 0; i <= ((hookTypesUpdateIndexDev: any): number); i++) { + const endIndex = + hookTypesUpdateIndexDev < 0 + ? hookTypesDev.length - 1 + : hookTypesUpdateIndexDev; + for (let i = 0; i <= endIndex; i++) { const oldHookName = hookTypesDev[i]; const newHookName = - i === ((hookTypesUpdateIndexDev: any): number) + (hookTypesUpdateIndexDev < 0 + ? null + : i === ((hookTypesUpdateIndexDev: any): number) ? currentHookName - : oldHookName; + : oldHookName) ?? undefined; let row = `${i + 1}. ${oldHookName}`; @@ -416,7 +421,10 @@ export function renderWithHooks( // Non-stateful hooks (e.g. context) don't get added to memoizedState, // so memoizedState would be null during updates and mounts. if (__DEV__) { - if (current !== null && current.memoizedState !== null) { + if ( + current !== null && + (current.mode & ConcurrentMode || current.memoizedState !== null) + ) { ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV; } else if (hookTypesDev !== null) { // This dispatcher handles an edge case where a component is updating, @@ -430,7 +438,9 @@ export function renderWithHooks( } } else { ReactCurrentDispatcher.current = - current === null || current.memoizedState === null + current === null || + ((current.mode & ConcurrentMode) === NoMode && + current.memoizedState === null) ? HooksDispatcherOnMount : HooksDispatcherOnUpdate; } @@ -485,7 +495,14 @@ export function renderWithHooks( ReactCurrentDispatcher.current = ContextOnlyDispatcher; if (__DEV__) { - workInProgress._debugHookTypes = hookTypesDev; + workInProgress._debugHookTypes = hookTypesDev ?? []; + + if (hookTypesDev !== null) { + if (currentHookNameInDev == null && hookTypesDev.length) { + // Renders no hook from some hooks + warnOnHookMismatchInDev(null); + } + } } // This check uses currentHook so that it works the same in DEV and prod bundles. diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index bacf9da7029a..b815f54aae50 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3847,6 +3847,39 @@ describe('ReactHooksWithNoopRenderer', () => { // expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 4')]); }); + it('mount first state', () => { + function App(props) { + let A; + if (props.loadA) { + const [_A] = useState(0); + A = _A; + } else { + A = '[not loaded]'; + } + + return ; + } + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield(['A: [not loaded]']); + expect(ReactNoop.getChildren()).toEqual([span('A: [not loaded]')]); + + ReactNoop.render(); + expect(() => { + expect(() => { + expect(Scheduler).toFlushAndYield(['A: 0']); + }).toThrow('Rendered more hooks than during the previous render'); + }).toErrorDev([ + 'Warning: React has detected a change in the order of Hooks called by App. ' + + 'This will lead to bugs and errors if not fixed. For more information, ' + + 'read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '1. undefined useState\n' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', + ]); + }); + it('unmount state', () => { let updateA; let updateB; @@ -3887,7 +3920,106 @@ describe('ReactHooksWithNoopRenderer', () => { ); }); - it('unmount effects', () => { + it('unmount last state', () => { + function App(props) { + let A; + if (props.loadA) { + const [_A] = useState(0); + A = _A; + } else { + A = '[not loaded]'; + } + + return ; + } + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield(['A: 0']); + expect(ReactNoop.getChildren()).toEqual([span('A: 0')]); + ReactNoop.render(); + expect(() => { + expect(Scheduler).toFlushAndYield(['A: [not loaded]']); + }).toErrorDev([ + 'Warning: React has detected a change in the order of Hooks called by App. ' + + 'This will lead to bugs and errors if not fixed. For more information, ' + + 'read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '1. useState undefined\n' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', + ]); + }); + + it('mount effect', () => { + function App(props) { + if (props.showMore) { + useEffect(() => { + Scheduler.unstable_yieldValue('Mount A'); + return () => { + Scheduler.unstable_yieldValue('Unmount A'); + }; + }, []); + } + + return null; + } + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([]); + + act(() => { + ReactNoop.render(); + expect(() => { + expect(() => { + expect(Scheduler).toFlushAndYield(['Mount A']); + }).toThrow('Rendered more hooks than during the previous render'); + }).toErrorDev([ + 'Warning: React has detected a change in the order of Hooks called by App. ' + + 'This will lead to bugs and errors if not fixed. For more information, ' + + 'read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '1. undefined useEffect\n' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', + ]); + }); + }); + + it('unmount effect', () => { + function App(props) { + if (props.showMore) { + useEffect(() => { + Scheduler.unstable_yieldValue('Mount A'); + return () => { + Scheduler.unstable_yieldValue('Unmount A'); + }; + }, []); + } + + return null; + } + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield(['Mount A']); + ReactNoop.render(); + expect(() => { + // We don't throw because it would require an additional prod check. + expect(Scheduler).not.toFlushAndThrow( + 'Rendered fewer hooks than expected. This may be caused by an ' + + 'accidental early return statement.', + ); + }).toErrorDev([ + 'Warning: React has detected a change in the order of Hooks called by App. ' + + 'This will lead to bugs and errors if not fixed. For more information, ' + + 'read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '1. useEffect undefined\n' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', + ]); + }); + + it('unmount additional effects', () => { function App(props) { useEffect(() => { Scheduler.unstable_yieldValue('Mount A'); From 9550dbd2feee4cae26137cecd9f32c8547261a2c Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Fri, 9 Sep 2022 14:20:35 -0700 Subject: [PATCH 2/4] Add a feature flag --- .../src/ReactFiberHooks.new.js | 18 +++++++++++------- .../src/ReactFiberHooks.old.js | 18 +++++++++++------- .../ReactHooksWithNoopRenderer-test.js | 18 ++++++++++++++---- packages/shared/ReactFeatureFlags.js | 4 ++++ .../forks/ReactFeatureFlags.native-fb.js | 2 ++ .../forks/ReactFeatureFlags.native-oss.js | 2 ++ .../forks/ReactFeatureFlags.test-renderer.js | 2 ++ .../ReactFeatureFlags.test-renderer.www.js | 2 ++ .../shared/forks/ReactFeatureFlags.testing.js | 2 ++ .../forks/ReactFeatureFlags.testing.www.js | 3 +++ .../forks/ReactFeatureFlags.www-dynamic.js | 1 + packages/shared/forks/ReactFeatureFlags.www.js | 1 + 12 files changed, 55 insertions(+), 18 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 3f3845948b7d..3263937a24ae 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -22,6 +22,7 @@ import type {HookFlags} from './ReactHookEffectTags'; import type {FiberRoot} from './ReactInternalTypes'; import type {Cache} from './ReactFiberCacheComponent.new'; import type {Flags} from './ReactFiberFlags'; +import {enableThrowOnMountForHookMismatch} from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import { @@ -290,7 +291,7 @@ function warnOnHookMismatchInDev(currentHookName: ?HookType) { ? null : i === ((hookTypesUpdateIndexDev: any): number) ? currentHookName - : oldHookName) ?? undefined; + : oldHookName) ?? 'undefined'; let row = `${i + 1}. ${oldHookName}`; @@ -423,7 +424,9 @@ export function renderWithHooks( if (__DEV__) { if ( current !== null && - (current.mode & ConcurrentMode || current.memoizedState !== null) + ((enableThrowOnMountForHookMismatch && + (current.mode & ConcurrentMode) !== NoMode) || + current.memoizedState !== null) ) { ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV; } else if (hookTypesDev !== null) { @@ -438,11 +441,12 @@ export function renderWithHooks( } } else { ReactCurrentDispatcher.current = - current === null || - ((current.mode & ConcurrentMode) === NoMode && - current.memoizedState === null) - ? HooksDispatcherOnMount - : HooksDispatcherOnUpdate; + current !== null && + ((enableThrowOnMountForHookMismatch && + (current.mode & ConcurrentMode) !== NoMode) || + current.memoizedState !== null) + ? HooksDispatcherOnUpdate + : HooksDispatcherOnMount; } let children = Component(props, secondArg); diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index b930bad8d715..7a423764418a 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -22,6 +22,7 @@ import type {HookFlags} from './ReactHookEffectTags'; import type {FiberRoot} from './ReactInternalTypes'; import type {Cache} from './ReactFiberCacheComponent.old'; import type {Flags} from './ReactFiberFlags'; +import {enableThrowOnMountForHookMismatch} from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import { @@ -290,7 +291,7 @@ function warnOnHookMismatchInDev(currentHookName: ?HookType) { ? null : i === ((hookTypesUpdateIndexDev: any): number) ? currentHookName - : oldHookName) ?? undefined; + : oldHookName) ?? 'undefined'; let row = `${i + 1}. ${oldHookName}`; @@ -423,7 +424,9 @@ export function renderWithHooks( if (__DEV__) { if ( current !== null && - (current.mode & ConcurrentMode || current.memoizedState !== null) + ((enableThrowOnMountForHookMismatch && + (current.mode & ConcurrentMode) !== NoMode) || + current.memoizedState !== null) ) { ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV; } else if (hookTypesDev !== null) { @@ -438,11 +441,12 @@ export function renderWithHooks( } } else { ReactCurrentDispatcher.current = - current === null || - ((current.mode & ConcurrentMode) === NoMode && - current.memoizedState === null) - ? HooksDispatcherOnMount - : HooksDispatcherOnUpdate; + current !== null && + ((enableThrowOnMountForHookMismatch && + (current.mode & ConcurrentMode) !== NoMode) || + current.memoizedState !== null) + ? HooksDispatcherOnUpdate + : HooksDispatcherOnMount; } let children = Component(props, secondArg); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index b815f54aae50..0e7a6e927fa9 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3866,9 +3866,14 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); expect(() => { - expect(() => { + const expectFlush = expect(() => { expect(Scheduler).toFlushAndYield(['A: 0']); - }).toThrow('Rendered more hooks than during the previous render'); + }); + if (gate(flag => flag.enableThrowOnMountForHookMismatch)) { + expectFlush.toThrow( + 'Rendered more hooks than during the previous render', + ); + } }).toErrorDev([ 'Warning: React has detected a change in the order of Hooks called by App. ' + 'This will lead to bugs and errors if not fixed. For more information, ' + @@ -3970,9 +3975,14 @@ describe('ReactHooksWithNoopRenderer', () => { act(() => { ReactNoop.render(); expect(() => { - expect(() => { + const expectFlush = expect(() => { expect(Scheduler).toFlushAndYield(['Mount A']); - }).toThrow('Rendered more hooks than during the previous render'); + }); + if (gate(flags => flags.enableThrowOnMountForHookMismatch)) { + expectFlush.toThrow( + 'Rendered more hooks than during the previous render', + ); + } }).toErrorDev([ 'Warning: React has detected a change in the order of Hooks called by App. ' + 'This will lead to bugs and errors if not fixed. For more information, ' + diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index ba7fcb61a613..0cd6585d7034 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -258,3 +258,7 @@ export const enableGetInspectorDataForInstanceInProduction = false; export const enableProfilerNestedUpdateScheduledHook = false; export const consoleManagedByDevToolsDuringStrictMode = true; + +// When going between 0 to n hooks in concurrent mode, throw "Rendered more/fewer" hooks than expected. +// This may be an invasive change so we're rolling it out slower. +export const enableThrowOnMountForHookMismatch = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 76f19551875b..71afbe755718 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -83,6 +83,8 @@ export const enableUseMutableSource = true; export const enableTransitionTracing = false; export const enableFloat = false; + +export const enableThrowOnMountForHookMismatch = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 6de6388896c2..7dddf3f2eeba 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -72,6 +72,8 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableFloat = false; + +export const enableThrowOnMountForHookMismatch = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 1ac2ff4f2acf..b0805162120d 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -72,6 +72,8 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableFloat = false; + +export const enableThrowOnMountForHookMismatch = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index e414784563f3..6e2ca1ae631f 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -74,6 +74,8 @@ export const enableUseMutableSource = true; export const enableTransitionTracing = false; export const enableFloat = false; + +export const enableThrowOnMountForHookMismatch = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 0d719ca3523d..31fb8e5465ae 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -72,6 +72,8 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableFloat = false; + +export const enableThrowOnMountForHookMismatch = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 6d2cd32d9f86..17e52f4d0353 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -73,6 +73,9 @@ export const enableUseMutableSource = true; export const enableTransitionTracing = false; export const enableFloat = false; + +export const enableThrowOnMountForHookMismatch = false; + // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 520b102b7d3f..839444de770e 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -28,6 +28,7 @@ export const consoleManagedByDevToolsDuringStrictMode = __VARIANT__; export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = __VARIANT__; export const enableClientRenderFallbackOnTextMismatch = __VARIANT__; export const enableTransitionTracing = __VARIANT__; +export const enableThrowOnMountForHookMismatch = __VARIANT__; // 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 2c59dde3a11b..444566fac4e3 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -34,6 +34,7 @@ export const { enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay, enableClientRenderFallbackOnTextMismatch, enableTransitionTracing, + enableThrowOnMountForHookMismatch, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build. From 606adfee898d928d757b7e3f15e24a53abc6a053 Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Fri, 9 Sep 2022 14:55:31 -0700 Subject: [PATCH 3/4] fix test --- .../ReactHooksWithNoopRenderer-test.js | 55 +++++++++++-------- .../ReactFeatureFlags.test-renderer.native.js | 2 + 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 0e7a6e927fa9..30017d35c9fe 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3866,13 +3866,12 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); expect(() => { - const expectFlush = expect(() => { - expect(Scheduler).toFlushAndYield(['A: 0']); - }); if (gate(flag => flag.enableThrowOnMountForHookMismatch)) { - expectFlush.toThrow( - 'Rendered more hooks than during the previous render', - ); + expect(() => { + expect(Scheduler).toFlushAndYield(['A: 0']); + }).toThrow('Rendered more hooks than during the previous render'); + } else { + expect(Scheduler).toFlushAndYield(['A: 0']); } }).toErrorDev([ 'Warning: React has detected a change in the order of Hooks called by App. ' + @@ -3974,24 +3973,34 @@ describe('ReactHooksWithNoopRenderer', () => { act(() => { ReactNoop.render(); - expect(() => { - const expectFlush = expect(() => { + if (gate(flags => flags.enableThrowOnMountForHookMismatch)) { + expect(() => { + expect(() => { + expect(Scheduler).toFlushAndYield(['Mount A']); + }).toThrow('Rendered more hooks than during the previous render'); + }).toErrorDev([ + 'Warning: React has detected a change in the order of Hooks called by App. ' + + 'This will lead to bugs and errors if not fixed. For more information, ' + + 'read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '1. undefined useEffect\n' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', + ]); + } else { + expect(() => { expect(Scheduler).toFlushAndYield(['Mount A']); - }); - if (gate(flags => flags.enableThrowOnMountForHookMismatch)) { - expectFlush.toThrow( - 'Rendered more hooks than during the previous render', - ); - } - }).toErrorDev([ - 'Warning: React has detected a change in the order of Hooks called by App. ' + - 'This will lead to bugs and errors if not fixed. For more information, ' + - 'read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks\n\n' + - ' Previous render Next render\n' + - ' ------------------------------------------------------\n' + - '1. undefined useEffect\n' + - ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', - ]); + }).toErrorDev([ + 'Warning: Internal React error: Expected static flag was missing. Please notify the React team.', + 'Warning: React has detected a change in the order of Hooks called by App. ' + + 'This will lead to bugs and errors if not fixed. For more information, ' + + 'read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '1. undefined useEffect\n' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', + ]); + } }); }); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index 8b3d4e230d43..fe357a9077b3 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -70,6 +70,8 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableFloat = false; + +export const enableThrowOnMountForHookMismatch = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; From e5440135f090aa5e0a185c7de085ef63f48213d2 Mon Sep 17 00:00:00 2001 From: Tianyu Yao Date: Fri, 16 Sep 2022 17:07:50 -0700 Subject: [PATCH 4/4] yarn lint-build --- packages/react-reconciler/src/ReactFiberHooks.new.js | 4 ++-- packages/react-reconciler/src/ReactFiberHooks.old.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 3263937a24ae..875714b288eb 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -291,7 +291,7 @@ function warnOnHookMismatchInDev(currentHookName: ?HookType) { ? null : i === ((hookTypesUpdateIndexDev: any): number) ? currentHookName - : oldHookName) ?? 'undefined'; + : oldHookName) || 'undefined'; let row = `${i + 1}. ${oldHookName}`; @@ -499,7 +499,7 @@ export function renderWithHooks( ReactCurrentDispatcher.current = ContextOnlyDispatcher; if (__DEV__) { - workInProgress._debugHookTypes = hookTypesDev ?? []; + workInProgress._debugHookTypes = hookTypesDev || []; if (hookTypesDev !== null) { if (currentHookNameInDev == null && hookTypesDev.length) { diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 7a423764418a..6879d275970d 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -291,7 +291,7 @@ function warnOnHookMismatchInDev(currentHookName: ?HookType) { ? null : i === ((hookTypesUpdateIndexDev: any): number) ? currentHookName - : oldHookName) ?? 'undefined'; + : oldHookName) || 'undefined'; let row = `${i + 1}. ${oldHookName}`; @@ -499,7 +499,7 @@ export function renderWithHooks( ReactCurrentDispatcher.current = ContextOnlyDispatcher; if (__DEV__) { - workInProgress._debugHookTypes = hookTypesDev ?? []; + workInProgress._debugHookTypes = hookTypesDev || []; if (hookTypesDev !== null) { if (currentHookNameInDev == null && hookTypesDev.length) {