diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index bec6bc2fe9bc4..2dc1ddb02cac2 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -93,7 +93,6 @@ function useContext( context: ReactContext, observedBits: void | number | boolean, ): T { - nextHook(); hookLog.push({ primitive: 'Context', stackError: new Error(), 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 02f416e808810..86c0b8c37a2b7 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,33 @@ 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 mountHookTypeDev(hookName: HookType) { + if (__DEV__) { + if (hookTypesDev === null) { + hookTypesDev = [hookName]; + } else { + hookTypesDev.push(hookName); + } + } +} + +function updateHookTypeDev(hookName: HookType) { + if (__DEV__) { + if (hookTypesDev !== null) { + if (hookTypesDev[++hookTypesUpdateIndexDev] !== hookName) { + warnOnHookMismatchInDev(hookName); + } + } + } +} + +function warnOnHookMismatchInDev(currentHookName: HookType) { if (__DEV__) { const componentName = getComponentName( ((currentlyRenderingFiber: any): Fiber).type, @@ -191,44 +212,42 @@ 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; i++) { + const oldHookName = hookTypesDev[i]; + const newHookName = + i === hookTypesUpdateIndexDev ? 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}`; + + // Extra space so second column lines up + // lol @ IE not supporting String#repeat + while (row.length < secondColumnStart) { + row += ' '; + } - row += newHookName + '\n'; + row += newHookName + '\n'; - table += row; - prevHook = (prevHook.next: any); - nextHook = (nextHook.next: any); - n++; - } + 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, - ); + 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 +312,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,12 +334,21 @@ 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) + if (__DEV__) { ReactCurrentDispatcher.current = - nextCurrentHook === null + hookTypesDev === null ? HooksDispatcherOnMountInDEV : HooksDispatcherOnUpdateInDEV; } else { + // TODO This check isn't always accurate. + // Not all hooks are added to the Fiber's list (e.g. context) + // so using a non-null current hook might indicate "mount" when it's really an "update". + // We don't have a better data structure to check in production bundles though. + ReactCurrentDispatcher.current = nextCurrentHook === null ? HooksDispatcherOnMount @@ -328,14 +363,17 @@ 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__) { + hookTypesUpdateIndexDev = -1; + } + ReactCurrentDispatcher.current = __DEV__ ? HooksDispatcherOnUpdateInDEV : HooksDispatcherOnUpdate; @@ -347,10 +385,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 +396,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 +461,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 +494,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 +543,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,22 +557,6 @@ function basicStateReducer(state: S, action: BasicStateAction): S { return typeof action === 'function' ? action(state) : action; } -function mountContext( - context: ReactContext, - observedBits: void | number | boolean, -): T { - mountWorkInProgressHook(); - return readContext(context, observedBits); -} - -function updateContext( - context: ReactContext, - observedBits: void | number | boolean, -): T { - updateWorkInProgressHook(); - return readContext(context, observedBits); -} - function mountReducer( reducer: (S, A) => S, initialArg: I, @@ -1150,7 +1171,7 @@ const HooksDispatcherOnMount: Dispatcher = { readContext, useCallback: mountCallback, - useContext: mountContext, + useContext: readContext, useEffect: mountEffect, useImperativeHandle: mountImperativeHandle, useLayoutEffect: mountLayoutEffect, @@ -1165,7 +1186,7 @@ const HooksDispatcherOnUpdate: Dispatcher = { readContext, useCallback: updateCallback, - useContext: updateContext, + useContext: readContext, useEffect: updateEffect, useImperativeHandle: updateImperativeHandle, useLayoutEffect: updateLayoutEffect, @@ -1212,6 +1233,7 @@ if (__DEV__) { useCallback(callback: T, deps: Array | void | null): T { currentHookNameInDev = 'useCallback'; + mountHookTypeDev(currentHookNameInDev); return mountCallback(callback, deps); }, useContext( @@ -1219,13 +1241,15 @@ if (__DEV__) { observedBits: void | number | boolean, ): T { currentHookNameInDev = 'useContext'; - return mountContext(context, observedBits); + mountHookTypeDev(currentHookNameInDev); + return readContext(context, observedBits); }, useEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { currentHookNameInDev = 'useEffect'; + mountHookTypeDev(currentHookNameInDev); return mountEffect(create, deps); }, useImperativeHandle( @@ -1234,6 +1258,7 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useImperativeHandle'; + mountHookTypeDev(currentHookNameInDev); return mountImperativeHandle(ref, create, deps); }, useLayoutEffect( @@ -1241,10 +1266,12 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useLayoutEffect'; + mountHookTypeDev(currentHookNameInDev); return mountLayoutEffect(create, deps); }, useMemo(create: () => T, deps: Array | void | null): T { currentHookNameInDev = 'useMemo'; + mountHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1259,6 +1286,7 @@ if (__DEV__) { init?: I => S, ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; + mountHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1269,12 +1297,14 @@ if (__DEV__) { }, useRef(initialValue: T): {current: T} { currentHookNameInDev = 'useRef'; + mountHookTypeDev(currentHookNameInDev); return mountRef(initialValue); }, useState( initialState: (() => S) | S, ): [S, Dispatch>] { currentHookNameInDev = 'useState'; + mountHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1285,6 +1315,7 @@ if (__DEV__) { }, useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { currentHookNameInDev = 'useDebugValue'; + mountHookTypeDev(currentHookNameInDev); return mountDebugValue(value, formatterFn); }, }; @@ -1299,6 +1330,7 @@ if (__DEV__) { useCallback(callback: T, deps: Array | void | null): T { currentHookNameInDev = 'useCallback'; + updateHookTypeDev(currentHookNameInDev); return updateCallback(callback, deps); }, useContext( @@ -1306,13 +1338,15 @@ if (__DEV__) { observedBits: void | number | boolean, ): T { currentHookNameInDev = 'useContext'; - return updateContext(context, observedBits); + updateHookTypeDev(currentHookNameInDev); + return readContext(context, observedBits); }, useEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { currentHookNameInDev = 'useEffect'; + updateHookTypeDev(currentHookNameInDev); return updateEffect(create, deps); }, useImperativeHandle( @@ -1321,6 +1355,7 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useImperativeHandle'; + updateHookTypeDev(currentHookNameInDev); return updateImperativeHandle(ref, create, deps); }, useLayoutEffect( @@ -1328,10 +1363,12 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useLayoutEffect'; + updateHookTypeDev(currentHookNameInDev); return updateLayoutEffect(create, deps); }, useMemo(create: () => T, deps: Array | void | null): T { currentHookNameInDev = 'useMemo'; + updateHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1346,6 +1383,7 @@ if (__DEV__) { init?: I => S, ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; + updateHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1356,12 +1394,14 @@ if (__DEV__) { }, useRef(initialValue: T): {current: T} { currentHookNameInDev = 'useRef'; + updateHookTypeDev(currentHookNameInDev); return updateRef(initialValue); }, useState( initialState: (() => S) | S, ): [S, Dispatch>] { currentHookNameInDev = 'useState'; + updateHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1372,6 +1412,7 @@ if (__DEV__) { }, useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { currentHookNameInDev = 'useDebugValue'; + updateHookTypeDev(currentHookNameInDev); return updateDebugValue(value, formatterFn); }, }; @@ -1388,6 +1429,7 @@ if (__DEV__) { useCallback(callback: T, deps: Array | void | null): T { currentHookNameInDev = 'useCallback'; warnInvalidHookAccess(); + mountHookTypeDev(currentHookNameInDev); return mountCallback(callback, deps); }, useContext( @@ -1396,7 +1438,8 @@ if (__DEV__) { ): T { currentHookNameInDev = 'useContext'; warnInvalidHookAccess(); - return mountContext(context, observedBits); + mountHookTypeDev(currentHookNameInDev); + return readContext(context, observedBits); }, useEffect( create: () => (() => void) | void, @@ -1404,6 +1447,7 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useEffect'; warnInvalidHookAccess(); + mountHookTypeDev(currentHookNameInDev); return mountEffect(create, deps); }, useImperativeHandle( @@ -1413,6 +1457,7 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useImperativeHandle'; warnInvalidHookAccess(); + mountHookTypeDev(currentHookNameInDev); return mountImperativeHandle(ref, create, deps); }, useLayoutEffect( @@ -1421,11 +1466,13 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useLayoutEffect'; warnInvalidHookAccess(); + mountHookTypeDev(currentHookNameInDev); return mountLayoutEffect(create, deps); }, useMemo(create: () => T, deps: Array | void | null): T { currentHookNameInDev = 'useMemo'; warnInvalidHookAccess(); + mountHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1441,6 +1488,7 @@ if (__DEV__) { ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; warnInvalidHookAccess(); + mountHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1452,6 +1500,7 @@ if (__DEV__) { useRef(initialValue: T): {current: T} { currentHookNameInDev = 'useRef'; warnInvalidHookAccess(); + mountHookTypeDev(currentHookNameInDev); return mountRef(initialValue); }, useState( @@ -1459,6 +1508,7 @@ if (__DEV__) { ): [S, Dispatch>] { currentHookNameInDev = 'useState'; warnInvalidHookAccess(); + mountHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { @@ -1470,6 +1520,7 @@ if (__DEV__) { useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { currentHookNameInDev = 'useDebugValue'; warnInvalidHookAccess(); + mountHookTypeDev(currentHookNameInDev); return mountDebugValue(value, formatterFn); }, }; @@ -1486,6 +1537,7 @@ if (__DEV__) { useCallback(callback: T, deps: Array | void | null): T { currentHookNameInDev = 'useCallback'; warnInvalidHookAccess(); + updateHookTypeDev(currentHookNameInDev); return updateCallback(callback, deps); }, useContext( @@ -1494,7 +1546,8 @@ if (__DEV__) { ): T { currentHookNameInDev = 'useContext'; warnInvalidHookAccess(); - return updateContext(context, observedBits); + updateHookTypeDev(currentHookNameInDev); + return readContext(context, observedBits); }, useEffect( create: () => (() => void) | void, @@ -1502,6 +1555,7 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useEffect'; warnInvalidHookAccess(); + updateHookTypeDev(currentHookNameInDev); return updateEffect(create, deps); }, useImperativeHandle( @@ -1511,6 +1565,7 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useImperativeHandle'; warnInvalidHookAccess(); + updateHookTypeDev(currentHookNameInDev); return updateImperativeHandle(ref, create, deps); }, useLayoutEffect( @@ -1519,11 +1574,13 @@ if (__DEV__) { ): void { currentHookNameInDev = 'useLayoutEffect'; warnInvalidHookAccess(); + updateHookTypeDev(currentHookNameInDev); return updateLayoutEffect(create, deps); }, useMemo(create: () => T, deps: Array | void | null): T { currentHookNameInDev = 'useMemo'; warnInvalidHookAccess(); + updateHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1539,6 +1596,7 @@ if (__DEV__) { ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; warnInvalidHookAccess(); + updateHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1550,6 +1608,7 @@ if (__DEV__) { useRef(initialValue: T): {current: T} { currentHookNameInDev = 'useRef'; warnInvalidHookAccess(); + updateHookTypeDev(currentHookNameInDev); return updateRef(initialValue); }, useState( @@ -1557,6 +1616,7 @@ if (__DEV__) { ): [S, Dispatch>] { currentHookNameInDev = 'useState'; warnInvalidHookAccess(); + updateHookTypeDev(currentHookNameInDev); const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { @@ -1568,6 +1628,7 @@ if (__DEV__) { useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { currentHookNameInDev = 'useDebugValue'; warnInvalidHookAccess(); + updateHookTypeDev(currentHookNameInDev); 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..bcc3754bd19a4 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -1002,7 +1002,7 @@ describe('ReactHooks', () => { }).toThrow('Rendered more hooks than during the previous render.'); if (__DEV__) { - expect(console.error).toHaveBeenCalledTimes(3); + expect(console.error).toHaveBeenCalledTimes(4); expect(console.error.calls.argsFor(0)[0]).toContain( 'Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks', ); @@ -1337,74 +1337,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 bb8e033623f3e..5f7906aef5e3a 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -1740,8 +1740,20 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); expect(() => { - expect(ReactNoop.flush()).toEqual(['A: 2, B: 3, C: 0']); - }).toThrow('Rendered more hooks than during the previous render'); + expect(() => { + expect(ReactNoop.flush()).toEqual(['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')]); @@ -1819,8 +1831,19 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); expect(() => { - expect(ReactNoop.flush()).toEqual([]); - }).toThrow('Rendered more hooks than during the previous render'); + expect(() => { + expect(ReactNoop.flush()).toEqual([]); + }).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();