From 4d8a8cf25296df0ef7f1c147fcc2831b789c0c57 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 14 Jan 2019 04:08:31 +0000 Subject: [PATCH 01/35] warn when returning different hooks on next render like it says. adds a field to Hook to track effect 'type', and compares when cloning subsequently. --- .../react-reconciler/src/ReactFiberHooks.js | 46 +++++++++++++------ .../src/__tests__/ReactHooks-test.internal.js | 26 +++++++++++ 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index b859f4067b6a6..efc7c35161e13 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -34,6 +34,7 @@ import { } from './ReactFiberScheduler'; import invariant from 'shared/invariant'; +import warning from 'shared/warning'; import areHookInputsEqual from 'shared/areHookInputsEqual'; type Update = { @@ -47,7 +48,18 @@ type UpdateQueue = { dispatch: any, }; +type HookType = + | 'state' + | 'reducer' + | 'context' + | 'ref' + | 'effect' + | 'callback' + | 'memo' + | 'imperative-handle'; + export type Hook = { + hookType: HookType, memoizedState: any, baseState: any, @@ -237,8 +249,9 @@ export function resetHooks(): void { numberOfReRenders = 0; } -function createHook(): Hook { +function createHook(hookType: HookType): Hook { return { + hookType, memoizedState: null, baseState: null, @@ -249,8 +262,13 @@ function createHook(): Hook { }; } -function cloneHook(hook: Hook): Hook { +function cloneHook(hook: Hook, hookType: HookType): Hook { + if (__DEV__ && hookType !== hook.hookType) { + warning(false, 'Bad hook order'); + } + return { + hookType: hook.hookType, memoizedState: hook.memoizedState, baseState: hook.baseState, @@ -261,7 +279,7 @@ function cloneHook(hook: Hook): Hook { }; } -function createWorkInProgressHook(): Hook { +function createWorkInProgressHook(hookType: HookType): Hook { if (workInProgressHook === null) { // This is the first hook in the list if (firstWorkInProgressHook === null) { @@ -269,10 +287,10 @@ function createWorkInProgressHook(): Hook { currentHook = firstCurrentHook; if (currentHook === null) { // This is a newly mounted hook - workInProgressHook = createHook(); + workInProgressHook = createHook(hookType); } else { // Clone the current hook. - workInProgressHook = cloneHook(currentHook); + workInProgressHook = cloneHook(currentHook, hookType); } firstWorkInProgressHook = workInProgressHook; } else { @@ -287,15 +305,15 @@ function createWorkInProgressHook(): Hook { let hook; if (currentHook === null) { // This is a newly mounted hook - hook = createHook(); + hook = createHook(hookType); } else { currentHook = currentHook.next; if (currentHook === null) { // This is a newly mounted hook - hook = createHook(); + hook = createHook(hookType); } else { // Clone the current hook. - hook = cloneHook(currentHook); + hook = cloneHook(currentHook, hookType); } } // Append to the end of the list @@ -346,7 +364,9 @@ export function useReducer( initialAction: A | void | null, ): [S, Dispatch] { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(); + workInProgressHook = createWorkInProgressHook( + reducer === basicStateReducer ? 'state' : 'reducer', + ); let queue: UpdateQueue | null = (workInProgressHook.queue: any); if (queue !== null) { // Already have a queue, so this is an update. @@ -499,7 +519,7 @@ function pushEffect(tag, create, destroy, inputs) { export function useRef(initialValue: T): {current: T} { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(); + workInProgressHook = createWorkInProgressHook('ref'); let ref; if (workInProgressHook.memoizedState === null) { @@ -535,7 +555,7 @@ export function useEffect( function useEffectImpl(fiberEffectTag, hookEffectTag, create, inputs): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(); + workInProgressHook = createWorkInProgressHook('effect'); let nextInputs = inputs !== undefined && inputs !== null ? inputs : [create]; let destroy = null; @@ -593,7 +613,7 @@ export function useCallback( inputs: Array | void | null, ): T { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(); + workInProgressHook = createWorkInProgressHook('callback'); const nextInputs = inputs !== undefined && inputs !== null ? inputs : [callback]; @@ -614,7 +634,7 @@ export function useMemo( inputs: Array | void | null, ): T { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(); + workInProgressHook = createWorkInProgressHook('memo'); const nextInputs = inputs !== undefined && inputs !== null ? inputs : [nextCreate]; diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 709d819d6713e..87055d87ab498 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -92,4 +92,30 @@ describe('ReactHooks', () => { const root = ReactTestRenderer.create(); expect(root.toJSON()).toMatchSnapshot(); }); + + it('warns on using a different hooks on subsequent renders', () => { + const {useState, useReducer} = React; + function App(props) { + if (props.flip) { + const [count, setCount] = useState(0); + const [state, dispatch] = useReducer( + (state, action) => action.payload, + 0, + ); + return null; + } else { + const [state, dispatch] = useReducer( + (state, action) => action.payload, + 0, + ); + const [count, setCount] = useState(0); + return null; + } + } + let root = ReactTestRenderer.create(); + expect(() => { + root.update(); + }).toWarnDev(['Warning: Bad hook order\n' + + ' in App (at **)']); + }); }); From 0daee73bd7c7c8b3487c35e39b6b2ee4158f76fa Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 14 Jan 2019 04:11:59 +0000 Subject: [PATCH 02/35] lint --- .../src/__tests__/ReactHooks-test.internal.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 87055d87ab498..639802a9bc805 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -112,10 +112,9 @@ describe('ReactHooks', () => { return null; } } - let root = ReactTestRenderer.create(); + let root = ReactTestRenderer.create(); expect(() => { root.update(); - }).toWarnDev(['Warning: Bad hook order\n' + - ' in App (at **)']); + }).toWarnDev(['Warning: Bad hook order\n' + ' in App (at **)']); }); }); From 623fb58b82c46572b3aafb6130835e58f8830e2f Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 14 Jan 2019 05:20:35 +0000 Subject: [PATCH 03/35] review changes - numbered enum for hook types - s/hookType/_debugType - better dce --- .../react-reconciler/src/ReactFiberHooks.js | 75 ++++++++++++------- .../src/__tests__/ReactHooks-test.internal.js | 4 +- 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index efc7c35161e13..8966a7785fa19 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -49,17 +49,27 @@ type UpdateQueue = { }; type HookType = - | 'state' - | 'reducer' - | 'context' - | 'ref' - | 'effect' - | 'callback' - | 'memo' - | 'imperative-handle'; + | 0 //'state' + | 1 // 'reducer' + | 2 // 'context' + | 3 // 'ref' + | 4 // 'effect' + | 5 // 'callback' + | 6 // 'memo' + | 7; // 'imperative-handle' + +const StateHook = 0; +const ReducerHook = 1; +const ContextHook = 2; +const RefHook = 3; +const EffectHook = 4; +const CallbackHook = 5; +const MemoHook = 6; +const ImperativeHandleHook = 7; export type Hook = { - hookType: HookType, + _debugType?: HookType, + memoizedState: any, baseState: any, @@ -250,8 +260,7 @@ export function resetHooks(): void { } function createHook(hookType: HookType): Hook { - return { - hookType, + let hook: Hook = { memoizedState: null, baseState: null, @@ -260,15 +269,14 @@ function createHook(hookType: HookType): Hook { next: null, }; -} - -function cloneHook(hook: Hook, hookType: HookType): Hook { - if (__DEV__ && hookType !== hook.hookType) { - warning(false, 'Bad hook order'); + if (__DEV__) { + hook._debugType = hookType; } + return hook; +} - return { - hookType: hook.hookType, +function cloneHook(hook: Hook): Hook { + let nextHook: Hook = { memoizedState: hook.memoizedState, baseState: hook.baseState, @@ -277,6 +285,17 @@ function cloneHook(hook: Hook, hookType: HookType): Hook { next: null, }; + + if (__DEV__) { + nextHook._debugType = hook._debugType; + } + return nextHook; +} + +function compareHookTypes(prevHookType, nextHookType) { + if (prevHookType !== nextHookType) { + warning(false, 'Bad hook order!'); + } } function createWorkInProgressHook(hookType: HookType): Hook { @@ -290,7 +309,10 @@ function createWorkInProgressHook(hookType: HookType): Hook { workInProgressHook = createHook(hookType); } else { // Clone the current hook. - workInProgressHook = cloneHook(currentHook, hookType); + if (__DEV__) { + compareHookTypes(currentHook._debugType, hookType); + } + workInProgressHook = cloneHook(currentHook); } firstWorkInProgressHook = workInProgressHook; } else { @@ -313,7 +335,10 @@ function createWorkInProgressHook(hookType: HookType): Hook { hook = createHook(hookType); } else { // Clone the current hook. - hook = cloneHook(currentHook, hookType); + if (__DEV__) { + compareHookTypes(currentHook._debugType, hookType); + } + hook = cloneHook(currentHook); } } // Append to the end of the list @@ -365,7 +390,7 @@ export function useReducer( ): [S, Dispatch] { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); workInProgressHook = createWorkInProgressHook( - reducer === basicStateReducer ? 'state' : 'reducer', + reducer === basicStateReducer ? StateHook : ReducerHook, ); let queue: UpdateQueue | null = (workInProgressHook.queue: any); if (queue !== null) { @@ -519,7 +544,7 @@ function pushEffect(tag, create, destroy, inputs) { export function useRef(initialValue: T): {current: T} { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook('ref'); + workInProgressHook = createWorkInProgressHook(RefHook); let ref; if (workInProgressHook.memoizedState === null) { @@ -555,7 +580,7 @@ export function useEffect( function useEffectImpl(fiberEffectTag, hookEffectTag, create, inputs): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook('effect'); + workInProgressHook = createWorkInProgressHook(EffectHook); let nextInputs = inputs !== undefined && inputs !== null ? inputs : [create]; let destroy = null; @@ -613,7 +638,7 @@ export function useCallback( inputs: Array | void | null, ): T { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook('callback'); + workInProgressHook = createWorkInProgressHook(CallbackHook); const nextInputs = inputs !== undefined && inputs !== null ? inputs : [callback]; @@ -634,7 +659,7 @@ export function useMemo( inputs: Array | void | null, ): T { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook('memo'); + workInProgressHook = createWorkInProgressHook(MemoHook); const nextInputs = inputs !== undefined && inputs !== null ? inputs : [nextCreate]; diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 639802a9bc805..585fdb120a3d3 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -93,7 +93,7 @@ describe('ReactHooks', () => { expect(root.toJSON()).toMatchSnapshot(); }); - it('warns on using a different hooks on subsequent renders', () => { + it('warns on using differently ordered hooks on subsequent renders', () => { const {useState, useReducer} = React; function App(props) { if (props.flip) { @@ -115,6 +115,6 @@ describe('ReactHooks', () => { let root = ReactTestRenderer.create(); expect(() => { root.update(); - }).toWarnDev(['Warning: Bad hook order\n' + ' in App (at **)']); + }).toWarnDev(['Warning: Bad hook order!\n in App (at **)']); }); }); From 1204a0c9b634f7af290e70a3febe65285b3b8edf Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 14 Jan 2019 05:41:20 +0000 Subject: [PATCH 04/35] cleaner detection location --- .../react-reconciler/src/ReactFiberHooks.js | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 8966a7785fa19..edf8b9c2a2087 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -287,17 +287,14 @@ function cloneHook(hook: Hook): Hook { }; if (__DEV__) { + if (!currentHook || currentHook._debugType !== hook._debugType) { + warning(false, 'Bad hook order!'); + } nextHook._debugType = hook._debugType; } return nextHook; } -function compareHookTypes(prevHookType, nextHookType) { - if (prevHookType !== nextHookType) { - warning(false, 'Bad hook order!'); - } -} - function createWorkInProgressHook(hookType: HookType): Hook { if (workInProgressHook === null) { // This is the first hook in the list @@ -308,10 +305,7 @@ function createWorkInProgressHook(hookType: HookType): Hook { // This is a newly mounted hook workInProgressHook = createHook(hookType); } else { - // Clone the current hook. - if (__DEV__) { - compareHookTypes(currentHook._debugType, hookType); - } + // Clone the current hook. workInProgressHook = cloneHook(currentHook); } firstWorkInProgressHook = workInProgressHook; @@ -335,9 +329,6 @@ function createWorkInProgressHook(hookType: HookType): Hook { hook = createHook(hookType); } else { // Clone the current hook. - if (__DEV__) { - compareHookTypes(currentHook._debugType, hookType); - } hook = cloneHook(currentHook); } } From d597fcdb601c3d2d6253f40058fc7b0f3a8dda47 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 14 Jan 2019 05:44:17 +0000 Subject: [PATCH 05/35] redundant comments --- packages/react-reconciler/src/ReactFiberHooks.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index edf8b9c2a2087..c500cc5254eaa 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -48,15 +48,7 @@ type UpdateQueue = { dispatch: any, }; -type HookType = - | 0 //'state' - | 1 // 'reducer' - | 2 // 'context' - | 3 // 'ref' - | 4 // 'effect' - | 5 // 'callback' - | 6 // 'memo' - | 7; // 'imperative-handle' +type HookType = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7; const StateHook = 0; const ReducerHook = 1; @@ -305,7 +297,7 @@ function createWorkInProgressHook(hookType: HookType): Hook { // This is a newly mounted hook workInProgressHook = createHook(hookType); } else { - // Clone the current hook. + // Clone the current hook. workInProgressHook = cloneHook(currentHook); } firstWorkInProgressHook = workInProgressHook; From 1c7314f948e37c90c4afac7514ecad4d62f43fcd Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 14 Jan 2019 05:49:19 +0000 Subject: [PATCH 06/35] different EffectHook / LayoutEffectHook --- packages/react-reconciler/src/ReactFiberHooks.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index c500cc5254eaa..e83b42554a99e 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -48,16 +48,17 @@ type UpdateQueue = { dispatch: any, }; -type HookType = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7; +type HookType = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8; const StateHook = 0; const ReducerHook = 1; const ContextHook = 2; const RefHook = 3; const EffectHook = 4; -const CallbackHook = 5; -const MemoHook = 6; -const ImperativeHandleHook = 7; +const LayoutEffectHook = 5; +const CallbackHook = 6; +const MemoHook = 7; +const ImperativeHandleHook = 8; export type Hook = { _debugType?: HookType, @@ -563,7 +564,7 @@ export function useEffect( function useEffectImpl(fiberEffectTag, hookEffectTag, create, inputs): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(EffectHook); + workInProgressHook = createWorkInProgressHook(fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook); let nextInputs = inputs !== undefined && inputs !== null ? inputs : [create]; let destroy = null; From 5221e916b6e9dfd46198cd5aabd3f3e61791473a Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 14 Jan 2019 06:04:43 +0000 Subject: [PATCH 07/35] prettier --- packages/react-reconciler/src/ReactFiberHooks.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index e83b42554a99e..ca57e24b273f9 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -564,7 +564,9 @@ export function useEffect( function useEffectImpl(fiberEffectTag, hookEffectTag, create, inputs): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook); + workInProgressHook = createWorkInProgressHook( + fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook, + ); let nextInputs = inputs !== undefined && inputs !== null ? inputs : [create]; let destroy = null; From bcbce2846c61c00ef90e562776bf5495b37a0a1c Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 14 Jan 2019 06:23:03 +0000 Subject: [PATCH 08/35] top level currentHookType --- .../react-reconciler/src/ReactFiberHooks.js | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index ca57e24b273f9..a4be2da9804df 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -60,6 +60,8 @@ const CallbackHook = 6; const MemoHook = 7; const ImperativeHandleHook = 8; +let currentHookType: HookType | null = null; + export type Hook = { _debugType?: HookType, @@ -252,7 +254,7 @@ export function resetHooks(): void { numberOfReRenders = 0; } -function createHook(hookType: HookType): Hook { +function createHook(): Hook { let hook: Hook = { memoizedState: null, @@ -263,7 +265,8 @@ function createHook(hookType: HookType): Hook { next: null, }; if (__DEV__) { - hook._debugType = hookType; + invariant(currentHookType !== null, 'Forgot to set currentHookType'); + hook._debugType = currentHookType; } return hook; } @@ -288,7 +291,7 @@ function cloneHook(hook: Hook): Hook { return nextHook; } -function createWorkInProgressHook(hookType: HookType): Hook { +function createWorkInProgressHook(): Hook { if (workInProgressHook === null) { // This is the first hook in the list if (firstWorkInProgressHook === null) { @@ -296,7 +299,7 @@ function createWorkInProgressHook(hookType: HookType): Hook { currentHook = firstCurrentHook; if (currentHook === null) { // This is a newly mounted hook - workInProgressHook = createHook(hookType); + workInProgressHook = createHook(); } else { // Clone the current hook. workInProgressHook = cloneHook(currentHook); @@ -314,12 +317,12 @@ function createWorkInProgressHook(hookType: HookType): Hook { let hook; if (currentHook === null) { // This is a newly mounted hook - hook = createHook(hookType); + hook = createHook(); } else { currentHook = currentHook.next; if (currentHook === null) { // This is a newly mounted hook - hook = createHook(hookType); + hook = createHook(); } else { // Clone the current hook. hook = cloneHook(currentHook); @@ -373,9 +376,8 @@ export function useReducer( initialAction: A | void | null, ): [S, Dispatch] { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook( - reducer === basicStateReducer ? StateHook : ReducerHook, - ); + currentHookType = reducer === basicStateReducer ? StateHook : ReducerHook; + workInProgressHook = createWorkInProgressHook(); let queue: UpdateQueue | null = (workInProgressHook.queue: any); if (queue !== null) { // Already have a queue, so this is an update. @@ -528,7 +530,8 @@ function pushEffect(tag, create, destroy, inputs) { export function useRef(initialValue: T): {current: T} { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(RefHook); + currentHookType = RefHook; + workInProgressHook = createWorkInProgressHook(); let ref; if (workInProgressHook.memoizedState === null) { @@ -564,9 +567,9 @@ export function useEffect( function useEffectImpl(fiberEffectTag, hookEffectTag, create, inputs): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook( - fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook, - ); + currentHookType = + fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook; + workInProgressHook = createWorkInProgressHook(); let nextInputs = inputs !== undefined && inputs !== null ? inputs : [create]; let destroy = null; @@ -624,7 +627,8 @@ export function useCallback( inputs: Array | void | null, ): T { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(CallbackHook); + currentHookType = CallbackHook; + workInProgressHook = createWorkInProgressHook(); const nextInputs = inputs !== undefined && inputs !== null ? inputs : [callback]; @@ -645,7 +649,8 @@ export function useMemo( inputs: Array | void | null, ): T { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(MemoHook); + currentHookType = MemoHook; + workInProgressHook = createWorkInProgressHook(); const nextInputs = inputs !== undefined && inputs !== null ? inputs : [nextCreate]; From a604ea119c7ee4e8f6e62459931f60792ab1a376 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 14 Jan 2019 07:10:37 +0000 Subject: [PATCH 09/35] nulling currentHookType need to verify dce still works --- packages/react-reconciler/src/ReactFiberHooks.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index a4be2da9804df..0d919241669c6 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -378,6 +378,7 @@ export function useReducer( currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); currentHookType = reducer === basicStateReducer ? StateHook : ReducerHook; workInProgressHook = createWorkInProgressHook(); + currentHookType = null; let queue: UpdateQueue | null = (workInProgressHook.queue: any); if (queue !== null) { // Already have a queue, so this is an update. @@ -532,6 +533,7 @@ export function useRef(initialValue: T): {current: T} { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); currentHookType = RefHook; workInProgressHook = createWorkInProgressHook(); + currentHookType = null; let ref; if (workInProgressHook.memoizedState === null) { @@ -570,6 +572,7 @@ function useEffectImpl(fiberEffectTag, hookEffectTag, create, inputs): void { currentHookType = fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook; workInProgressHook = createWorkInProgressHook(); + currentHookType = null; let nextInputs = inputs !== undefined && inputs !== null ? inputs : [create]; let destroy = null; @@ -629,7 +632,7 @@ export function useCallback( currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); currentHookType = CallbackHook; workInProgressHook = createWorkInProgressHook(); - + currentHookType = null; const nextInputs = inputs !== undefined && inputs !== null ? inputs : [callback]; @@ -651,6 +654,7 @@ export function useMemo( currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); currentHookType = MemoHook; workInProgressHook = createWorkInProgressHook(); + currentHookType = null; const nextInputs = inputs !== undefined && inputs !== null ? inputs : [nextCreate]; From 8b22c873788626b9c4410995e926b4bfb9821b63 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Thu, 17 Jan 2019 23:50:10 +0000 Subject: [PATCH 10/35] small enhancements --- .../react-reconciler/src/ReactFiberHooks.js | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 0d919241669c6..233262e5c03c3 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -255,19 +255,28 @@ export function resetHooks(): void { } function createHook(): Hook { - let hook: Hook = { - memoizedState: null, + let hook: Hook = __DEV__ + ? { + _debugType: currentHookType, - baseState: null, - queue: null, - baseUpdate: null, + memoizedState: null, + + baseState: null, + queue: null, + baseUpdate: null, + + next: null, + } + : { + memoizedState: null, + + baseState: null, + queue: null, + baseUpdate: null, + + next: null, + }; - next: null, - }; - if (__DEV__) { - invariant(currentHookType !== null, 'Forgot to set currentHookType'); - hook._debugType = currentHookType; - } return hook; } @@ -283,7 +292,7 @@ function cloneHook(hook: Hook): Hook { }; if (__DEV__) { - if (!currentHook || currentHook._debugType !== hook._debugType) { + if (currentHookType !== hook._debugType) { warning(false, 'Bad hook order!'); } nextHook._debugType = hook._debugType; From 95aa0035d921f2dfbe1a98357b83068c626c4d8a Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Thu, 17 Jan 2019 23:58:30 +0000 Subject: [PATCH 11/35] hook order checks for useContext/useImperative --- packages/react-reconciler/src/ReactFiberHooks.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 233262e5c03c3..58e8a1ef2545d 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -365,6 +365,11 @@ export function useContext( ): T { // Ensure we're in a function component (class components support only the // .unstable_read() form) + if(__DEV__){ + currentHookType = ContextHook + createWorkInProgressHook() + currentHookType = null + } resolveCurrentlyRenderingFiber(); return readContext(context, observedBits); } @@ -579,7 +584,8 @@ export function useEffect( function useEffectImpl(fiberEffectTag, hookEffectTag, create, inputs): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); currentHookType = - fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook; + currentHookType || // it could be an ImperativeHandleHook + (fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook); workInProgressHook = createWorkInProgressHook(); currentHookType = null; @@ -614,6 +620,7 @@ export function useImperativeHandle( ? inputs.concat([ref]) : [ref, create]; + currentHookType = ImperativeHandleHook // TODO: I've implemented this on top of useEffect because it's almost the // same thing, and it would require an equal amount of code. It doesn't seem // like a common enough use case to justify the additional size. From 4676b6baac053127273a194af3bbacaffc9137e3 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Fri, 18 Jan 2019 00:05:02 +0000 Subject: [PATCH 12/35] prettier --- packages/react-reconciler/src/ReactFiberHooks.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 97a5a7729485e..d0907b34307bb 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -431,10 +431,10 @@ export function useContext( } // Ensure we're in a function component (class components support only the // .unstable_read() form) - if(__DEV__){ - currentHookType = ContextHook - createWorkInProgressHook() - currentHookType = null + if (__DEV__) { + currentHookType = ContextHook; + createWorkInProgressHook(); + currentHookType = null; } resolveCurrentlyRenderingFiber(); return readContext(context, observedBits); @@ -722,7 +722,7 @@ export function useImperativeHandle( const nextDeps = deps !== null && deps !== undefined ? deps.concat([ref]) : [ref]; - currentHookType = ImperativeHandleHook + currentHookType = ImperativeHandleHook; // TODO: I've implemented this on top of useEffect because it's almost the // same thing, and it would require an equal amount of code. It doesn't seem // like a common enough use case to justify the additional size. @@ -770,7 +770,9 @@ export function useCallback( currentHookType = CallbackHook; workInProgressHook = createWorkInProgressHook(); currentHookType = null; + const nextDeps = deps === undefined ? null : deps; + const prevState = workInProgressHook.memoizedState; if (prevState !== null) { if (nextDeps !== null) { From 5a94b5404c58a4d07e1b1ada084243694d3713cf Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Fri, 18 Jan 2019 00:37:05 +0000 Subject: [PATCH 13/35] stray whitespace --- packages/react-reconciler/src/ReactFiberHooks.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index d0907b34307bb..a22eb83ea5dd0 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -70,7 +70,6 @@ let currentHookType: HookType | null = null; export type Hook = { _debugType?: HookType, - memoizedState: any, baseState: any, @@ -325,7 +324,6 @@ function createHook(): Hook { let hook: Hook = __DEV__ ? { _debugType: currentHookType, - memoizedState: null, baseState: null, From aca87cd7eab6617ca4a5440a5a7b4a719da26b30 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Fri, 18 Jan 2019 01:18:16 +0000 Subject: [PATCH 14/35] move some bits around --- .../react-reconciler/src/ReactFiberHooks.js | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index a22eb83ea5dd0..71be7f88e1bc9 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -426,14 +426,12 @@ export function useContext( ): T { if (__DEV__) { currentHookNameInDev = 'useContext'; - } - // Ensure we're in a function component (class components support only the - // .unstable_read() form) - if (__DEV__) { currentHookType = ContextHook; createWorkInProgressHook(); currentHookType = null; } + // Ensure we're in a function component (class components support only the + // .unstable_read() form) resolveCurrentlyRenderingFiber(); return readContext(context, observedBits); } @@ -461,8 +459,8 @@ export function useReducer( currentHookNameInDev = 'useReducer'; } } - currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); currentHookType = reducer === basicStateReducer ? StateHook : ReducerHook; + currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); workInProgressHook = createWorkInProgressHook(); currentHookType = null; let queue: UpdateQueue | null = (workInProgressHook.queue: any); @@ -679,9 +677,10 @@ export function useEffect( function useEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - currentHookType = - currentHookType || // it could be an ImperativeHandleHook - (fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook); + if (currentHookType !== ImperativeHandleHook) { + currentHookType = + fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook; + } workInProgressHook = createWorkInProgressHook(); currentHookType = null; @@ -716,11 +715,11 @@ export function useImperativeHandle( if (__DEV__) { currentHookNameInDev = 'useImperativeHandle'; } + currentHookType = ImperativeHandleHook; // TODO: If deps are provided, should we skip comparing the ref itself? const nextDeps = deps !== null && deps !== undefined ? deps.concat([ref]) : [ref]; - currentHookType = ImperativeHandleHook; // TODO: I've implemented this on top of useEffect because it's almost the // same thing, and it would require an equal amount of code. It doesn't seem // like a common enough use case to justify the additional size. @@ -764,8 +763,8 @@ export function useCallback( if (__DEV__) { currentHookNameInDev = 'useCallback'; } - currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); currentHookType = CallbackHook; + currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); workInProgressHook = createWorkInProgressHook(); currentHookType = null; @@ -791,8 +790,8 @@ export function useMemo( if (__DEV__) { currentHookNameInDev = 'useMemo'; } - currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); currentHookType = MemoHook; + currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); workInProgressHook = createWorkInProgressHook(); currentHookType = null; From 428a3eb5aaa44b0a76fe053fa7b4426a0e6a081c Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Fri, 18 Jan 2019 01:55:04 +0000 Subject: [PATCH 15/35] better errors --- .../react-reconciler/src/ReactFiberHooks.js | 46 ++++++++++++++----- .../src/__tests__/ReactHooks-test.internal.js | 7 ++- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 71be7f88e1bc9..01f221d956633 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -65,6 +65,20 @@ const LayoutEffectHook = 5; const CallbackHook = 6; const MemoHook = 7; const ImperativeHandleHook = 8; +const DebugValueHook = 9; + +const HookDevNames = [ + 'useState', + 'useReducer', + 'useContext', + 'useRef', + 'useEffect', + 'useLayoutEffect', + 'useCallback', + 'useMemo', + 'useImperativeHandle', + 'useDebugValue', +]; let currentHookType: HookType | null = null; @@ -358,7 +372,17 @@ function cloneHook(hook: Hook): Hook { if (__DEV__) { if (currentHookType !== hook._debugType) { - warning(false, 'Bad hook order!'); + warning( + false, + 'React just detected that you called %s in a position previously called by %s. ' + + 'This breaks the rules of hooks, and will cause bugs and errors. To fix this ' + + 'make sure your component is returning the same hooks in the same order on every render. ' + + 'Learn more about the rules of hooks here: https://reactjs.org/docs/hooks-rules.html', // todo - short url + // $FlowFixMe + HookDevNames[currentHookType], + // $FlowFixMe + HookDevNames[hook._debugType], + ); } nextHook._debugType = hook._debugType; } @@ -425,7 +449,7 @@ export function useContext( observedBits: void | number | boolean, ): T { if (__DEV__) { - currentHookNameInDev = 'useContext'; + currentHookNameInDev = HookDevNames[ContextHook]; currentHookType = ContextHook; createWorkInProgressHook(); currentHookType = null; @@ -440,7 +464,7 @@ export function useState( initialState: (() => S) | S, ): [S, Dispatch>] { if (__DEV__) { - currentHookNameInDev = 'useState'; + currentHookNameInDev = HookDevNames[StateHook]; } return useReducer( basicStateReducer, @@ -456,7 +480,7 @@ export function useReducer( ): [S, Dispatch] { if (__DEV__) { if (reducer !== basicStateReducer) { - currentHookNameInDev = 'useReducer'; + currentHookNameInDev = HookDevNames[ReducerHook]; } } currentHookType = reducer === basicStateReducer ? StateHook : ReducerHook; @@ -655,7 +679,7 @@ export function useLayoutEffect( deps: Array | void | null, ): void { if (__DEV__) { - currentHookNameInDev = 'useLayoutEffect'; + currentHookNameInDev = HookDevNames[LayoutEffectHook]; } useEffectImpl(UpdateEffect, UnmountMutation | MountLayout, create, deps); } @@ -665,7 +689,7 @@ export function useEffect( deps: Array | void | null, ): void { if (__DEV__) { - currentHookNameInDev = 'useEffect'; + currentHookNameInDev = HookDevNames[EffectHook]; } useEffectImpl( UpdateEffect | PassiveEffect, @@ -677,7 +701,7 @@ export function useEffect( function useEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - if (currentHookType !== ImperativeHandleHook) { + if (currentHookType !== ImperativeHandleHook) { currentHookType = fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook; } @@ -713,7 +737,7 @@ export function useImperativeHandle( deps: Array | void | null, ): void { if (__DEV__) { - currentHookNameInDev = 'useImperativeHandle'; + currentHookNameInDev = HookDevNames[ImperativeHandleHook]; } currentHookType = ImperativeHandleHook; // TODO: If deps are provided, should we skip comparing the ref itself? @@ -745,7 +769,7 @@ export function useDebugValue( formatterFn: ?(value: any) => any, ): void { if (__DEV__) { - currentHookNameInDev = 'useDebugValue'; + currentHookNameInDev = HookDevNames[DebugValueHook]; } // This will trigger a warning if the hook is used in a non-Function component. @@ -761,7 +785,7 @@ export function useCallback( deps: Array | void | null, ): T { if (__DEV__) { - currentHookNameInDev = 'useCallback'; + currentHookNameInDev = HookDevNames[CallbackHook]; } currentHookType = CallbackHook; currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); @@ -788,7 +812,7 @@ export function useMemo( deps: Array | void | null, ): T { if (__DEV__) { - currentHookNameInDev = 'useMemo'; + currentHookNameInDev = HookDevNames[MemoHook]; } currentHookType = MemoHook; currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 6fd020b5b02ea..ac4cc8011878d 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -540,6 +540,11 @@ describe('ReactHooks', () => { let root = ReactTestRenderer.create(); expect(() => { root.update(); - }).toWarnDev(['Warning: Bad hook order!\n in App (at **)']); + }).toWarnDev('Warning: React just detected that you called useState ' + + 'in a position previously called by useReducer. This breaks ' + + 'the rules of hooks, and will cause bugs and errors. To fix this ' + + 'make sure your component is returning the same hooks in the same ' + + 'order on every render. Learn more about the rules of hooks here: ' + + 'https://reactjs.org/docs/hooks-rules.html'); }); }); From 6cd966ca609d6b96cef4fbd57784f75d7eb11426 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Fri, 18 Jan 2019 03:12:07 +0000 Subject: [PATCH 16/35] pass tests --- .../src/__tests__/ReactHooks-test.internal.js | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index ac4cc8011878d..2941d6640c13b 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -540,11 +540,19 @@ describe('ReactHooks', () => { let root = ReactTestRenderer.create(); expect(() => { root.update(); - }).toWarnDev('Warning: React just detected that you called useState ' + - 'in a position previously called by useReducer. This breaks ' + - 'the rules of hooks, and will cause bugs and errors. To fix this ' + - 'make sure your component is returning the same hooks in the same ' + - 'order on every render. Learn more about the rules of hooks here: ' + - 'https://reactjs.org/docs/hooks-rules.html'); + }).toWarnDev([ + 'Warning: React just detected that you called useState ' + + 'in a position previously called by useReducer. This breaks ' + + 'the rules of hooks, and will cause bugs and errors. To fix this ' + + 'make sure your component is returning the same hooks in the same ' + + 'order on every render. Learn more about the rules of hooks here: ' + + 'https://reactjs.org/docs/hooks-rules.html', + 'Warning: React just detected that you called useReducer ' + + 'in a position previously called by useState. This breaks ' + + 'the rules of hooks, and will cause bugs and errors. To fix this ' + + 'make sure your component is returning the same hooks in the same ' + + 'order on every render. Learn more about the rules of hooks here: ' + + 'https://reactjs.org/docs/hooks-rules.html', + ]); }); }); From 13debbb2045fada49676978009d484df6ad1e5da Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Fri, 18 Jan 2019 03:20:19 +0000 Subject: [PATCH 17/35] lint, flow --- packages/react-reconciler/src/ReactFiberHooks.js | 1 + .../src/__tests__/ReactHooks-test.internal.js | 12 ++++-------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 01f221d956633..4145f9a047c3a 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -337,6 +337,7 @@ export function resetHooks(): void { function createHook(): Hook { let hook: Hook = __DEV__ ? { + // $FlowFixMe _debugType: currentHookType, memoizedState: null, diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 2941d6640c13b..e189f8eb3f47e 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -521,21 +521,17 @@ describe('ReactHooks', () => { it('warns on using differently ordered hooks on subsequent renders', () => { const {useState, useReducer} = React; function App(props) { + /* eslint-disable no-unused-vars */ if (props.flip) { const [count, setCount] = useState(0); - const [state, dispatch] = useReducer( - (state, action) => action.payload, - 0, - ); + const [state, dispatch] = useReducer((s, a) => a, 0); return null; } else { - const [state, dispatch] = useReducer( - (state, action) => action.payload, - 0, - ); + const [state, dispatch] = useReducer((s, a) => a, 0); const [count, setCount] = useState(0); return null; } + /* eslint-enable no-unused-vars */ } let root = ReactTestRenderer.create(); expect(() => { From 9bd7051d36d7b84ad81cbc1b62c01bb161aa388b Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Fri, 18 Jan 2019 18:50:29 +0000 Subject: [PATCH 18/35] show a before - after diff --- .../react-reconciler/src/ReactFiberHooks.js | 57 +++++++++--- .../src/__tests__/ReactHooks-test.internal.js | 93 +++++++++++++++---- 2 files changed, 118 insertions(+), 32 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 0a7f48adb5e46..a28d2a3b2a693 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -81,6 +81,7 @@ const HookDevNames = [ ]; let currentHookType: HookType | null = null; +let currentHookMatches = []; export type Hook = { _debugType?: HookType, @@ -199,6 +200,40 @@ function areHookInputsEqual( return true; } +function verifyHookCallOrder() { + let mismatched = false; + for (let i = 0; i < currentHookMatches.length; i++) { + if (currentHookMatches[i][0] !== currentHookMatches[i][1]) { + mismatched = true; + break; + } + } + if (mismatched) { + const previousOrder = currentHookMatches + .map( + // $FlowFixMe - we're certain that prev is a number + ([prev, next]) => '- ' + HookDevNames[prev], + ) + .join('\n'); + const nextOrder = currentHookMatches + .map( + // $FlowFixMe - we're certain that next is a number + ([prev, next]) => '- ' + HookDevNames[next], + ) + .join('\n'); + warning( + false, + 'React just detected that you changed the order of hooks called across separate renders.\n' + + 'Previously, your component called the following hooks:\n%s\n' + + 'But more recently, your component called the following:\n%s\n' + + 'This will lead to bugs and errors if not fixed. ' + + 'For more information, read the rules of hooks: https://fb.me/rules-of-hooks', + previousOrder, + nextOrder, + ); + } +} + export function renderWithHooks( current: Fiber | null, workInProgress: Fiber, @@ -250,6 +285,8 @@ export function renderWithHooks( getComponentName(Component), ); } + verifyHookCallOrder(); + currentHookMatches = []; } } while (didScheduleRenderPhaseUpdate); @@ -327,7 +364,10 @@ export function resetHooks(): void { if (__DEV__) { currentHookNameInDev = undefined; + verifyHookCallOrder(); } + currentHookType = null; + currentHookMatches = []; didScheduleRenderPhaseUpdate = false; renderPhaseUpdates = null; @@ -337,7 +377,7 @@ export function resetHooks(): void { function createHook(): Hook { let hook: Hook = __DEV__ ? { - // $FlowFixMe + // $FlowFixMe - we're certain we've set currentHookType by now _debugType: currentHookType, memoizedState: null, @@ -372,19 +412,8 @@ function cloneHook(hook: Hook): Hook { }; if (__DEV__) { - if (currentHookType !== hook._debugType) { - warning( - false, - 'React just detected that you called %s in a position previously called by %s. ' + - 'This breaks the rules of hooks, and will cause bugs and errors. To fix this ' + - 'make sure your component is returning the same hooks in the same order on every render. ' + - 'Learn more about the rules of hooks here: https://reactjs.org/docs/hooks-rules.html', // todo - short url - // $FlowFixMe - HookDevNames[currentHookType], - // $FlowFixMe - HookDevNames[hook._debugType], - ); - } + // anything faster than this? + currentHookMatches.push([currentHookType, hook._debugType]); nextHook._debugType = hook._debugType; } return nextHook; diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 9f99c387f45c8..af09d095f27b8 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -602,32 +602,89 @@ describe('ReactHooks', () => { function App(props) { /* eslint-disable no-unused-vars */ if (props.flip) { - const [count, setCount] = useState(0); - const [state, dispatch] = useReducer((s, a) => a, 0); - return null; + useState(0); + useReducer((s, a) => a, 0); } else { - const [state, dispatch] = useReducer((s, a) => a, 0); - const [count, setCount] = useState(0); - return null; + useReducer((s, a) => a, 0); + useState(0); } + return null; /* eslint-enable no-unused-vars */ } let root = ReactTestRenderer.create(); expect(() => { root.update(); }).toWarnDev([ - 'Warning: React just detected that you called useState ' + - 'in a position previously called by useReducer. This breaks ' + - 'the rules of hooks, and will cause bugs and errors. To fix this ' + - 'make sure your component is returning the same hooks in the same ' + - 'order on every render. Learn more about the rules of hooks here: ' + - 'https://reactjs.org/docs/hooks-rules.html', - 'Warning: React just detected that you called useReducer ' + - 'in a position previously called by useState. This breaks ' + - 'the rules of hooks, and will cause bugs and errors. To fix this ' + - 'make sure your component is returning the same hooks in the same ' + - 'order on every render. Learn more about the rules of hooks here: ' + - 'https://reactjs.org/docs/hooks-rules.html', + 'Warning: React just detected that you changed the order of hooks called across separate renders.\n' + + 'Previously, your component called the following hooks:\n' + + '- useState\n' + + '- useReducer\n' + + 'But more recently, your component called the following:\n' + + '- useReducer\n' + + '- useState\n' + + 'This will lead to bugs and errors if not fixed. For more information, ' + + 'read the rules of hooks: https://fb.me/rules-of-hooks', ]); + + root.update(); + + expect(() => { + root.update(); + }).toWarnDev([ + 'Warning: React just detected that you changed the order of hooks called across separate renders.\n' + + 'Previously, your component called the following hooks:\n' + + '- useState\n' + + '- useReducer\n' + + 'But more recently, your component called the following:\n' + + '- useReducer\n' + + '- useState\n' + + 'This will lead to bugs and errors if not fixed. For more information, ' + + 'read the rules of hooks: https://fb.me/rules-of-hooks', + ]); + }); + + it('detects a bad hook order even if the component throws', () => { + const {useState, useReducer, useContext} = React; + + function App(props) { + /* eslint-disable no-unused-vars */ + if (props.flip) { + useState(0); + useReducer((s, a) => a, 0); + throw new Error('custom error'); + } else { + useReducer((s, a) => a, 0); + useState(0); + } + return null; + /* eslint-enable no-unused-vars */ + } + let root = ReactTestRenderer.create(); + expect(() => { + expect(() => root.update()).toThrow(); + }).toWarnDev( + [ + 'Warning: React just detected that you changed the order of hooks called across separate renders.\n' + + 'Previously, your component called the following hooks:\n' + + '- useState\n' + + '- useReducer\n' + + 'But more recently, your component called the following:\n' + + '- useReducer\n' + + '- useState\n' + + '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' + + ' in App (at **)', + 'Warning: React just detected that you changed the order of hooks called across separate renders.\n' + + 'Previously, your component called the following hooks:\n' + + '- useState\n' + + '- useReducer\n' + + 'But more recently, your component called the following:\n' + + '- useReducer\n' + + '- useState\n' + + 'This will lead to bugs and errors if not fixed. For more information, ' + + 'read the rules of hooks: https://fb.me/rules-of-hooks', + ], + {withoutStack: 1}, + ); }); }); From f09966d8b486fa1383eb6609abf617a85ead3d34 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Fri, 18 Jan 2019 19:52:59 +0000 Subject: [PATCH 19/35] an error stack in the warning --- .../react-reconciler/src/ReactFiberHooks.js | 43 +++++++++------ .../src/__tests__/ReactHooks-test.internal.js | 53 +++++-------------- 2 files changed, 41 insertions(+), 55 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index a28d2a3b2a693..f4a375bb945ab 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -82,6 +82,7 @@ const HookDevNames = [ let currentHookType: HookType | null = null; let currentHookMatches = []; +let currentHookMismatch = null; export type Hook = { _debugType?: HookType, @@ -201,35 +202,34 @@ function areHookInputsEqual( } function verifyHookCallOrder() { - let mismatched = false; - for (let i = 0; i < currentHookMatches.length; i++) { - if (currentHookMatches[i][0] !== currentHookMatches[i][1]) { - mismatched = true; - break; - } - } - if (mismatched) { + if (currentHookMismatch) { const previousOrder = currentHookMatches .map( // $FlowFixMe - we're certain that prev is a number - ([prev, next]) => '- ' + HookDevNames[prev], + ([prev, next]) => ' * ' + HookDevNames[prev], ) .join('\n'); const nextOrder = currentHookMatches .map( // $FlowFixMe - we're certain that next is a number - ([prev, next]) => '- ' + HookDevNames[next], + ([prev, next]) => ' * ' + HookDevNames[next], ) .join('\n'); + + const trace = currentHookMismatch; + warning( false, - 'React just detected that you changed the order of hooks called across separate renders.\n' + - 'Previously, your component called the following hooks:\n%s\n' + - 'But more recently, your component called the following:\n%s\n' + + 'React has detected a change in the order of hooks called.\n' + + 'On the first render, the following hooks were called:\n%s\n\n' + + 'On the most recent render, the following hooks were called:\n%s\n\n' + 'This will lead to bugs and errors if not fixed. ' + - 'For more information, read the rules of hooks: https://fb.me/rules-of-hooks', + 'For more information, read the rules of hooks: https://fb.me/rules-of-hooks\n\n' + + '%s\n' + + 'This error occured:\n', previousOrder, nextOrder, + trace, ); } } @@ -287,6 +287,7 @@ export function renderWithHooks( } verifyHookCallOrder(); currentHookMatches = []; + currentHookMismatch = null; } } while (didScheduleRenderPhaseUpdate); @@ -365,10 +366,10 @@ export function resetHooks(): void { if (__DEV__) { currentHookNameInDev = undefined; verifyHookCallOrder(); + currentHookMatches = []; + currentHookMismatch = null; } currentHookType = null; - currentHookMatches = []; - didScheduleRenderPhaseUpdate = false; renderPhaseUpdates = null; numberOfReRenders = -1; @@ -414,6 +415,16 @@ function cloneHook(hook: Hook): Hook { if (__DEV__) { // anything faster than this? currentHookMatches.push([currentHookType, hook._debugType]); + if (currentHookType !== hook._debugType) { + currentHookMismatch = + currentHookMismatch || + new Error('tracer').stack + // the first few lines are always 'react' code. + // todo - maybe show a nice tree of hooks? + .split('\n') + .slice(3, 10) + .join('\n'); + } nextHook._debugType = hook._debugType; } return nextHook; diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index af09d095f27b8..46e7c9111195d 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -615,36 +615,27 @@ describe('ReactHooks', () => { expect(() => { root.update(); }).toWarnDev([ - 'Warning: React just detected that you changed the order of hooks called across separate renders.\n' + - 'Previously, your component called the following hooks:\n' + - '- useState\n' + - '- useReducer\n' + - 'But more recently, your component called the following:\n' + - '- useReducer\n' + - '- useState\n' + - 'This will lead to bugs and errors if not fixed. For more information, ' + - 'read the rules of hooks: https://fb.me/rules-of-hooks', + 'Warning: React has detected a change in the order of hooks called.\n' + + 'On the first render, the following hooks were called:\n' + + ' * useState\n' + + ' * useReducer\n\n' + + 'On the most recent render, the following hooks were called:\n' + + ' * useReducer\n' + + ' * useState\n', ]); + // then a render that shouldn't warn root.update(); expect(() => { root.update(); }).toWarnDev([ - 'Warning: React just detected that you changed the order of hooks called across separate renders.\n' + - 'Previously, your component called the following hooks:\n' + - '- useState\n' + - '- useReducer\n' + - 'But more recently, your component called the following:\n' + - '- useReducer\n' + - '- useState\n' + - 'This will lead to bugs and errors if not fixed. For more information, ' + - 'read the rules of hooks: https://fb.me/rules-of-hooks', + 'Warning: React has detected a change in the order of hooks called', ]); }); it('detects a bad hook order even if the component throws', () => { - const {useState, useReducer, useContext} = React; + const {useState, useReducer} = React; function App(props) { /* eslint-disable no-unused-vars */ @@ -661,28 +652,12 @@ describe('ReactHooks', () => { } let root = ReactTestRenderer.create(); expect(() => { - expect(() => root.update()).toThrow(); + expect(() => root.update()).toThrow('custom error'); }).toWarnDev( + // TODO - this should log just once [ - 'Warning: React just detected that you changed the order of hooks called across separate renders.\n' + - 'Previously, your component called the following hooks:\n' + - '- useState\n' + - '- useReducer\n' + - 'But more recently, your component called the following:\n' + - '- useReducer\n' + - '- useState\n' + - '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' + - ' in App (at **)', - 'Warning: React just detected that you changed the order of hooks called across separate renders.\n' + - 'Previously, your component called the following hooks:\n' + - '- useState\n' + - '- useReducer\n' + - 'But more recently, your component called the following:\n' + - '- useReducer\n' + - '- useState\n' + - 'This will lead to bugs and errors if not fixed. For more information, ' + - 'read the rules of hooks: https://fb.me/rules-of-hooks', + 'Warning: React has detected a change in the order of hooks called', + 'Warning: React has detected a change in the order of hooks called', ], {withoutStack: 1}, ); From e3660691b95a49c5c02843a8c4010a0807b77755 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sat, 19 Jan 2019 14:47:19 +0000 Subject: [PATCH 20/35] lose currentHookMatches, fix a test --- .../react-reconciler/src/ReactFiberHooks.js | 55 ++++++++----------- .../src/__tests__/ReactHooks-test.internal.js | 19 +++---- 2 files changed, 33 insertions(+), 41 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index f4a375bb945ab..8e6f8b530837f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -81,7 +81,6 @@ const HookDevNames = [ ]; let currentHookType: HookType | null = null; -let currentHookMatches = []; let currentHookMismatch = null; export type Hook = { @@ -201,22 +200,21 @@ function areHookInputsEqual( return true; } -function verifyHookCallOrder() { - if (currentHookMismatch) { - const previousOrder = currentHookMatches - .map( - // $FlowFixMe - we're certain that prev is a number - ([prev, next]) => ' * ' + HookDevNames[prev], - ) - .join('\n'); - const nextOrder = currentHookMatches - .map( - // $FlowFixMe - we're certain that next is a number - ([prev, next]) => ' * ' + HookDevNames[next], - ) - .join('\n'); - - const trace = currentHookMismatch; +function flushHookMismatchWarnings() { + if (__DEV__ && currentHookMismatch) { + let current = firstCurrentHook; + let previousOrder = []; + while (current) { + previousOrder.push(' * ' + HookDevNames[current._debugType]); + current = current.next; + } + + let workInProgress = firstWorkInProgressHook; + let nextOrder = []; + while (workInProgress) { + nextOrder.push(' * ' + HookDevNames[workInProgress._debugType]); + workInProgress = workInProgress.next; + } warning( false, @@ -227,11 +225,12 @@ function verifyHookCallOrder() { 'For more information, read the rules of hooks: https://fb.me/rules-of-hooks\n\n' + '%s\n' + 'This error occured:\n', - previousOrder, - nextOrder, - trace, + previousOrder.join('\n'), + nextOrder.join('\n'), + currentHookMismatch, ); } + currentHookMismatch = null; } export function renderWithHooks( @@ -285,9 +284,7 @@ export function renderWithHooks( getComponentName(Component), ); } - verifyHookCallOrder(); - currentHookMatches = []; - currentHookMismatch = null; + flushHookMismatchWarnings(); } } while (didScheduleRenderPhaseUpdate); @@ -348,6 +345,7 @@ export function resetHooks(): void { if (!enableHooks) { return; } + flushHookMismatchWarnings(); // This is used to reset the state of this module when a component throws. // It's also called inside mountIndeterminateComponent if we determine the @@ -365,11 +363,9 @@ export function resetHooks(): void { if (__DEV__) { currentHookNameInDev = undefined; - verifyHookCallOrder(); - currentHookMatches = []; currentHookMismatch = null; + currentHookType = null; } - currentHookType = null; didScheduleRenderPhaseUpdate = false; renderPhaseUpdates = null; numberOfReRenders = -1; @@ -378,8 +374,7 @@ export function resetHooks(): void { function createHook(): Hook { let hook: Hook = __DEV__ ? { - // $FlowFixMe - we're certain we've set currentHookType by now - _debugType: currentHookType, + _debugType: ((currentHookType: any): HookType), memoizedState: null, baseState: null, @@ -413,8 +408,6 @@ function cloneHook(hook: Hook): Hook { }; if (__DEV__) { - // anything faster than this? - currentHookMatches.push([currentHookType, hook._debugType]); if (currentHookType !== hook._debugType) { currentHookMismatch = currentHookMismatch || @@ -425,7 +418,7 @@ function cloneHook(hook: Hook): Hook { .slice(3, 10) .join('\n'); } - nextHook._debugType = hook._debugType; + nextHook._debugType = currentHookType; } return nextHook; } diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 46e7c9111195d..450373a4b6ad4 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -617,18 +617,15 @@ describe('ReactHooks', () => { }).toWarnDev([ 'Warning: React has detected a change in the order of hooks called.\n' + 'On the first render, the following hooks were called:\n' + - ' * useState\n' + - ' * useReducer\n\n' + - 'On the most recent render, the following hooks were called:\n' + ' * useReducer\n' + - ' * useState\n', - ]); - - // then a render that shouldn't warn - root.update(); + ' * useState\n\n' + + 'On the most recent render, the following hooks were called:\n' + + ' * useState\n' + + ' * useReducer\n', + ]); expect(() => { - root.update(); + root.update(); }).toWarnDev([ 'Warning: React has detected a change in the order of hooks called', ]); @@ -654,7 +651,9 @@ describe('ReactHooks', () => { expect(() => { expect(() => root.update()).toThrow('custom error'); }).toWarnDev( - // TODO - this should log just once + // TODO - this should log just once + // it logs it twice because of the replay logic in the scheduler I think? + // must fix in some way, but pretty edge casey so letting this for now [ 'Warning: React has detected a change in the order of hooks called', 'Warning: React has detected a change in the order of hooks called', From d8c4236f3c1ad5b2ff033e165607f9284ccc17d8 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sat, 19 Jan 2019 15:01:44 +0000 Subject: [PATCH 21/35] tidy --- packages/react-reconciler/src/ReactFiberHooks.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 8e6f8b530837f..bfd513c38e0d3 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -363,8 +363,6 @@ export function resetHooks(): void { if (__DEV__) { currentHookNameInDev = undefined; - currentHookMismatch = null; - currentHookType = null; } didScheduleRenderPhaseUpdate = false; renderPhaseUpdates = null; @@ -743,7 +741,8 @@ export function useEffect( function useEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - if (currentHookType !== ImperativeHandleHook) { + if (currentHookType === null) { + // it could be an ImperativeHandleHook currentHookType = fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook; } From 4801b120c89adbd68ecdf2ff67cb03da9467b52c Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sat, 19 Jan 2019 15:13:53 +0000 Subject: [PATCH 22/35] clear the mismatch only in dev --- packages/react-reconciler/src/ReactFiberHooks.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index bfd513c38e0d3..41c0f857d2b0f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -229,8 +229,8 @@ function flushHookMismatchWarnings() { nextOrder.join('\n'), currentHookMismatch, ); - } - currentHookMismatch = null; + currentHookMismatch = null; + } } export function renderWithHooks( From c77e4489068434261c3ff6de778a9a569bb6591d Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sat, 19 Jan 2019 16:13:34 +0000 Subject: [PATCH 23/35] pass flow --- packages/react-reconciler/src/ReactFiberHooks.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 41c0f857d2b0f..badfbd66e4c87 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -205,14 +205,14 @@ function flushHookMismatchWarnings() { let current = firstCurrentHook; let previousOrder = []; while (current) { - previousOrder.push(' * ' + HookDevNames[current._debugType]); + previousOrder.push(' * ' + HookDevNames[((current._debugType:any):HookType)]); current = current.next; } let workInProgress = firstWorkInProgressHook; let nextOrder = []; while (workInProgress) { - nextOrder.push(' * ' + HookDevNames[workInProgress._debugType]); + nextOrder.push(' * ' + HookDevNames[((workInProgress._debugType:any):HookType)]); workInProgress = workInProgress.next; } @@ -416,7 +416,7 @@ function cloneHook(hook: Hook): Hook { .slice(3, 10) .join('\n'); } - nextHook._debugType = currentHookType; + nextHook._debugType = ((currentHookType:any):HookType); } return nextHook; } From b6fb3ca65c9a86ff88a94542338171b1910a310d Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 21 Jan 2019 18:09:26 +0000 Subject: [PATCH 24/35] side by side diff --- .../react-reconciler/src/ReactFiberHooks.js | 63 ++++++++++++++----- .../src/__tests__/ReactHooks-test.internal.js | 39 +++++++----- 2 files changed, 71 insertions(+), 31 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index badfbd66e4c87..2c5f7ca95e49e 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -81,6 +81,8 @@ const HookDevNames = [ ]; let currentHookType: HookType | null = null; +// the first instance of a hook mismatch in a component, +// represented by a portion of it's stacktrace let currentHookMismatch = null; export type Hook = { @@ -200,37 +202,70 @@ function areHookInputsEqual( return true; } +// till we have String::padEnd, a small function to +// right-pad strings with spaces till a minimum length +function padEndSpaces(string: string, length: number) { + if (string.length >= length) { + return string; + } else { + return string + ' ' + new Array(length - string.length).join(' '); + } +} + function flushHookMismatchWarnings() { + // we'll show the diff of the low level hooks, + // and a stack trace so the dev can locate where + // the first mismatch is coming from if (__DEV__ && currentHookMismatch) { + const hookStackDiff = []; let current = firstCurrentHook; let previousOrder = []; while (current) { - previousOrder.push(' * ' + HookDevNames[((current._debugType:any):HookType)]); + previousOrder.push(HookDevNames[((current._debugType: any): HookType)]); current = current.next; } - let workInProgress = firstWorkInProgressHook; let nextOrder = []; while (workInProgress) { - nextOrder.push(' * ' + HookDevNames[((workInProgress._debugType:any):HookType)]); + nextOrder.push( + HookDevNames[((workInProgress._debugType: any): HookType)], + ); workInProgress = workInProgress.next; } - + // some bookkeeping for formatting the output table + const columnLength = Math.max( + ' Previous render'.length, + ...previousOrder.map(hook => hook.length), + ); + const hookStackHeader = + padEndSpaces(' Previous render', columnLength) + + ' Next render\n' + + padEndSpaces(' ---', columnLength) + + ' ---'; + const hookStackLength = Math.max(previousOrder.length, nextOrder.length); + for (let i = 0; i < hookStackLength; i++) { + hookStackDiff.push( + (previousOrder[i] !== nextOrder[i] ? '× ' : ' ') + + padEndSpaces(previousOrder[i], columnLength) + + ' ' + + nextOrder[i], + ); + } warning( false, 'React has detected a change in the order of hooks called.\n' + - 'On the first render, the following hooks were called:\n%s\n\n' + - 'On the most recent render, the following hooks were called:\n%s\n\n' + - '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' + '%s\n' + - 'This error occured:\n', - previousOrder.join('\n'), - nextOrder.join('\n'), + '%s\n\n' + + 'Error trace:\n' + + '%s\n' + + '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', + hookStackHeader, + hookStackDiff.join('\n'), currentHookMismatch, ); currentHookMismatch = null; - } + } } export function renderWithHooks( @@ -410,13 +445,11 @@ function cloneHook(hook: Hook): Hook { currentHookMismatch = currentHookMismatch || new Error('tracer').stack - // the first few lines are always 'react' code. - // todo - maybe show a nice tree of hooks? .split('\n') .slice(3, 10) .join('\n'); } - nextHook._debugType = ((currentHookType:any):HookType); + nextHook._debugType = ((currentHookType: any): HookType); } return nextHook; } diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 450373a4b6ad4..c184864784f06 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -599,14 +599,17 @@ describe('ReactHooks', () => { it('warns on using differently ordered hooks on subsequent renders', () => { const {useState, useReducer} = React; + function useCustomHook() { + return useState(0); + } function App(props) { /* eslint-disable no-unused-vars */ if (props.flip) { - useState(0); + useCustomHook(0); useReducer((s, a) => a, 0); } else { useReducer((s, a) => a, 0); - useState(0); + useCustomHook(0); } return null; /* eslint-enable no-unused-vars */ @@ -616,13 +619,11 @@ describe('ReactHooks', () => { root.update(); }).toWarnDev([ 'Warning: React has detected a change in the order of hooks called.\n' + - 'On the first render, the following hooks were called:\n' + - ' * useReducer\n' + - ' * useState\n\n' + - 'On the most recent render, the following hooks were called:\n' + - ' * useState\n' + - ' * useReducer\n', - ]); + ' Previous render Next render\n' + + ' --- ---\n' + + '× useReducer useState\n' + + '× useState useReducer\n', + ]); expect(() => { root.update(); @@ -633,16 +634,18 @@ describe('ReactHooks', () => { 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) { - useState(0); + useCustomHook(); useReducer((s, a) => a, 0); throw new Error('custom error'); } else { useReducer((s, a) => a, 0); - useState(0); + useCustomHook(); } return null; /* eslint-enable no-unused-vars */ @@ -651,11 +654,15 @@ describe('ReactHooks', () => { expect(() => { expect(() => root.update()).toThrow('custom error'); }).toWarnDev( - // TODO - this should log just once - // it logs it twice because of the replay logic in the scheduler I think? - // must fix in some way, but pretty edge casey so letting this for now + // TODO - this should log just once + // it logs it twice because of the replay logic in the scheduler I think? + // must fix in some way, but pretty edge casey so letting this for now [ - 'Warning: React has detected a change in the order of hooks called', + 'Warning: React has detected a change in the order of hooks called.\n' + + ' Previous render Next render\n' + + ' --- ---\n' + + '× useReducer useState\n' + + '× useState useReducer\n', 'Warning: React has detected a change in the order of hooks called', ], {withoutStack: 1}, From 055ebf6e15f86395819e6f8ba152bda05654e115 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 21 Jan 2019 18:35:21 +0000 Subject: [PATCH 25/35] tweak warning --- packages/react-reconciler/src/ReactFiberHooks.js | 11 ++++++----- .../src/__tests__/ReactHooks-test.internal.js | 12 ++++++------ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 2c5f7ca95e49e..e60b304d1a994 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -245,7 +245,7 @@ function flushHookMismatchWarnings() { const hookStackLength = Math.max(previousOrder.length, nextOrder.length); for (let i = 0; i < hookStackLength; i++) { hookStackDiff.push( - (previousOrder[i] !== nextOrder[i] ? '× ' : ' ') + + (previousOrder[i] !== nextOrder[i] ? '> ' : ' ') + padEndSpaces(previousOrder[i], columnLength) + ' ' + nextOrder[i], @@ -253,13 +253,14 @@ function flushHookMismatchWarnings() { } warning( false, - 'React has detected a change in the order of hooks called.\n' + + 'React has detected a change in the order of hooks called by %s.\n' + '%s\n' + '%s\n\n' + 'Error trace:\n' + - '%s\n' + + '%s\n\n' + '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', + 'For more information, read the rules of hooks: https://fb.me/rules-of-hooks\n', + getComponentName(currentlyRenderingFiber.type), hookStackHeader, hookStackDiff.join('\n'), currentHookMismatch, @@ -446,7 +447,7 @@ function cloneHook(hook: Hook): Hook { currentHookMismatch || new Error('tracer').stack .split('\n') - .slice(3, 10) + .slice(4) .join('\n'); } nextHook._debugType = ((currentHookType: any): HookType); diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index c184864784f06..65e891b09d9e9 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -618,11 +618,11 @@ describe('ReactHooks', () => { expect(() => { root.update(); }).toWarnDev([ - 'Warning: React has detected a change in the order of hooks called.\n' + + 'Warning: React has detected a change in the order of hooks called by App.\n' + ' Previous render Next render\n' + ' --- ---\n' + - '× useReducer useState\n' + - '× useState useReducer\n', + '> useReducer useState\n' + + '> useState useReducer\n', ]); expect(() => { @@ -658,11 +658,11 @@ describe('ReactHooks', () => { // it logs it twice because of the replay logic in the scheduler I think? // must fix in some way, but pretty edge casey so letting this for now [ - 'Warning: React has detected a change in the order of hooks called.\n' + + 'Warning: React has detected a change in the order of hooks called by App.\n' + ' Previous render Next render\n' + ' --- ---\n' + - '× useReducer useState\n' + - '× useState useReducer\n', + '> useReducer useState\n' + + '> useState useReducer\n', 'Warning: React has detected a change in the order of hooks called', ], {withoutStack: 1}, From e06b72bca9d741dbbc7d6c04abfb1b1ed8aa3660 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 21 Jan 2019 19:19:03 +0000 Subject: [PATCH 26/35] pass flow --- .../react-reconciler/src/ReactFiberHooks.js | 55 +++++++++++-------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 1b0c2b0143122..a69a78a40d964 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -86,7 +86,6 @@ let currentHookType: HookType | null = null; let currentHookMismatch = null; export type Hook = { - _debugType?: HookType, memoizedState: any, baseState: any, @@ -96,6 +95,10 @@ export type Hook = { next: Hook | null, }; +type HookDev = Hook & { + _debugType: HookType, +}; + type Effect = { tag: HookEffectTag, create: () => mixed, @@ -221,15 +224,13 @@ function flushHookMismatchWarnings() { let current = firstCurrentHook; let previousOrder = []; while (current) { - previousOrder.push(HookDevNames[((current._debugType: any): HookType)]); + previousOrder.push(HookDevNames[((current: any): HookDev)._debugType]); current = current.next; } let workInProgress = firstWorkInProgressHook; let nextOrder = []; while (workInProgress) { - nextOrder.push( - HookDevNames[((workInProgress._debugType: any): HookType)], - ); + nextOrder.push(HookDevNames[((workInProgress: any): HookDev)._debugType]); workInProgress = workInProgress.next; } // some bookkeeping for formatting the output table @@ -260,7 +261,7 @@ function flushHookMismatchWarnings() { '%s\n\n' + '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', - getComponentName(currentlyRenderingFiber.type), + getComponentName(((currentlyRenderingFiber: any): Fiber).type), hookStackHeader, hookStackDiff.join('\n'), currentHookMismatch, @@ -431,26 +432,34 @@ function createHook(): Hook { } function cloneHook(hook: Hook): Hook { - let nextHook: Hook = { - memoizedState: hook.memoizedState, + let nextHook: Hook = __DEV__ + ? { + _debugType: ((currentHookType: any): HookType), + memoizedState: hook.memoizedState, - baseState: hook.baseState, - queue: hook.queue, - baseUpdate: hook.baseUpdate, + baseState: hook.baseState, + queue: hook.queue, + baseUpdate: hook.baseUpdate, - next: null, - }; + next: null, + } + : { + memoizedState: hook.memoizedState, - if (__DEV__) { - if (currentHookType !== hook._debugType) { - currentHookMismatch = - currentHookMismatch || - new Error('tracer').stack - .split('\n') - .slice(4) - .join('\n'); + baseState: hook.baseState, + queue: hook.queue, + baseUpdate: hook.baseUpdate, + + next: null, + }; + + if (__DEV__ && !currentHookMismatch) { + if (currentHookType !== ((hook: any): HookDev)._debugType) { + currentHookMismatch = new Error('tracer').stack + .split('\n') + .slice(4) + .join('\n'); } - nextHook._debugType = ((currentHookType: any): HookType); } return nextHook; } @@ -775,7 +784,7 @@ export function useEffect( function useEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - if (currentHookType === null) { + if (currentHookType !== ImperativeHandleHook) { // it could be an ImperativeHandleHook currentHookType = fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook; From 190e9b1bf71776b7531cd6f903255a1666554de0 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 21 Jan 2019 20:25:26 +0000 Subject: [PATCH 27/35] dedupe warnings per fiber, nits --- .../react-reconciler/src/ReactFiberHooks.js | 106 ++++++++++-------- .../src/__tests__/ReactHooks-test.internal.js | 9 +- 2 files changed, 63 insertions(+), 52 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index a69a78a40d964..7427057a939ee 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -84,6 +84,8 @@ let currentHookType: HookType | null = null; // the first instance of a hook mismatch in a component, // represented by a portion of it's stacktrace let currentHookMismatch = null; +const PossiblyWeakSet = typeof WeakSet === 'function' ? WeakSet : Set; +const hookMismatchedFibers = new PossiblyWeakSet(); export type Hook = { memoizedState: any, @@ -219,53 +221,67 @@ function flushHookMismatchWarnings() { // we'll show the diff of the low level hooks, // and a stack trace so the dev can locate where // the first mismatch is coming from - if (__DEV__ && currentHookMismatch) { - const hookStackDiff = []; - let current = firstCurrentHook; - let previousOrder = []; - while (current) { - previousOrder.push(HookDevNames[((current: any): HookDev)._debugType]); - current = current.next; - } - let workInProgress = firstWorkInProgressHook; - let nextOrder = []; - while (workInProgress) { - nextOrder.push(HookDevNames[((workInProgress: any): HookDev)._debugType]); - workInProgress = workInProgress.next; - } - // some bookkeeping for formatting the output table - const columnLength = Math.max( - ' Previous render'.length, - ...previousOrder.map(hook => hook.length), - ); - const hookStackHeader = - padEndSpaces(' Previous render', columnLength) + - ' Next render\n' + - padEndSpaces(' ---', columnLength) + - ' ---'; - const hookStackLength = Math.max(previousOrder.length, nextOrder.length); - for (let i = 0; i < hookStackLength; i++) { - hookStackDiff.push( - (previousOrder[i] !== nextOrder[i] ? '> ' : ' ') + - padEndSpaces(previousOrder[i], columnLength) + - ' ' + - nextOrder[i], + if (__DEV__) { + if ( + currentHookMismatch && + !hookMismatchedFibers.has(currentlyRenderingFiber) + ) { + let componentName = getComponentName( + ((currentlyRenderingFiber: any): Fiber).type, + ); + hookMismatchedFibers.add(currentlyRenderingFiber); + const hookStackDiff = []; + let current = firstCurrentHook; + let previousOrder = []; + while (current !== null) { + previousOrder.push(HookDevNames[((current: any): HookDev)._debugType]); + current = current.next; + } + let workInProgress = firstWorkInProgressHook; + let nextOrder = []; + while (workInProgress !== null) { + nextOrder.push( + HookDevNames[((workInProgress: any): HookDev)._debugType], + ); + workInProgress = workInProgress.next; + } + // some bookkeeping for formatting the output table + const columnLength = Math.max.apply( + null, + previousOrder + .map(hook => hook.length) + .concat(' Previous render'.length), + ); + + const hookStackHeader = + padEndSpaces(' Previous render', columnLength) + + ' Next render\n' + + padEndSpaces(' ---', columnLength) + + ' ---'; + const hookStackLength = Math.max(previousOrder.length, nextOrder.length); + for (let i = 0; i < hookStackLength; i++) { + hookStackDiff.push( + (previousOrder[i] !== nextOrder[i] ? '> ' : ' ') + + padEndSpaces(previousOrder[i], columnLength) + + ' ' + + nextOrder[i], + ); + } + warning( + false, + 'React has detected a change in the order of hooks called by %s.\n' + + '%s\n' + + '%s\n\n' + + 'The first mismatched hook was at:\n' + + '%s\n\n' + + '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', + componentName, + hookStackHeader, + hookStackDiff.join('\n'), + currentHookMismatch, ); } - warning( - false, - 'React has detected a change in the order of hooks called by %s.\n' + - '%s\n' + - '%s\n\n' + - 'Error trace:\n' + - '%s\n\n' + - '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', - getComponentName(((currentlyRenderingFiber: any): Fiber).type), - hookStackHeader, - hookStackDiff.join('\n'), - currentHookMismatch, - ); currentHookMismatch = null; } } diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 9b24dedb03c98..da9a646b3f88c 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -718,18 +718,13 @@ describe('ReactHooks', () => { expect(() => { expect(() => root.update()).toThrow('custom error'); }).toWarnDev( - // TODO - this should log just once - // it logs it twice because of the replay logic in the scheduler I think? - // must fix in some way, but pretty edge casey so letting this for now [ 'Warning: React has detected a change in the order of hooks called by App.\n' + ' Previous render Next render\n' + ' --- ---\n' + '> useReducer useState\n' + - '> useState useReducer\n', - 'Warning: React has detected a change in the order of hooks called', - ], - {withoutStack: 1}, + '> useState useReducer\n' + ] ); }); From f37f4fd91e5ce2ecc894147e8861ca81fb0f173c Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 21 Jan 2019 20:48:08 +0000 Subject: [PATCH 28/35] better format --- .../react-reconciler/src/ReactFiberHooks.js | 31 +++++++++++------- .../src/__tests__/ReactHooks-test.internal.js | 32 ++++++++++--------- 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 7427057a939ee..b18906aecde45 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -250,35 +250,42 @@ function flushHookMismatchWarnings() { null, previousOrder .map(hook => hook.length) - .concat(' Previous render'.length), + .concat(' Previous render'.length), ); - const hookStackHeader = - padEndSpaces(' Previous render', columnLength) + - ' Next render\n' + - padEndSpaces(' ---', columnLength) + - ' ---'; + let hookStackHeader = + padEndSpaces(' Previous render', columnLength) + ' Next render\n'; + const hookStackWidth = hookStackHeader.length; + hookStackHeader += ' ' + new Array(hookStackWidth - 2).join('-'); + let hookStackFooter = ' ' + new Array(hookStackWidth - 2).join('^'); + const hookStackLength = Math.max(previousOrder.length, nextOrder.length); for (let i = 0; i < hookStackLength; i++) { hookStackDiff.push( - (previousOrder[i] !== nextOrder[i] ? '> ' : ' ') + + padEndSpaces(i + 1 + '. ', 3) + padEndSpaces(previousOrder[i], columnLength) + ' ' + nextOrder[i], ); + if (previousOrder[i] !== nextOrder[i]) { + break; + } } warning( false, - 'React has detected a change in the order of hooks called by %s.\n' + + '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' + '%s\n' + + '%s\n' + + '%s\n' + + 'The first Hook type mismatch occured at:\n' + '%s\n\n' + - 'The first mismatched hook was at:\n' + - '%s\n\n' + - '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', + 'This error occurred in the following component:', componentName, hookStackHeader, hookStackDiff.join('\n'), + hookStackFooter, currentHookMismatch, ); } diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index da9a646b3f88c..5f909fa78401f 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -682,17 +682,19 @@ describe('ReactHooks', () => { expect(() => { root.update(); }).toWarnDev([ - 'Warning: React has detected a change in the order of hooks called by App.\n' + - ' Previous render Next render\n' + - ' --- ---\n' + - '> useReducer useState\n' + - '> useState useReducer\n', + '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' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^', ]); expect(() => { root.update(); }).toWarnDev([ - 'Warning: React has detected a change in the order of hooks called', + 'Warning: React has detected a change in the order of Hooks called', ]); }); @@ -717,15 +719,15 @@ describe('ReactHooks', () => { 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.\n' + - ' Previous render Next render\n' + - ' --- ---\n' + - '> useReducer useState\n' + - '> useState useReducer\n' - ] - ); + }).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' + + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^', + ]); }); it('double-invokes components with Hooks in Strict Mode', () => { From 3e8a2170509dcc27d7290abeb54c724ddbe08bec Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Mon, 21 Jan 2019 21:02:08 +0000 Subject: [PATCH 29/35] nit --- packages/react-reconciler/src/ReactFiberHooks.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index ab1601c40e289..18551e71c03eb 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -476,12 +476,14 @@ function cloneHook(hook: Hook): Hook { next: null, }; - if (__DEV__ && !currentHookMismatch) { - if (currentHookType !== ((hook: any): HookDev)._debugType) { - currentHookMismatch = new Error('tracer').stack - .split('\n') - .slice(4) - .join('\n'); + if (__DEV__) { + if (!currentHookMismatch) { + if (currentHookType !== ((hook: any): HookDev)._debugType) { + currentHookMismatch = new Error('tracer').stack + .split('\n') + .slice(4) + .join('\n'); + } } } return nextHook; From f4936758eee46c46acc3b3cc098d4da590d295fd Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Tue, 22 Jan 2019 10:32:02 +0000 Subject: [PATCH 30/35] fix bad merge, pass flow --- .../react-reconciler/src/ReactFiberHooks.js | 4 +- .../src/__tests__/ReactHooks-test.internal.js | 220 ------------------ 2 files changed, 2 insertions(+), 222 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index ed2a154dbce72..a5cc6058cfd80 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -224,12 +224,12 @@ function flushHookMismatchWarnings() { if (__DEV__) { if ( currentHookMismatch && - !hookMismatchedFibers.has(currentlyRenderingFiber) + !hookMismatchedFibers.has(((currentlyRenderingFiber:any):Fiber)) ) { let componentName = getComponentName( ((currentlyRenderingFiber: any): Fiber).type, ); - hookMismatchedFibers.add(currentlyRenderingFiber); + hookMismatchedFibers.add(((currentlyRenderingFiber:any):Fiber)); const hookStackDiff = []; let current = firstCurrentHook; let previousOrder = []; diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 3ee18aee600d8..c9f845880fc38 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -769,224 +769,4 @@ describe('ReactHooks', () => { ]); }); - it('double-invokes components with Hooks in Strict Mode', () => { - ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = true; - - const {useState, StrictMode} = React; - let renderCount = 0; - - function NoHooks() { - renderCount++; - return
; - } - - function HasHooks() { - useState(0); - renderCount++; - return
; - } - - const FwdRef = React.forwardRef((props, ref) => { - renderCount++; - return
; - }); - - const FwdRefHasHooks = React.forwardRef((props, ref) => { - useState(0); - renderCount++; - return
; - }); - - const Memo = React.memo(props => { - renderCount++; - return
; - }); - - const MemoHasHooks = React.memo(props => { - useState(0); - renderCount++; - return
; - }); - - function Factory() { - return { - state: {}, - render() { - renderCount++; - return
; - }, - }; - } - - let renderer = ReactTestRenderer.create(null); - - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(1); - - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(1); - - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(1); - - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(__DEV__ ? 2 : 1); // Treated like a class - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(__DEV__ ? 2 : 1); // Treated like a class - - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(__DEV__ ? 2 : 1); // Has Hooks - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(__DEV__ ? 2 : 1); // Has Hooks - - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(__DEV__ ? 2 : 1); // Has Hooks - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(__DEV__ ? 2 : 1); // Has Hooks - - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(__DEV__ ? 2 : 1); // Has Hooks - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(__DEV__ ? 2 : 1); // Has Hooks - }); - - it('double-invokes useMemo in DEV StrictMode despite []', () => { - ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = true; - const {useMemo, StrictMode} = React; - - let useMemoCount = 0; - function BadUseMemo() { - useMemo(() => { - useMemoCount++; - }, []); - return
; - } - - useMemoCount = 0; - ReactTestRenderer.create( - - - , - ); - expect(useMemoCount).toBe(__DEV__ ? 2 : 1); // Has Hooks - }); - }); From a985e720ba635ee8b5a695a770d92ee12009ad04 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Tue, 22 Jan 2019 10:45:21 +0000 Subject: [PATCH 31/35] lint --- packages/react-reconciler/src/ReactFiberHooks.js | 4 ++-- .../src/__tests__/ReactHooks-test.internal.js | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index a5cc6058cfd80..f09829edbcbda 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -224,12 +224,12 @@ function flushHookMismatchWarnings() { if (__DEV__) { if ( currentHookMismatch && - !hookMismatchedFibers.has(((currentlyRenderingFiber:any):Fiber)) + !hookMismatchedFibers.has(((currentlyRenderingFiber: any): Fiber)) ) { let componentName = getComponentName( ((currentlyRenderingFiber: any): Fiber).type, ); - hookMismatchedFibers.add(((currentlyRenderingFiber:any):Fiber)); + hookMismatchedFibers.add(((currentlyRenderingFiber: any): Fiber)); const hookStackDiff = []; let current = firstCurrentHook; let previousOrder = []; diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index c9f845880fc38..c827cbabfd147 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -768,5 +768,4 @@ describe('ReactHooks', () => { ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^', ]); }); - }); From 1621a2d9416776d3950a80a9631c7e310b13508c Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Tue, 22 Jan 2019 10:53:02 +0000 Subject: [PATCH 32/35] missing hooktype enum --- packages/react-reconciler/src/ReactFiberHooks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index f09829edbcbda..ffe9f33be2295 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -54,7 +54,7 @@ type UpdateQueue = { eagerState: S | null, }; -type HookType = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8; +type HookType = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9; const StateHook = 0; const ReducerHook = 1; From 5ecda4c791730e57e56956632a4003da220b76d0 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Tue, 22 Jan 2019 21:46:23 +0000 Subject: [PATCH 33/35] merge currentHookType/currentHookNameInDev, fix nits --- .../react-reconciler/src/ReactFiberHooks.js | 242 +++++++++--------- .../src/__tests__/ReactHooks-test.internal.js | 7 +- 2 files changed, 119 insertions(+), 130 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index ffe9f33be2295..4f594691d0ce7 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -54,38 +54,27 @@ type UpdateQueue = { eagerState: S | null, }; -type HookType = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9; - -const StateHook = 0; -const ReducerHook = 1; -const ContextHook = 2; -const RefHook = 3; -const EffectHook = 4; -const LayoutEffectHook = 5; -const CallbackHook = 6; -const MemoHook = 7; -const ImperativeHandleHook = 8; -const DebugValueHook = 9; - -const HookDevNames = [ - 'useState', - 'useReducer', - 'useContext', - 'useRef', - 'useEffect', - 'useLayoutEffect', - 'useCallback', - 'useMemo', - 'useImperativeHandle', - 'useDebugValue', -]; - -let currentHookType: HookType | null = null; +opaque type HookType = string; + +const StateHook: HookType = 'useState'; +const ReducerHook: HookType = 'useReducer'; +const ContextHook: HookType = 'useContext'; +const RefHook: HookType = 'useRef'; +const EffectHook: HookType = 'useEffect'; +const LayoutEffectHook: HookType = 'useLayoutEffect'; +const CallbackHook: HookType = 'useCallback'; +const MemoHook: HookType = 'useMemo'; +const ImperativeHandleHook: HookType = 'useImperativeHandle'; +const DebugValueHook: HookType = 'useDebugValue'; + // the first instance of a hook mismatch in a component, // represented by a portion of it's stacktrace let currentHookMismatch = null; -const PossiblyWeakSet = typeof WeakSet === 'function' ? WeakSet : Set; -const hookMismatchedFibers = new PossiblyWeakSet(); + +let didWarnAboutMismatchedHooksForComponent; +if (__DEV__) { + didWarnAboutMismatchedHooksForComponent = new Set(); +} export type Hook = { memoizedState: any, @@ -155,7 +144,7 @@ let numberOfReRenders: number = -1; const RE_RENDER_LIMIT = 25; // In DEV, this is the name of the currently executing primitive hook -let currentHookNameInDev: ?string; +let currentHookNameInDev: ?HookType = null; function resolveCurrentlyRenderingFiber(): Fiber { invariant( @@ -210,10 +199,12 @@ function areHookInputsEqual( // till we have String::padEnd, a small function to // right-pad strings with spaces till a minimum length function padEndSpaces(string: string, length: number) { - if (string.length >= length) { - return string; - } else { - return string + ' ' + new Array(length - string.length).join(' '); + if (__DEV__) { + if (string.length >= length) { + return string; + } else { + return string + ' ' + new Array(length - string.length).join(' '); + } } } @@ -222,74 +213,75 @@ function flushHookMismatchWarnings() { // and a stack trace so the dev can locate where // the first mismatch is coming from if (__DEV__) { - if ( - currentHookMismatch && - !hookMismatchedFibers.has(((currentlyRenderingFiber: any): Fiber)) - ) { + if (currentHookMismatch) { let componentName = getComponentName( ((currentlyRenderingFiber: any): Fiber).type, ); - hookMismatchedFibers.add(((currentlyRenderingFiber: any): Fiber)); - const hookStackDiff = []; - let current = firstCurrentHook; - let previousOrder = []; - while (current !== null) { - previousOrder.push(HookDevNames[((current: any): HookDev)._debugType]); - current = current.next; - } - let workInProgress = firstWorkInProgressHook; - let nextOrder = []; - while (workInProgress !== null) { - nextOrder.push( - HookDevNames[((workInProgress: any): HookDev)._debugType], + if (!didWarnAboutMismatchedHooksForComponent.has(componentName)) { + didWarnAboutMismatchedHooksForComponent.add(componentName); + const hookStackDiff = []; + let current = firstCurrentHook; + let previousOrder = []; + while (current !== null) { + previousOrder.push(((current: any): HookDev)._debugType); + current = current.next; + } + let workInProgress = firstWorkInProgressHook; + let nextOrder = []; + while (workInProgress !== null) { + nextOrder.push(((workInProgress: any): HookDev)._debugType); + workInProgress = workInProgress.next; + } + // some bookkeeping for formatting the output table + const columnLength = Math.max.apply( + null, + previousOrder + .map(hook => hook.length) + .concat(' Previous render'.length), ); - workInProgress = workInProgress.next; - } - // some bookkeeping for formatting the output table - const columnLength = Math.max.apply( - null, - previousOrder - .map(hook => hook.length) - .concat(' Previous render'.length), - ); - let hookStackHeader = - padEndSpaces(' Previous render', columnLength) + ' Next render\n'; - const hookStackWidth = hookStackHeader.length; - hookStackHeader += ' ' + new Array(hookStackWidth - 2).join('-'); - let hookStackFooter = ' ' + new Array(hookStackWidth - 2).join('^'); - - const hookStackLength = Math.max(previousOrder.length, nextOrder.length); - for (let i = 0; i < hookStackLength; i++) { - hookStackDiff.push( - padEndSpaces(i + 1 + '. ', 3) + - padEndSpaces(previousOrder[i], columnLength) + - ' ' + - nextOrder[i], + let hookStackHeader = + ((padEndSpaces(' Previous render', columnLength): any): string) + + ' Next render\n'; + const hookStackWidth = hookStackHeader.length; + hookStackHeader += ' ' + new Array(hookStackWidth - 2).join('-'); + let hookStackFooter = ' ' + new Array(hookStackWidth - 2).join('^'); + + const hookStackLength = Math.max( + previousOrder.length, + nextOrder.length, ); - if (previousOrder[i] !== nextOrder[i]) { - break; + for (let i = 0; i < hookStackLength; i++) { + hookStackDiff.push( + ((padEndSpaces(i + 1 + '. ', 3): any): string) + + ((padEndSpaces(previousOrder[i], columnLength): any): string) + + ' ' + + nextOrder[i], + ); + if (previousOrder[i] !== nextOrder[i]) { + break; + } } + 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' + + '%s\n' + + '%s\n' + + '%s\n' + + 'The first Hook type mismatch occured at:\n' + + '%s\n\n' + + 'This error occurred in the following component:', + componentName, + hookStackHeader, + hookStackDiff.join('\n'), + hookStackFooter, + currentHookMismatch, + ); } - 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' + - '%s\n' + - '%s\n' + - '%s\n' + - 'The first Hook type mismatch occured at:\n' + - '%s\n\n' + - 'This error occurred in the following component:', - componentName, - hookStackHeader, - hookStackDiff.join('\n'), - hookStackFooter, - currentHookMismatch, - ); + currentHookMismatch = null; } - currentHookMismatch = null; } } @@ -372,7 +364,7 @@ export function renderWithHooks( componentUpdateQueue = null; if (__DEV__) { - currentHookNameInDev = undefined; + currentHookNameInDev = null; } // These were reset above @@ -405,7 +397,9 @@ export function resetHooks(): void { if (!enableHooks) { return; } - flushHookMismatchWarnings(); + if (__DEV__) { + flushHookMismatchWarnings(); + } // This is used to reset the state of this module when a component throws. // It's also called inside mountIndeterminateComponent if we determine the @@ -422,8 +416,9 @@ export function resetHooks(): void { componentUpdateQueue = null; if (__DEV__) { - currentHookNameInDev = undefined; + currentHookNameInDev = null; } + didScheduleRenderPhaseUpdate = false; renderPhaseUpdates = null; numberOfReRenders = -1; @@ -432,7 +427,7 @@ export function resetHooks(): void { function createHook(): Hook { let hook: Hook = __DEV__ ? { - _debugType: ((currentHookType: any): HookType), + _debugType: ((currentHookNameInDev: any): HookType), memoizedState: null, baseState: null, @@ -457,7 +452,7 @@ function createHook(): Hook { function cloneHook(hook: Hook): Hook { let nextHook: Hook = __DEV__ ? { - _debugType: ((currentHookType: any): HookType), + _debugType: ((currentHookNameInDev: any): HookType), memoizedState: hook.memoizedState, baseState: hook.baseState, @@ -478,7 +473,7 @@ function cloneHook(hook: Hook): Hook { if (__DEV__) { if (!currentHookMismatch) { - if (currentHookType !== ((hook: any): HookDev)._debugType) { + if (currentHookNameInDev !== ((hook: any): HookDev)._debugType) { currentHookMismatch = new Error('tracer').stack .split('\n') .slice(4) @@ -549,10 +544,9 @@ export function useContext( observedBits: void | number | boolean, ): T { if (__DEV__) { - currentHookNameInDev = HookDevNames[ContextHook]; - currentHookType = ContextHook; + currentHookNameInDev = ContextHook; createWorkInProgressHook(); - currentHookType = null; + currentHookNameInDev = null; } // Ensure we're in a function component (class components support only the // .unstable_read() form) @@ -564,7 +558,7 @@ export function useState( initialState: (() => S) | S, ): [S, Dispatch>] { if (__DEV__) { - currentHookNameInDev = HookDevNames[StateHook]; + currentHookNameInDev = StateHook; } return useReducer( basicStateReducer, @@ -580,13 +574,12 @@ export function useReducer( ): [S, Dispatch] { if (__DEV__) { if (reducer !== basicStateReducer) { - currentHookNameInDev = HookDevNames[ReducerHook]; + currentHookNameInDev = ReducerHook; } } - currentHookType = reducer === basicStateReducer ? StateHook : ReducerHook; let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber()); workInProgressHook = createWorkInProgressHook(); - currentHookType = null; + currentHookNameInDev = null; let queue: UpdateQueue | null = (workInProgressHook.queue: any); if (queue !== null) { // Already have a queue, so this is an update. @@ -765,9 +758,11 @@ function pushEffect(tag, create, destroy, deps) { export function useRef(initialValue: T): {current: T} { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - currentHookType = RefHook; + if (__DEV__) { + currentHookNameInDev = RefHook; + } workInProgressHook = createWorkInProgressHook(); - currentHookType = null; + currentHookNameInDev = null; let ref; if (workInProgressHook.memoizedState === null) { @@ -787,7 +782,9 @@ export function useLayoutEffect( deps: Array | void | null, ): void { if (__DEV__) { - currentHookNameInDev = HookDevNames[LayoutEffectHook]; + if (currentHookNameInDev !== ImperativeHandleHook) { + currentHookNameInDev = LayoutEffectHook; + } } useEffectImpl(UpdateEffect, UnmountMutation | MountLayout, create, deps); } @@ -797,7 +794,7 @@ export function useEffect( deps: Array | void | null, ): void { if (__DEV__) { - currentHookNameInDev = HookDevNames[EffectHook]; + currentHookNameInDev = EffectHook; } useEffectImpl( UpdateEffect | PassiveEffect, @@ -809,13 +806,7 @@ export function useEffect( function useEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - if (currentHookType !== ImperativeHandleHook) { - // it could be an ImperativeHandleHook - currentHookType = - fiberEffectTag === UpdateEffect ? EffectHook : LayoutEffectHook; - } workInProgressHook = createWorkInProgressHook(); - currentHookType = null; const nextDeps = deps === undefined ? null : deps; let destroy = null; @@ -826,6 +817,7 @@ function useEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { const prevDeps = prevEffect.deps; if (areHookInputsEqual(nextDeps, prevDeps)) { pushEffect(NoHookEffect, create, destroy, nextDeps); + currentHookNameInDev = null; return; } } @@ -838,6 +830,7 @@ function useEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { destroy, nextDeps, ); + currentHookNameInDev = null; } export function useImperativeHandle( @@ -846,7 +839,7 @@ export function useImperativeHandle( deps: Array | void | null, ): void { if (__DEV__) { - currentHookNameInDev = HookDevNames[ImperativeHandleHook]; + currentHookNameInDev = ImperativeHandleHook; warning( typeof create === 'function', 'Expected useImperativeHandle() second argument to be a function ' + @@ -854,7 +847,6 @@ export function useImperativeHandle( create !== null ? typeof create : 'null', ); } - currentHookType = ImperativeHandleHook; // TODO: If deps are provided, should we skip comparing the ref itself? const nextDeps = deps !== null && deps !== undefined ? deps.concat([ref]) : [ref]; @@ -892,7 +884,7 @@ export function useDebugValue( formatterFn: ?(value: any) => any, ): void { if (__DEV__) { - currentHookNameInDev = HookDevNames[DebugValueHook]; + currentHookNameInDev = DebugValueHook; } // This will trigger a warning if the hook is used in a non-Function component. @@ -908,12 +900,10 @@ export function useCallback( deps: Array | void | null, ): T { if (__DEV__) { - currentHookNameInDev = HookDevNames[CallbackHook]; + currentHookNameInDev = CallbackHook; } - currentHookType = CallbackHook; currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); workInProgressHook = createWorkInProgressHook(); - currentHookType = null; const nextDeps = deps === undefined ? null : deps; @@ -922,11 +912,13 @@ export function useCallback( if (nextDeps !== null) { const prevDeps: Array | null = prevState[1]; if (areHookInputsEqual(nextDeps, prevDeps)) { + currentHookNameInDev = null; return prevState[0]; } } } workInProgressHook.memoizedState = [callback, nextDeps]; + currentHookNameInDev = null; return callback; } @@ -935,12 +927,10 @@ export function useMemo( deps: Array | void | null, ): T { if (__DEV__) { - currentHookNameInDev = HookDevNames[MemoHook]; + currentHookNameInDev = MemoHook; } - currentHookType = MemoHook; let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber()); workInProgressHook = createWorkInProgressHook(); - currentHookType = null; const nextDeps = deps === undefined ? null : deps; @@ -950,6 +940,7 @@ export function useMemo( if (nextDeps !== null) { const prevDeps: Array | null = prevState[1]; if (areHookInputsEqual(nextDeps, prevDeps)) { + currentHookNameInDev = null; return prevState[0]; } } @@ -960,6 +951,7 @@ export function useMemo( const nextValue = nextCreate(); currentlyRenderingFiber = fiber; workInProgressHook.memoizedState = [nextValue, nextDeps]; + currentHookNameInDev = null; return nextValue; } diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index c827cbabfd147..0ee829c1b6e88 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -730,11 +730,8 @@ describe('ReactHooks', () => { ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^', ]); - expect(() => { - root.update(); - }).toWarnDev([ - 'Warning: React has detected a change in the order of Hooks called', - ]); + // further warnings for this component are silenced + root.update(); }); it('detects a bad hook order even if the component throws', () => { From d4860ca98e0ade68935605cfda7347a90a4c4ee3 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Tue, 22 Jan 2019 22:00:04 +0000 Subject: [PATCH 34/35] lint --- .../react-reconciler/src/__tests__/ReactHooks-test.internal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 0ee829c1b6e88..45810597c87ab 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -731,7 +731,7 @@ describe('ReactHooks', () => { ]); // further warnings for this component are silenced - root.update(); + root.update(); }); it('detects a bad hook order even if the component throws', () => { From 3ad6523edf9fa1dba1f9d96f5aeb3de113f5d1bf Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Tue, 22 Jan 2019 22:23:44 +0000 Subject: [PATCH 35/35] final nits --- .../react-reconciler/src/ReactFiberHooks.js | 53 +++++++++---------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 4f594691d0ce7..825ea9c08503c 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -54,18 +54,17 @@ type UpdateQueue = { eagerState: S | null, }; -opaque type HookType = string; - -const StateHook: HookType = 'useState'; -const ReducerHook: HookType = 'useReducer'; -const ContextHook: HookType = 'useContext'; -const RefHook: HookType = 'useRef'; -const EffectHook: HookType = 'useEffect'; -const LayoutEffectHook: HookType = 'useLayoutEffect'; -const CallbackHook: HookType = 'useCallback'; -const MemoHook: HookType = 'useMemo'; -const ImperativeHandleHook: HookType = 'useImperativeHandle'; -const DebugValueHook: HookType = 'useDebugValue'; +type HookType = + | 'useState' + | 'useReducer' + | 'useContext' + | 'useRef' + | 'useEffect' + | 'useLayoutEffect' + | 'useCallback' + | 'useMemo' + | 'useImperativeHandle' + | 'useDebugValue'; // the first instance of a hook mismatch in a component, // represented by a portion of it's stacktrace @@ -213,7 +212,7 @@ function flushHookMismatchWarnings() { // and a stack trace so the dev can locate where // the first mismatch is coming from if (__DEV__) { - if (currentHookMismatch) { + if (currentHookMismatch !== null) { let componentName = getComponentName( ((currentlyRenderingFiber: any): Fiber).type, ); @@ -221,13 +220,13 @@ function flushHookMismatchWarnings() { didWarnAboutMismatchedHooksForComponent.add(componentName); const hookStackDiff = []; let current = firstCurrentHook; - let previousOrder = []; + const previousOrder = []; while (current !== null) { previousOrder.push(((current: any): HookDev)._debugType); current = current.next; } let workInProgress = firstWorkInProgressHook; - let nextOrder = []; + const nextOrder = []; while (workInProgress !== null) { nextOrder.push(((workInProgress: any): HookDev)._debugType); workInProgress = workInProgress.next; @@ -245,7 +244,7 @@ function flushHookMismatchWarnings() { ' Next render\n'; const hookStackWidth = hookStackHeader.length; hookStackHeader += ' ' + new Array(hookStackWidth - 2).join('-'); - let hookStackFooter = ' ' + new Array(hookStackWidth - 2).join('^'); + const hookStackFooter = ' ' + new Array(hookStackWidth - 2).join('^'); const hookStackLength = Math.max( previousOrder.length, @@ -544,7 +543,7 @@ export function useContext( observedBits: void | number | boolean, ): T { if (__DEV__) { - currentHookNameInDev = ContextHook; + currentHookNameInDev = 'useContext'; createWorkInProgressHook(); currentHookNameInDev = null; } @@ -558,7 +557,7 @@ export function useState( initialState: (() => S) | S, ): [S, Dispatch>] { if (__DEV__) { - currentHookNameInDev = StateHook; + currentHookNameInDev = 'useState'; } return useReducer( basicStateReducer, @@ -574,7 +573,7 @@ export function useReducer( ): [S, Dispatch] { if (__DEV__) { if (reducer !== basicStateReducer) { - currentHookNameInDev = ReducerHook; + currentHookNameInDev = 'useReducer'; } } let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber()); @@ -759,7 +758,7 @@ function pushEffect(tag, create, destroy, deps) { export function useRef(initialValue: T): {current: T} { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); if (__DEV__) { - currentHookNameInDev = RefHook; + currentHookNameInDev = 'useRef'; } workInProgressHook = createWorkInProgressHook(); currentHookNameInDev = null; @@ -782,8 +781,8 @@ export function useLayoutEffect( deps: Array | void | null, ): void { if (__DEV__) { - if (currentHookNameInDev !== ImperativeHandleHook) { - currentHookNameInDev = LayoutEffectHook; + if (currentHookNameInDev !== 'useImperativeHandle') { + currentHookNameInDev = 'useLayoutEffect'; } } useEffectImpl(UpdateEffect, UnmountMutation | MountLayout, create, deps); @@ -794,7 +793,7 @@ export function useEffect( deps: Array | void | null, ): void { if (__DEV__) { - currentHookNameInDev = EffectHook; + currentHookNameInDev = 'useEffect'; } useEffectImpl( UpdateEffect | PassiveEffect, @@ -839,7 +838,7 @@ export function useImperativeHandle( deps: Array | void | null, ): void { if (__DEV__) { - currentHookNameInDev = ImperativeHandleHook; + currentHookNameInDev = 'useImperativeHandle'; warning( typeof create === 'function', 'Expected useImperativeHandle() second argument to be a function ' + @@ -884,7 +883,7 @@ export function useDebugValue( formatterFn: ?(value: any) => any, ): void { if (__DEV__) { - currentHookNameInDev = DebugValueHook; + currentHookNameInDev = 'useDebugValue'; } // This will trigger a warning if the hook is used in a non-Function component. @@ -900,7 +899,7 @@ export function useCallback( deps: Array | void | null, ): T { if (__DEV__) { - currentHookNameInDev = CallbackHook; + currentHookNameInDev = 'useCallback'; } currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); workInProgressHook = createWorkInProgressHook(); @@ -927,7 +926,7 @@ export function useMemo( deps: Array | void | null, ): T { if (__DEV__) { - currentHookNameInDev = MemoHook; + currentHookNameInDev = 'useMemo'; } let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber()); workInProgressHook = createWorkInProgressHook();