diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js index 3c7972aff4df8..8c56c8cd775ac 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js @@ -427,4 +427,42 @@ describe('ReactHooksInspectionIntegration', () => { expect(setterCalls[0]).not.toBe(initial); expect(setterCalls[1]).toBe(initial); }); + + // This test case is based on an open source bug report: + // facebookincubator/redux-react-hook/issues/34#issuecomment-466693787 + it('should properly advance the current hook for useContext', () => { + const MyContext = React.createContext(1); + + let incrementCount; + + function Foo(props) { + const context = React.useContext(MyContext); + const [data, setData] = React.useState({count: context}); + + incrementCount = () => setData(({count}) => ({count: count + 1})); + + return
count: {data.count}
; + } + + const renderer = ReactTestRenderer.create(); + expect(renderer.toJSON()).toEqual({ + type: 'div', + props: {}, + children: ['count: ', '1'], + }); + + act(incrementCount); + expect(renderer.toJSON()).toEqual({ + type: 'div', + props: {}, + children: ['count: ', '2'], + }); + + const childFiber = renderer.root._currentFiber(); + const tree = ReactDebugTools.inspectHooksOfFiber(childFiber); + expect(tree).toEqual([ + {name: 'Context', value: 1, subHooks: []}, + {name: 'State', value: {count: 2}, subHooks: []}, + ]); + }); }); diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 681e3e419e135..08bff4899e9ed 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -15,6 +15,7 @@ import type {SideEffectTag} from 'shared/ReactSideEffectTags'; import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {UpdateQueue} from './ReactUpdateQueue'; import type {ContextDependencyList} from './ReactFiberNewContext'; +import type {HookType} from './ReactFiberHooks'; import invariant from 'shared/invariant'; import warningWithoutStack from 'shared/warningWithoutStack'; @@ -204,6 +205,9 @@ export type Fiber = {| _debugSource?: Source | null, _debugOwner?: Fiber | null, _debugIsCurrentlyTiming?: boolean, + + // Used to verify that the order of hooks does not change between renders. + _debugHookTypes?: Array | null, |}; let debugCounter; @@ -285,6 +289,7 @@ function FiberNode( this._debugSource = null; this._debugOwner = null; this._debugIsCurrentlyTiming = false; + this._debugHookTypes = null; if (!hasBadMapPolyfill && typeof Object.preventExtensions === 'function') { Object.preventExtensions(this); } @@ -370,6 +375,7 @@ export function createWorkInProgress( workInProgress._debugID = current._debugID; workInProgress._debugSource = current._debugSource; workInProgress._debugOwner = current._debugOwner; + workInProgress._debugHookTypes = current._debugHookTypes; } workInProgress.alternate = current; @@ -723,5 +729,6 @@ export function assignFiberPropertiesInDEV( target._debugSource = source._debugSource; target._debugOwner = source._debugOwner; target._debugIsCurrentlyTiming = source._debugIsCurrentlyTiming; + target._debugHookTypes = source._debugHookTypes; return target; } diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 702814f9c449d..9244864b11c2f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -93,7 +93,7 @@ type UpdateQueue = { eagerState: S | null, }; -type HookType = +export type HookType = | 'useState' | 'useReducer' | 'useContext' @@ -120,10 +120,6 @@ export type Hook = { next: Hook | null, }; -type HookDev = Hook & { - _debugType: HookType, -}; - type Effect = { tag: HookEffectTag, create: () => (() => void) | void, @@ -150,7 +146,6 @@ let currentlyRenderingFiber: Fiber | null = null; // current hook list is the list that belongs to the current fiber. The // work-in-progress hook list is a new list that will be added to the // work-in-progress fiber. -let firstCurrentHook: Hook | null = null; let currentHook: Hook | null = null; let nextCurrentHook: Hook | null = null; let firstWorkInProgressHook: Hook | null = null; @@ -183,7 +178,38 @@ const RE_RENDER_LIMIT = 25; // In DEV, this is the name of the currently executing primitive hook let currentHookNameInDev: ?HookType = null; -function warnOnHookMismatchInDev() { +// In DEV, this list ensures that hooks are called in the same order between renders. +// The list stores the order of hooks used during the initial render (mount). +// Subsequent renders (updates) reference this list. +let hookTypesDev: Array | null = null; +let hookTypesUpdateIndexDev: number = -1; + +function mountHookTypesDev() { + if (__DEV__) { + const hookName = ((currentHookNameInDev: any): HookType); + + if (hookTypesDev === null) { + hookTypesDev = [hookName]; + } else { + hookTypesDev.push(hookName); + } + } +} + +function updateHookTypesDev() { + if (__DEV__) { + const hookName = ((currentHookNameInDev: any): HookType); + + if (hookTypesDev !== null) { + hookTypesUpdateIndexDev++; + if (hookTypesDev[hookTypesUpdateIndexDev] !== hookName) { + warnOnHookMismatchInDev(hookName); + } + } + } +} + +function warnOnHookMismatchInDev(currentHookName: HookType) { if (__DEV__) { const componentName = getComponentName( ((currentlyRenderingFiber: any): Fiber).type, @@ -191,44 +217,44 @@ function warnOnHookMismatchInDev() { if (!didWarnAboutMismatchedHooksForComponent.has(componentName)) { didWarnAboutMismatchedHooksForComponent.add(componentName); - const secondColumnStart = 22; + if (hookTypesDev !== null) { + let table = ''; - let table = ''; - let prevHook: HookDev | null = (firstCurrentHook: any); - let nextHook: HookDev | null = (firstWorkInProgressHook: any); - let n = 1; - while (prevHook !== null && nextHook !== null) { - const oldHookName = prevHook._debugType; - const newHookName = nextHook._debugType; + const secondColumnStart = 30; - let row = `${n}. ${oldHookName}`; + for (let i = 0; i <= ((hookTypesUpdateIndexDev: any): number); i++) { + const oldHookName = hookTypesDev[i]; + const newHookName = + i === ((hookTypesUpdateIndexDev: any): number) + ? currentHookName + : oldHookName; - // Extra space so second column lines up - // lol @ IE not supporting String#repeat - while (row.length < secondColumnStart) { - row += ' '; - } + let row = `${i + 1}. ${oldHookName}`; - row += newHookName + '\n'; + // Extra space so second column lines up + // lol @ IE not supporting String#repeat + while (row.length < secondColumnStart) { + row += ' '; + } - table += row; - prevHook = (prevHook.next: any); - nextHook = (nextHook.next: any); - n++; - } + row += newHookName + '\n'; - warning( - false, - 'React has detected a change in the order of Hooks called by %s. ' + - 'This will lead to bugs and errors if not fixed. ' + - 'For more information, read the Rules of Hooks: https://fb.me/rules-of-hooks\n\n' + - ' Previous render Next render\n' + - ' -------------------------------\n' + - '%s' + - ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n', - componentName, - table, - ); + table += row; + } + + warning( + false, + 'React has detected a change in the order of Hooks called by %s. ' + + 'This will lead to bugs and errors if not fixed. ' + + 'For more information, read the Rules of Hooks: https://fb.me/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '%s' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n', + componentName, + table, + ); + } } } } @@ -293,8 +319,15 @@ export function renderWithHooks( ): any { renderExpirationTime = nextRenderExpirationTime; currentlyRenderingFiber = workInProgress; - firstCurrentHook = nextCurrentHook = - current !== null ? current.memoizedState : null; + nextCurrentHook = current !== null ? current.memoizedState : null; + + if (__DEV__) { + hookTypesDev = + current !== null + ? ((current._debugHookTypes: any): Array) + : null; + hookTypesUpdateIndexDev = -1; + } // The following should have already been reset // currentHook = null; @@ -308,11 +341,26 @@ export function renderWithHooks( // numberOfReRenders = 0; // sideEffectTag = 0; + // TODO Warn if no hooks are used at all during mount, then some are used during update. + // Currently we will identify the update render as a mount because nextCurrentHook === null. + // This is tricky because it's valid for certain types of components (e.g. React.lazy) + + // Using nextCurrentHook to differentiate between mount/update only works if at least one stateful hook is used. + // Non-stateful hooks (e.g. context) don't get added to memoizedState, + // so nextCurrentHook would be null during updates and mounts. if (__DEV__) { - ReactCurrentDispatcher.current = - nextCurrentHook === null - ? HooksDispatcherOnMountInDEV - : HooksDispatcherOnUpdateInDEV; + if (nextCurrentHook !== null) { + ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV; + } else if (hookTypesDev !== null) { + // This dispatcher handles an edge case where a component is updating, + // but no stateful hooks have been used. + // We want to match the production code behavior (which will use HooksDispatcherOnMount), + // but with the extra DEV validation to ensure hooks ordering hasn't changed. + // This dispatcher does that. + ReactCurrentDispatcher.current = HooksDispatcherOnMountWithHookTypesInDEV; + } else { + ReactCurrentDispatcher.current = HooksDispatcherOnMountInDEV; + } } else { ReactCurrentDispatcher.current = nextCurrentHook === null @@ -328,14 +376,18 @@ export function renderWithHooks( numberOfReRenders += 1; // Start over from the beginning of the list - firstCurrentHook = nextCurrentHook = - current !== null ? current.memoizedState : null; + nextCurrentHook = current !== null ? current.memoizedState : null; nextWorkInProgressHook = firstWorkInProgressHook; currentHook = null; workInProgressHook = null; componentUpdateQueue = null; + if (__DEV__) { + // Also validate hook order for cascading updates. + hookTypesUpdateIndexDev = -1; + } + ReactCurrentDispatcher.current = __DEV__ ? HooksDispatcherOnUpdateInDEV : HooksDispatcherOnUpdate; @@ -347,10 +399,6 @@ export function renderWithHooks( numberOfReRenders = 0; } - if (__DEV__) { - currentHookNameInDev = null; - } - // We can assume the previous dispatcher is always this one, since we set it // at the beginning of the render phase and there's no re-entrancy. ReactCurrentDispatcher.current = ContextOnlyDispatcher; @@ -362,19 +410,30 @@ export function renderWithHooks( renderedWork.updateQueue = (componentUpdateQueue: any); renderedWork.effectTag |= sideEffectTag; + if (__DEV__) { + renderedWork._debugHookTypes = hookTypesDev; + } + + // This check uses currentHook so that it works the same in DEV and prod bundles. + // hookTypesDev could catch more cases (e.g. context) but only in DEV bundles. const didRenderTooFewHooks = currentHook !== null && currentHook.next !== null; renderExpirationTime = NoWork; currentlyRenderingFiber = null; - firstCurrentHook = null; currentHook = null; nextCurrentHook = null; firstWorkInProgressHook = null; workInProgressHook = null; nextWorkInProgressHook = null; + if (__DEV__) { + currentHookNameInDev = null; + hookTypesDev = null; + hookTypesUpdateIndexDev = -1; + } + remainingExpirationTime = NoWork; componentUpdateQueue = null; sideEffectTag = 0; @@ -416,21 +475,23 @@ export function resetHooks(): void { renderExpirationTime = NoWork; currentlyRenderingFiber = null; - firstCurrentHook = null; currentHook = null; nextCurrentHook = null; firstWorkInProgressHook = null; workInProgressHook = null; nextWorkInProgressHook = null; - remainingExpirationTime = NoWork; - componentUpdateQueue = null; - sideEffectTag = 0; - if (__DEV__) { + hookTypesDev = null; + hookTypesUpdateIndexDev = -1; + currentHookNameInDev = null; } + remainingExpirationTime = NoWork; + componentUpdateQueue = null; + sideEffectTag = 0; + didScheduleRenderPhaseUpdate = false; renderPhaseUpdates = null; numberOfReRenders = 0; @@ -447,9 +508,6 @@ function mountWorkInProgressHook(): Hook { next: null, }; - if (__DEV__) { - (hook: any)._debugType = (currentHookNameInDev: any); - } if (workInProgressHook === null) { // This is the first hook in the list firstWorkInProgressHook = workInProgressHook = hook; @@ -499,13 +557,6 @@ function updateWorkInProgressHook(): Hook { workInProgressHook = workInProgressHook.next = newHook; } nextCurrentHook = currentHook.next; - - if (__DEV__) { - (newHook: any)._debugType = (currentHookNameInDev: any); - if (currentHookNameInDev !== ((currentHook: any): HookDev)._debugType) { - warnOnHookMismatchInDev(); - } - } } return workInProgressHook; } @@ -520,26 +571,6 @@ function basicStateReducer(state: S, action: BasicStateAction): S { return typeof action === 'function' ? action(state) : action; } -function mountContext( - context: ReactContext, - observedBits: void | number | boolean, -): T { - if (__DEV__) { - mountWorkInProgressHook(); - } - return readContext(context, observedBits); -} - -function updateContext( - context: ReactContext, - observedBits: void | number | boolean, -): T { - if (__DEV__) { - updateWorkInProgressHook(); - } - return readContext(context, observedBits); -} - function mountReducer( reducer: (S, A) => S, initialArg: I, @@ -1181,6 +1212,7 @@ const HooksDispatcherOnUpdate: Dispatcher = { }; let HooksDispatcherOnMountInDEV: Dispatcher | null = null; +let HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher | null = null; let HooksDispatcherOnUpdateInDEV: Dispatcher | null = null; let InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher | null = null; let InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher | null = null; @@ -1216,6 +1248,104 @@ if (__DEV__) { useCallback(callback: T, deps: Array | void | null): T { currentHookNameInDev = 'useCallback'; + mountHookTypesDev(); + return mountCallback(callback, deps); + }, + useContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + currentHookNameInDev = 'useContext'; + mountHookTypesDev(); + return readContext(context, observedBits); + }, + useEffect( + create: () => (() => void) | void, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useEffect'; + mountHookTypesDev(); + return mountEffect(create, deps); + }, + useImperativeHandle( + ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, + create: () => T, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useImperativeHandle'; + mountHookTypesDev(); + return mountImperativeHandle(ref, create, deps); + }, + useLayoutEffect( + create: () => (() => void) | void, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useLayoutEffect'; + mountHookTypesDev(); + return mountLayoutEffect(create, deps); + }, + useMemo(create: () => T, deps: Array | void | null): T { + currentHookNameInDev = 'useMemo'; + mountHookTypesDev(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; + try { + return mountMemo(create, deps); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useReducer( + reducer: (S, A) => S, + initialArg: I, + init?: I => S, + ): [S, Dispatch] { + currentHookNameInDev = 'useReducer'; + mountHookTypesDev(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; + try { + return mountReducer(reducer, initialArg, init); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useRef(initialValue: T): {current: T} { + currentHookNameInDev = 'useRef'; + mountHookTypesDev(); + return mountRef(initialValue); + }, + useState( + initialState: (() => S) | S, + ): [S, Dispatch>] { + currentHookNameInDev = 'useState'; + mountHookTypesDev(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; + try { + return mountState(initialState); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { + currentHookNameInDev = 'useDebugValue'; + mountHookTypesDev(); + return mountDebugValue(value, formatterFn); + }, + }; + + HooksDispatcherOnMountWithHookTypesInDEV = { + readContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + return readContext(context, observedBits); + }, + + useCallback(callback: T, deps: Array | void | null): T { + currentHookNameInDev = 'useCallback'; + updateHookTypesDev(); return mountCallback(callback, deps); }, useContext( @@ -1223,13 +1353,15 @@ if (__DEV__) { observedBits: void | number | boolean, ): T { currentHookNameInDev = 'useContext'; - return mountContext(context, observedBits); + updateHookTypesDev(); + return readContext(context, observedBits); }, useEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { currentHookNameInDev = 'useEffect'; + updateHookTypesDev(); return mountEffect(create, deps); }, useImperativeHandle( @@ -1238,6 +1370,7 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useImperativeHandle'; + updateHookTypesDev(); return mountImperativeHandle(ref, create, deps); }, useLayoutEffect( @@ -1245,10 +1378,12 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useLayoutEffect'; + updateHookTypesDev(); return mountLayoutEffect(create, deps); }, useMemo(create: () => T, deps: Array | void | null): T { currentHookNameInDev = 'useMemo'; + updateHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1263,6 +1398,7 @@ if (__DEV__) { init?: I => S, ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; + updateHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1273,12 +1409,14 @@ if (__DEV__) { }, useRef(initialValue: T): {current: T} { currentHookNameInDev = 'useRef'; + updateHookTypesDev(); return mountRef(initialValue); }, useState( initialState: (() => S) | S, ): [S, Dispatch>] { currentHookNameInDev = 'useState'; + updateHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1289,6 +1427,7 @@ if (__DEV__) { }, useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { currentHookNameInDev = 'useDebugValue'; + updateHookTypesDev(); return mountDebugValue(value, formatterFn); }, }; @@ -1303,6 +1442,7 @@ if (__DEV__) { useCallback(callback: T, deps: Array | void | null): T { currentHookNameInDev = 'useCallback'; + updateHookTypesDev(); return updateCallback(callback, deps); }, useContext( @@ -1310,13 +1450,15 @@ if (__DEV__) { observedBits: void | number | boolean, ): T { currentHookNameInDev = 'useContext'; - return updateContext(context, observedBits); + updateHookTypesDev(); + return readContext(context, observedBits); }, useEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { currentHookNameInDev = 'useEffect'; + updateHookTypesDev(); return updateEffect(create, deps); }, useImperativeHandle( @@ -1325,6 +1467,7 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useImperativeHandle'; + updateHookTypesDev(); return updateImperativeHandle(ref, create, deps); }, useLayoutEffect( @@ -1332,10 +1475,12 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useLayoutEffect'; + updateHookTypesDev(); return updateLayoutEffect(create, deps); }, useMemo(create: () => T, deps: Array | void | null): T { currentHookNameInDev = 'useMemo'; + updateHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1350,6 +1495,7 @@ if (__DEV__) { init?: I => S, ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; + updateHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1360,12 +1506,14 @@ if (__DEV__) { }, useRef(initialValue: T): {current: T} { currentHookNameInDev = 'useRef'; + updateHookTypesDev(); return updateRef(initialValue); }, useState( initialState: (() => S) | S, ): [S, Dispatch>] { currentHookNameInDev = 'useState'; + updateHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1376,6 +1524,7 @@ if (__DEV__) { }, useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { currentHookNameInDev = 'useDebugValue'; + updateHookTypesDev(); return updateDebugValue(value, formatterFn); }, }; @@ -1392,6 +1541,7 @@ if (__DEV__) { useCallback(callback: T, deps: Array | void | null): T { currentHookNameInDev = 'useCallback'; warnInvalidHookAccess(); + mountHookTypesDev(); return mountCallback(callback, deps); }, useContext( @@ -1400,7 +1550,8 @@ if (__DEV__) { ): T { currentHookNameInDev = 'useContext'; warnInvalidHookAccess(); - return mountContext(context, observedBits); + mountHookTypesDev(); + return readContext(context, observedBits); }, useEffect( create: () => (() => void) | void, @@ -1408,6 +1559,7 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useEffect'; warnInvalidHookAccess(); + mountHookTypesDev(); return mountEffect(create, deps); }, useImperativeHandle( @@ -1417,6 +1569,7 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useImperativeHandle'; warnInvalidHookAccess(); + mountHookTypesDev(); return mountImperativeHandle(ref, create, deps); }, useLayoutEffect( @@ -1425,11 +1578,13 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useLayoutEffect'; warnInvalidHookAccess(); + mountHookTypesDev(); return mountLayoutEffect(create, deps); }, useMemo(create: () => T, deps: Array | void | null): T { currentHookNameInDev = 'useMemo'; warnInvalidHookAccess(); + mountHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1445,6 +1600,7 @@ if (__DEV__) { ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; warnInvalidHookAccess(); + mountHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1456,6 +1612,7 @@ if (__DEV__) { useRef(initialValue: T): {current: T} { currentHookNameInDev = 'useRef'; warnInvalidHookAccess(); + mountHookTypesDev(); return mountRef(initialValue); }, useState( @@ -1463,6 +1620,7 @@ if (__DEV__) { ): [S, Dispatch>] { currentHookNameInDev = 'useState'; warnInvalidHookAccess(); + mountHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1474,6 +1632,7 @@ if (__DEV__) { useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { currentHookNameInDev = 'useDebugValue'; warnInvalidHookAccess(); + mountHookTypesDev(); return mountDebugValue(value, formatterFn); }, }; @@ -1490,6 +1649,7 @@ if (__DEV__) { useCallback(callback: T, deps: Array | void | null): T { currentHookNameInDev = 'useCallback'; warnInvalidHookAccess(); + updateHookTypesDev(); return updateCallback(callback, deps); }, useContext( @@ -1498,7 +1658,8 @@ if (__DEV__) { ): T { currentHookNameInDev = 'useContext'; warnInvalidHookAccess(); - return updateContext(context, observedBits); + updateHookTypesDev(); + return readContext(context, observedBits); }, useEffect( create: () => (() => void) | void, @@ -1506,6 +1667,7 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useEffect'; warnInvalidHookAccess(); + updateHookTypesDev(); return updateEffect(create, deps); }, useImperativeHandle( @@ -1515,6 +1677,7 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useImperativeHandle'; warnInvalidHookAccess(); + updateHookTypesDev(); return updateImperativeHandle(ref, create, deps); }, useLayoutEffect( @@ -1523,11 +1686,13 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useLayoutEffect'; warnInvalidHookAccess(); + updateHookTypesDev(); return updateLayoutEffect(create, deps); }, useMemo(create: () => T, deps: Array | void | null): T { currentHookNameInDev = 'useMemo'; warnInvalidHookAccess(); + updateHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1543,6 +1708,7 @@ if (__DEV__) { ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; warnInvalidHookAccess(); + updateHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1554,6 +1720,7 @@ if (__DEV__) { useRef(initialValue: T): {current: T} { currentHookNameInDev = 'useRef'; warnInvalidHookAccess(); + updateHookTypesDev(); return updateRef(initialValue); }, useState( @@ -1561,6 +1728,7 @@ if (__DEV__) { ): [S, Dispatch>] { currentHookNameInDev = 'useState'; warnInvalidHookAccess(); + updateHookTypesDev(); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1572,6 +1740,7 @@ if (__DEV__) { useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { currentHookNameInDev = 'useDebugValue'; warnInvalidHookAccess(); + updateHookTypesDev(); return updateDebugValue(value, formatterFn); }, }; diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index b7094beccd64c..8a3e7d98a86f7 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -984,8 +984,6 @@ describe('ReactHooks', () => { it('warns when calling hooks inside useReducer', () => { const {useReducer, useState, useRef} = React; - spyOnDev(console, 'error'); - function App() { const [value, dispatch] = useReducer((state, action) => { useRef(0); @@ -997,16 +995,23 @@ describe('ReactHooks', () => { useState(); return value; } - expect(() => { - ReactTestRenderer.create(); - }).toThrow('Rendered more hooks than during the previous render.'); - if (__DEV__) { - expect(console.error).toHaveBeenCalledTimes(3); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks', - ); - } + expect(() => { + expect(() => { + ReactTestRenderer.create(); + }).toThrow('Rendered more hooks than during the previous render.'); + }).toWarnDev([ + 'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks', + 'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks', + '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://fb.me/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '1. useReducer useReducer\n' + + '2. useState useRef\n' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', + ]); }); it("warns when calling hooks inside useState's initialize function", () => { @@ -1337,74 +1342,261 @@ describe('ReactHooks', () => { expect(useMemoCount).toBe(__DEV__ ? 2 : 1); // Has Hooks }); - it('warns on using differently ordered hooks on subsequent renders', () => { - const {useState, useReducer, useRef} = React; - function useCustomHook() { - return useState(0); - } - function App(props) { - /* eslint-disable no-unused-vars */ - if (props.flip) { - useCustomHook(0); - useReducer((s, a) => a, 0); - } else { - useReducer((s, a) => a, 0); - useCustomHook(0); - } - // This should not appear in the warning message because it occurs after - // the first mismatch - const ref = useRef(null); - return null; - /* eslint-enable no-unused-vars */ - } - let root = ReactTestRenderer.create(); - expect(() => { - root.update(); - }).toWarnDev([ - '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://fb.me/rules-of-hooks\n\n' + - ' Previous render Next render\n' + - ' -------------------------------\n' + - '1. useReducer useState\n' + - ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' + - ' in App (at **)', - ]); + describe('hook ordering', () => { + const useCallbackHelper = () => React.useCallback(() => {}, []); + const useContextHelper = () => React.useContext(React.createContext()); + const useDebugValueHelper = () => React.useDebugValue('abc'); + const useEffectHelper = () => React.useEffect(() => () => {}, []); + const useImperativeHandleHelper = () => { + React.useImperativeHandle({current: null}, () => ({}), []); + }; + const useLayoutEffectHelper = () => + React.useLayoutEffect(() => () => {}, []); + const useMemoHelper = () => React.useMemo(() => 123, []); + const useReducerHelper = () => React.useReducer((s, a) => a, 0); + const useRefHelper = () => React.useRef(null); + const useStateHelper = () => React.useState(0); + + // We don't include useImperativeHandleHelper in this set, + // because it generates an additional warning about the inputs length changing. + // We test it below with its own test. + let orderedHooks = [ + useCallbackHelper, + useContextHelper, + useDebugValueHelper, + useEffectHelper, + useLayoutEffectHelper, + useMemoHelper, + useReducerHelper, + useRefHelper, + useStateHelper, + ]; + + const formatHookNamesToMatchErrorMessage = (hookNameA, hookNameB) => { + return `use${hookNameA}${' '.repeat(24 - hookNameA.length)}${ + hookNameB ? `use${hookNameB}` : undefined + }`; + }; - // further warnings for this component are silenced - root.update(); - }); + orderedHooks.forEach((firstHelper, index) => { + const secondHelper = + index > 0 + ? orderedHooks[index - 1] + : orderedHooks[orderedHooks.length - 1]; + + const hookNameA = firstHelper.name + .replace('use', '') + .replace('Helper', ''); + const hookNameB = secondHelper.name + .replace('use', '') + .replace('Helper', ''); + + it(`warns on using differently ordered hooks (${hookNameA}, ${hookNameB}) on subsequent renders`, () => { + function App(props) { + /* eslint-disable no-unused-vars */ + if (props.update) { + secondHelper(); + firstHelper(); + } else { + firstHelper(); + secondHelper(); + } + // This should not appear in the warning message because it occurs after the first mismatch + useRefHelper(); + return null; + /* eslint-enable no-unused-vars */ + } + let root = ReactTestRenderer.create(); + expect(() => { + try { + root.update(); + } catch (error) { + // Swapping certain types of hooks will cause runtime errors. + // This is okay as far as this test is concerned. + // We just want to verify that warnings are always logged. + } + }).toWarnDev([ + '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://fb.me/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + `1. ${formatHookNamesToMatchErrorMessage(hookNameA, hookNameB)}\n` + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' + + ' in App (at **)', + ]); + + // further warnings for this component are silenced + try { + root.update(); + } catch (error) { + // Swapping certain types of hooks will cause runtime errors. + // This is okay as far as this test is concerned. + // We just want to verify that warnings are always logged. + } + }); - it('detects a bad hook order even if the component throws', () => { - const {useState, useReducer} = React; - function useCustomHook() { - useState(0); - } - function App(props) { - /* eslint-disable no-unused-vars */ - if (props.flip) { - useCustomHook(); - useReducer((s, a) => a, 0); - throw new Error('custom error'); - } else { - useReducer((s, a) => a, 0); - useCustomHook(); + it(`warns when more hooks (${(hookNameA, + hookNameB)}) are used during update than mount`, () => { + function App(props) { + /* eslint-disable no-unused-vars */ + if (props.update) { + firstHelper(); + secondHelper(); + } else { + firstHelper(); + } + return null; + /* eslint-enable no-unused-vars */ + } + let root = ReactTestRenderer.create(); + expect(() => { + try { + root.update(); + } catch (error) { + // Swapping certain types of hooks will cause runtime errors. + // This is okay as far as this test is concerned. + // We just want to verify that warnings are always logged. + } + }).toWarnDev([ + '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://fb.me/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + `1. ${formatHookNamesToMatchErrorMessage(hookNameA, hookNameA)}\n` + + `2. undefined use${hookNameB}\n` + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' + + ' in App (at **)', + ]); + }); + }); + + // We don't include useContext or useDebugValue in this set, + // because they aren't added to the hooks list and so won't throw. + let hooksInList = [ + useCallbackHelper, + useEffectHelper, + useImperativeHandleHelper, + useLayoutEffectHelper, + useMemoHelper, + useReducerHelper, + useRefHelper, + useStateHelper, + ]; + + hooksInList.forEach((firstHelper, index) => { + const secondHelper = + index > 0 + ? hooksInList[index - 1] + : hooksInList[hooksInList.length - 1]; + + const hookNameA = firstHelper.name + .replace('use', '') + .replace('Helper', ''); + const hookNameB = secondHelper.name + .replace('use', '') + .replace('Helper', ''); + + it(`warns when fewer hooks (${(hookNameA, + hookNameB)}) are used during update than mount`, () => { + function App(props) { + /* eslint-disable no-unused-vars */ + if (props.update) { + firstHelper(); + } else { + firstHelper(); + secondHelper(); + } + return null; + /* eslint-enable no-unused-vars */ + } + let root = ReactTestRenderer.create(); + expect(() => { + root.update(); + }).toThrow('Rendered fewer hooks than expected.'); + }); + }); + + it( + 'warns on using differently ordered hooks ' + + '(useImperativeHandleHelper, useMemoHelper) on subsequent renders', + () => { + function App(props) { + /* eslint-disable no-unused-vars */ + if (props.update) { + useMemoHelper(); + useImperativeHandleHelper(); + } else { + useImperativeHandleHelper(); + useMemoHelper(); + } + // This should not appear in the warning message because it occurs after the first mismatch + useRefHelper(); + return null; + /* eslint-enable no-unused-vars */ + } + let root = ReactTestRenderer.create(); + expect(() => { + try { + root.update(); + } catch (error) { + // Swapping certain types of hooks will cause runtime errors. + // This is okay as far as this test is concerned. + // We just want to verify that warnings are always logged. + } + }).toWarnDev([ + '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://fb.me/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + `1. ${formatHookNamesToMatchErrorMessage( + 'ImperativeHandle', + 'Memo', + )}\n` + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' + + ' in App (at **)', + ]); + + // further warnings for this component are silenced + root.update(); + }, + ); + + it('detects a bad hook order even if the component throws', () => { + const {useState, useReducer} = React; + function useCustomHook() { + useState(0); } - return null; - /* eslint-enable no-unused-vars */ - } - let root = ReactTestRenderer.create(); - expect(() => { - expect(() => root.update()).toThrow('custom error'); - }).toWarnDev([ - '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://fb.me/rules-of-hooks\n\n' + - ' Previous render Next render\n' + - ' -------------------------------\n' + - '1. useReducer useState\n' + - ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^', - ]); + function App(props) { + /* eslint-disable no-unused-vars */ + if (props.update) { + useCustomHook(); + useReducer((s, a) => a, 0); + throw new Error('custom error'); + } else { + useReducer((s, a) => a, 0); + useCustomHook(); + } + return null; + /* eslint-enable no-unused-vars */ + } + let root = ReactTestRenderer.create(); + expect(() => { + expect(() => root.update()).toThrow( + 'custom error', + ); + }).toWarnDev([ + '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://fb.me/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '1. useReducer useState\n' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', + ]); + }); }); // Regression test for #14674 diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index adca4cbc6de8d..9a1c73a62f291 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -1739,8 +1739,20 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); expect(() => { - expect(ReactNoop).toFlushAndYield(['A: 2, B: 3, C: 0']); - }).toThrow('Rendered more hooks than during the previous render'); + expect(() => { + expect(ReactNoop).toFlushAndYield(['A: 2, B: 3, C: 0']); + }).toThrow('Rendered more hooks than during the previous render'); + }).toWarnDev([ + '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://fb.me/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '1. useState useState\n' + + '2. useState useState\n' + + '3. undefined useState\n' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', + ]); // Uncomment if/when we support this again // expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 0')]); @@ -1818,8 +1830,19 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); expect(() => { - expect(ReactNoop).toFlushAndYield([]); - }).toThrow('Rendered more hooks than during the previous render'); + expect(() => { + expect(ReactNoop).toFlushAndYield([]); + }).toThrow('Rendered more hooks than during the previous render'); + }).toWarnDev([ + '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://fb.me/rules-of-hooks\n\n' + + ' Previous render Next render\n' + + ' ------------------------------------------------------\n' + + '1. useEffect useEffect\n' + + '2. undefined useEffect\n' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n', + ]); // Uncomment if/when we support this again // ReactNoop.flushPassiveEffects();