From eb9daa3ceebc24dfd7bf3a65fb9bc29a496bb45c Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 24 Jan 2019 16:26:47 -0800 Subject: [PATCH 1/9] Move DEV-only function right above where it's used I don't like looking at this top-level function #petty --- .../react-reconciler/src/ReactFiberHooks.js | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 5965f4d77ed41..3682fba1f7039 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -210,18 +210,6 @@ 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 (__DEV__) { - 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 @@ -254,6 +242,13 @@ function flushHookMismatchWarnings() { .concat(' Previous render'.length), ); + const padEndSpaces = (string, length) => { + if (string.length >= length) { + return string; + } + return string + ' ' + new Array(length - string.length).join(' '); + }; + let hookStackHeader = ((padEndSpaces(' Previous render', columnLength): any): string) + ' Next render\n'; From d36b34d52ab0117e398f6fbe95207d99eebecd98 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 24 Jan 2019 17:08:44 -0800 Subject: [PATCH 2/9] Use different dispatchers for functions & classes Classes support readContext, but not any of the other dispatcher methods. Function support all methods. This is a more robust version of our previous strategy of checking whether `currentlyRenderingFiber` is null. As a next step, we can use a separate dispatcher for each phase of the render cycle (mount versus update). --- .../react-debug-tools/src/ReactDebugHooks.js | 2 +- .../src/server/ReactPartialRendererHooks.js | 2 +- .../src/ReactFiberDispatcher.js | 36 ------- .../react-reconciler/src/ReactFiberHooks.js | 98 +++++++++++++++---- .../src/ReactFiberScheduler.js | 7 +- .../src/ReactShallowRenderer.js | 2 +- packages/react/src/ReactCurrentDispatcher.js | 2 +- 7 files changed, 87 insertions(+), 62 deletions(-) delete mode 100644 packages/react-reconciler/src/ReactFiberDispatcher.js diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index b53fab87d46d8..426d20a1328a6 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -10,7 +10,7 @@ import type {ReactContext, ReactProviderType} from 'shared/ReactTypes'; import type {Fiber} from 'react-reconciler/src/ReactFiber'; import type {Hook} from 'react-reconciler/src/ReactFiberHooks'; -import typeof {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberDispatcher'; +import type {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberHooks'; import ErrorStackParser from 'error-stack-parser'; import ReactSharedInternals from 'shared/ReactSharedInternals'; diff --git a/packages/react-dom/src/server/ReactPartialRendererHooks.js b/packages/react-dom/src/server/ReactPartialRendererHooks.js index 1689d830c184a..749437e1dd3da 100644 --- a/packages/react-dom/src/server/ReactPartialRendererHooks.js +++ b/packages/react-dom/src/server/ReactPartialRendererHooks.js @@ -7,7 +7,7 @@ * @flow */ -import typeof {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberDispatcher'; +import type {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberHooks'; import type {ThreadID} from './ReactThreadIDAllocator'; import type {ReactContext} from 'shared/ReactTypes'; diff --git a/packages/react-reconciler/src/ReactFiberDispatcher.js b/packages/react-reconciler/src/ReactFiberDispatcher.js deleted file mode 100644 index f1572072b3f6f..0000000000000 --- a/packages/react-reconciler/src/ReactFiberDispatcher.js +++ /dev/null @@ -1,36 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -import {readContext} from './ReactFiberNewContext'; -import { - useCallback, - useContext, - useEffect, - useImperativeHandle, - useDebugValue, - useLayoutEffect, - useMemo, - useReducer, - useRef, - useState, -} from './ReactFiberHooks'; - -export const Dispatcher = { - readContext, - useCallback, - useContext, - useEffect, - useImperativeHandle, - useDebugValue, - useLayoutEffect, - useMemo, - useReducer, - useRef, - useState, -}; diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 3682fba1f7039..42760959566d4 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -12,6 +12,8 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {HookEffectTag} from './ReactHookEffectTags'; +import ReactSharedInternals from 'shared/ReactSharedInternals'; + import {NoWork} from './ReactFiberExpirationTime'; import { readContext, @@ -42,6 +44,32 @@ import getComponentName from 'shared/getComponentName'; import is from 'shared/objectIs'; import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork'; +const {ReactCurrentDispatcher} = ReactSharedInternals; + +export type Dispatcher = { + useState(initialState: (() => S) | S): [S, Dispatch>], + useReducer( + reducer: (S, A) => S, + initialState: S, + initialAction: A | void | null, + ): [S, Dispatch], + useContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T, + useRef(initialValue: T): {current: T}, + useEffect(create: () => mixed, deps: Array | void | null): void, + useLayoutEffect(create: () => mixed, deps: Array | void | null): void, + useCallback(callback: T, deps: Array | void | null): T, + useMemo(nextCreate: () => T, deps: Array | void | null): T, + useImperativeHandle( + ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, + create: () => T, + deps: Array | void | null, + ): void, + useDebugValue(value: any, formatterFn: ?(value: any) => any): void, +}; + type Update = { expirationTime: ExpirationTime, action: A, @@ -168,6 +196,13 @@ function resolveCurrentlyRenderingFiber(): Fiber { return currentlyRenderingFiber; } +function throwInvalidHookError() { + invariant( + false, + 'Hooks can only be called inside the body of a function component.', + ); +} + function areHookInputsEqual( nextDeps: Array, prevDeps: Array | null, @@ -322,6 +357,8 @@ export function renderWithHooks( // renderPhaseUpdates = null; // numberOfReRenders = -1; + ReactCurrentDispatcher.current = HooksDispatcher; + let children; do { didScheduleRenderPhaseUpdate = false; @@ -351,6 +388,8 @@ export function renderWithHooks( } } while (didScheduleRenderPhaseUpdate); + ReactCurrentDispatcher.current = ContextOnlyDispatcher; + renderPhaseUpdates = null; numberOfReRenders = -1; @@ -549,7 +588,7 @@ function basicStateReducer(state: S, action: BasicStateAction): S { return typeof action === 'function' ? action(state) : action; } -export function useContext( +function useContext( context: ReactContext, observedBits: void | number | boolean, ): T { @@ -564,7 +603,7 @@ export function useContext( return readContext(context, observedBits); } -export function useState( +function useState( initialState: (() => S) | S, ): [S, Dispatch>] { if (__DEV__) { @@ -577,7 +616,7 @@ export function useState( ); } -export function useReducer( +function useReducer( reducer: (S, A) => S, initialState: S, initialAction: A | void | null, @@ -783,7 +822,7 @@ function pushEffect(tag, create, destroy, deps) { return effect; } -export function useRef(initialValue: T): {current: T} { +function useRef(initialValue: T): {current: T} { currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); if (__DEV__) { currentHookNameInDev = 'useRef'; @@ -806,7 +845,7 @@ export function useRef(initialValue: T): {current: T} { return ref; } -export function useLayoutEffect( +function useLayoutEffect( create: () => mixed, deps: Array | void | null, ): void { @@ -818,7 +857,7 @@ export function useLayoutEffect( useEffectImpl(UpdateEffect, UnmountMutation | MountLayout, create, deps); } -export function useEffect( +function useEffect( create: () => mixed, deps: Array | void | null, ): void { @@ -866,7 +905,7 @@ function useEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { } } -export function useImperativeHandle( +function useImperativeHandle( ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, create: () => T, deps: Array | void | null, @@ -912,10 +951,7 @@ export function useImperativeHandle( }, nextDeps); } -export function useDebugValue( - value: any, - formatterFn: ?(value: any) => any, -): void { +function useDebugValue(value: any, formatterFn: ?(value: any) => any): void { if (__DEV__) { currentHookNameInDev = 'useDebugValue'; } @@ -928,10 +964,7 @@ export function useDebugValue( // so that e.g. DevTools can display custom hook values. } -export function useCallback( - callback: T, - deps: Array | void | null, -): T { +function useCallback(callback: T, deps: Array | void | null): T { if (__DEV__) { currentHookNameInDev = 'useCallback'; } @@ -957,10 +990,7 @@ export function useCallback( return callback; } -export function useMemo( - nextCreate: () => T, - deps: Array | void | null, -): T { +function useMemo(nextCreate: () => T, deps: Array | void | null): T { if (__DEV__) { currentHookNameInDev = 'useMemo'; } @@ -1124,3 +1154,33 @@ function dispatchAction( scheduleWork(fiber, expirationTime); } } + +export const ContextOnlyDispatcher: Dispatcher = { + readContext, + + useCallback: throwInvalidHookError, + useContext: throwInvalidHookError, + useEffect: throwInvalidHookError, + useImperativeHandle: throwInvalidHookError, + useLayoutEffect: throwInvalidHookError, + useMemo: throwInvalidHookError, + useReducer: throwInvalidHookError, + useRef: throwInvalidHookError, + useState: throwInvalidHookError, + useDebugValue: throwInvalidHookError, +}; + +export const HooksDispatcher: Dispatcher = { + readContext, + + useCallback, + useContext, + useEffect, + useImperativeHandle, + useLayoutEffect, + useMemo, + useReducer, + useRef, + useState, + useDebugValue, +}; diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 1563f54b69a82..bb199da8565c8 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -164,7 +164,7 @@ import { commitDetachRef, commitPassiveHookEffects, } from './ReactFiberCommitWork'; -import {Dispatcher} from './ReactFiberDispatcher'; +import {ContextOnlyDispatcher} from './ReactFiberHooks'; export type Thenable = { then(resolve: () => mixed, reject?: () => mixed): mixed, @@ -1216,7 +1216,8 @@ function renderRoot(root: FiberRoot, isYieldy: boolean): void { flushPassiveEffects(); isWorking = true; - ReactCurrentDispatcher.current = Dispatcher; + const previousDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = ContextOnlyDispatcher; const expirationTime = root.nextExpirationTimeToWorkOn; @@ -1377,7 +1378,7 @@ function renderRoot(root: FiberRoot, isYieldy: boolean): void { // We're done performing work. Time to clean up. isWorking = false; - ReactCurrentDispatcher.current = null; + ReactCurrentDispatcher.current = previousDispatcher; resetContextDependences(); resetHooks(); diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 815220efb7c1d..ee8b68b0f857e 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -18,7 +18,7 @@ import ReactSharedInternals from 'shared/ReactSharedInternals'; import warning from 'shared/warning'; import is from 'shared/objectIs'; -import typeof {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberDispatcher'; +import type {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactFiberHooks'; import type {ReactContext} from 'shared/ReactTypes'; import type {ReactElement} from 'shared/ReactElementType'; diff --git a/packages/react/src/ReactCurrentDispatcher.js b/packages/react/src/ReactCurrentDispatcher.js index 089082d8d84a0..5b08f8502cc91 100644 --- a/packages/react/src/ReactCurrentDispatcher.js +++ b/packages/react/src/ReactCurrentDispatcher.js @@ -7,7 +7,7 @@ * @flow */ -import typeof {Dispatcher} from 'react-reconciler/src/ReactFiberDispatcher'; +import type {Dispatcher} from 'react-reconciler/src/ReactFiberHooks'; /** * Keeps track of the current dispatcher. From 5f747a4278804fe19309e61c1cbbc42d776385f6 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 25 Jan 2019 19:58:44 -0800 Subject: [PATCH 3/9] Use separate dispatchers for mount and update --- .../react-reconciler/src/ReactFiberHooks.js | 1039 +++++++++++------ .../src/__tests__/ReactHooks-test.internal.js | 4 +- 2 files changed, 693 insertions(+), 350 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 42760959566d4..1848aee5b0b13 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -8,6 +8,7 @@ */ import type {ReactContext} from 'shared/ReactTypes'; +import type {SideEffectTag} from 'shared/ReactSideEffectTags'; import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {HookEffectTag} from './ReactHookEffectTags'; @@ -15,11 +16,7 @@ import type {HookEffectTag} from './ReactHookEffectTags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import {NoWork} from './ReactFiberExpirationTime'; -import { - readContext, - enterDisallowedContextReadInDEV, - exitDisallowedContextReadInDEV, -} from './ReactFiberNewContext'; +import {readContext} from './ReactFiberNewContext'; import { Update as UpdateEffect, Passive as PassiveEffect, @@ -100,7 +97,6 @@ type HookType = // the first instance of a hook mismatch in a component, // represented by a portion of its stacktrace let currentHookMismatchInDev = null; -let isInHookUserCodeInDev = false; let didWarnAboutMismatchedHooksForComponent; if (__DEV__) { @@ -154,6 +150,7 @@ let workInProgressHook: Hook | null = null; let remainingExpirationTime: ExpirationTime = NoWork; let componentUpdateQueue: FunctionComponentUpdateQueue | null = null; +let sideEffectTag: SideEffectTag = 0; // Updates scheduled during render will trigger an immediate re-render at the // end of the current pass. We can't store these updates on the normal queue, @@ -171,31 +168,12 @@ let renderPhaseUpdates: Map< Update, > | null = null; // Counter to prevent infinite loops. -let numberOfReRenders: number = -1; +let numberOfReRenders: number = 0; const RE_RENDER_LIMIT = 25; // In DEV, this is the name of the currently executing primitive hook let currentHookNameInDev: ?HookType = null; -function resolveCurrentlyRenderingFiber(): Fiber { - invariant( - currentlyRenderingFiber !== null, - 'Hooks can only be called inside the body of a function component.', - ); - if (__DEV__) { - // Check if we're inside Hooks like useMemo(). DEV-only for perf. - // TODO: we can make a better warning message with currentHookNameInDev - // if we also make sure it's consistently assigned in the right order. - warning( - !isInHookUserCodeInDev, - 'Hooks can only be called inside the body of a function component. ' + - 'Do not call Hooks inside other Hooks. For more information, see ' + - 'https://fb.me/rules-of-hooks', - ); - } - return currentlyRenderingFiber; -} - function throwInvalidHookError() { invariant( false, @@ -341,11 +319,6 @@ export function renderWithHooks( currentlyRenderingFiber = workInProgress; firstCurrentHook = current !== null ? current.memoizedState : null; - if (__DEV__) { - exitDisallowedContextReadInDEV(); - isInHookUserCodeInDev = false; - } - // The following should have already been reset // currentHook = null; // workInProgressHook = null; @@ -355,49 +328,67 @@ export function renderWithHooks( // didScheduleRenderPhaseUpdate = false; // renderPhaseUpdates = null; - // numberOfReRenders = -1; + // numberOfReRenders = 0; + // sideEffectTag = 0; - ReactCurrentDispatcher.current = HooksDispatcher; + if (__DEV__) { + ReactCurrentDispatcher.current = + current === null + ? HooksDispatcherOnMountInDEV + : HooksDispatcherOnUpdateInDEV; + } else { + ReactCurrentDispatcher.current = + current === null ? HooksDispatcherOnMount : HooksDispatcherOnUpdate; + } - let children; - do { - didScheduleRenderPhaseUpdate = false; - numberOfReRenders += 1; + let children = Component(props, refOrContext); - // Start over from the beginning of the list - currentHook = null; - workInProgressHook = null; - componentUpdateQueue = null; + if (didScheduleRenderPhaseUpdate) { + ReactCurrentDispatcher.current = __DEV__ + ? HooksDispatcherOnUpdateInDEV + : HooksDispatcherOnUpdate; + do { + didScheduleRenderPhaseUpdate = false; + numberOfReRenders += 1; - children = Component(props, refOrContext); + // Start over from the beginning of the list + currentHook = null; + workInProgressHook = null; + componentUpdateQueue = null; - if (__DEV__) { - if ( - current !== null && - workInProgressHook !== null && - currentHook === null - ) { - warning( - false, - '%s: Rendered more hooks than during the previous render. This is ' + - 'not currently supported and may lead to unexpected behavior.', - getComponentName(Component), - ); - } - flushHookMismatchWarnings(); - } - } while (didScheduleRenderPhaseUpdate); + children = Component(props, refOrContext); + } while (didScheduleRenderPhaseUpdate); + renderPhaseUpdates = null; + numberOfReRenders = 0; + } + + // We can assume the previous dispatcher is always this one, since we set it + // at the beginning of the render phase and there's no re-entrancy. ReactCurrentDispatcher.current = ContextOnlyDispatcher; - renderPhaseUpdates = null; - numberOfReRenders = -1; + if (__DEV__) { + if ( + current !== null && + workInProgressHook !== null && + currentHook === null + ) { + warning( + false, + '%s: Rendered more hooks than during the previous render. This is ' + + 'not currently supported and may lead to unexpected behavior.', + getComponentName(Component), + ); + } + flushHookMismatchWarnings(); + } const renderedWork: Fiber = (currentlyRenderingFiber: any); renderedWork.memoizedState = firstWorkInProgressHook; renderedWork.expirationTime = remainingExpirationTime; - renderedWork.updateQueue = componentUpdateQueue; + renderedWork.updateQueue = (componentUpdateQueue: any); + renderedWork.effectTag |= sideEffectTag; const didRenderTooFewHooks = currentHook !== null && currentHook.next !== null; @@ -412,6 +403,7 @@ export function renderWithHooks( remainingExpirationTime = NoWork; componentUpdateQueue = null; + sideEffectTag = 0; if (__DEV__) { currentHookNameInDev = null; @@ -420,7 +412,7 @@ export function renderWithHooks( // These were reset above // didScheduleRenderPhaseUpdate = false; // renderPhaseUpdates = null; - // numberOfReRenders = -1; + // numberOfReRenders = 0; invariant( !didRenderTooFewHooks, @@ -446,10 +438,12 @@ export function bailoutHooks( export function resetHooks(): void { if (__DEV__) { flushHookMismatchWarnings(); - exitDisallowedContextReadInDEV(); - isInHookUserCodeInDev = false; } + // We can assume the previous dispatcher is always this one, since we set it + // at the beginning of the render phase and there's no re-entrancy. + ReactCurrentDispatcher.current = ContextOnlyDispatcher; + // This is used to reset the state of this module when a component throws. // It's also called inside mountIndeterminateComponent if we determine the // component is a module-style component. @@ -463,6 +457,7 @@ export function resetHooks(): void { remainingExpirationTime = NoWork; componentUpdateQueue = null; + sideEffectTag = 0; if (__DEV__) { currentHookNameInDev = null; @@ -470,7 +465,7 @@ export function resetHooks(): void { didScheduleRenderPhaseUpdate = false; renderPhaseUpdates = null; - numberOfReRenders = -1; + numberOfReRenders = 0; } function createHook(): Hook { @@ -533,7 +528,18 @@ function cloneHook(hook: Hook): Hook { return nextHook; } -function createWorkInProgressHook(): Hook { +function mountWorkInProgressHook(): Hook { + if (workInProgressHook === null) { + // This is the first hook in the list + firstWorkInProgressHook = workInProgressHook = createHook(); + } else { + // Append to the end of the list + workInProgressHook = workInProgressHook.next = createHook(); + } + return workInProgressHook; +} + +function updateWorkInProgressHook(): Hook { if (workInProgressHook === null) { // This is the first hook in the list if (firstWorkInProgressHook === null) { @@ -588,212 +594,234 @@ function basicStateReducer(state: S, action: BasicStateAction): S { return typeof action === 'function' ? action(state) : action; } -function useContext( +function mountContext( context: ReactContext, observedBits: void | number | boolean, ): T { if (__DEV__) { currentHookNameInDev = 'useContext'; - createWorkInProgressHook(); + mountWorkInProgressHook(); currentHookNameInDev = null; } - // Ensure we're in a function component (class components support only the - // .unstable_read() form) - resolveCurrentlyRenderingFiber(); return readContext(context, observedBits); } -function useState( - initialState: (() => S) | S, -): [S, Dispatch>] { +function updateContext( + context: ReactContext, + observedBits: void | number | boolean, +): T { if (__DEV__) { - currentHookNameInDev = 'useState'; + currentHookNameInDev = 'useContext'; + updateWorkInProgressHook(); + currentHookNameInDev = null; } - return useReducer( - basicStateReducer, - // useReducer has a special case to support lazy useState initializers - (initialState: any), - ); + return readContext(context, observedBits); } -function useReducer( +function mountReducerImpl( + hook: Hook, reducer: (S, A) => S, - initialState: S, - initialAction: A | void | null, + initialState: void | S, + initialAction: void | null | A, ): [S, Dispatch] { - if (__DEV__) { - if (reducer !== basicStateReducer) { - currentHookNameInDev = 'useReducer'; + if (reducer === basicStateReducer) { + // Special case for `useState`. + if (typeof initialState === 'function') { + initialState = initialState(); } + } else if (initialAction !== undefined && initialAction !== null) { + initialState = reducer(initialState, initialAction); } - currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(); + hook.memoizedState = hook.baseState = initialState; + const queue = (hook.queue = { + last: null, + dispatch: null, + eagerReducer: reducer, + eagerState: initialState, + }); + const dispatch: Dispatch = (queue.dispatch = (dispatchAction.bind( + null, + currentlyRenderingFiber, + queue, + ): any)); + return [hook.memoizedState, dispatch]; +} + +function mountReducer( + reducer: (S, A) => S, + initialState: void | S, + initialAction: void | null | A, +): [S, Dispatch] { if (__DEV__) { - currentHookNameInDev = null; + currentHookNameInDev = 'useReducer'; + } + const hook = mountWorkInProgressHook(); + return mountReducerImpl(hook, reducer, initialState, initialAction); +} + +function updateReducerImpl( + hook: Hook, + reducer: (S, A) => S, + initialState: void | S, + initialAction: void | null | A, +): [S, Dispatch] { + const queue = hook.queue; + + if (queue === null) { + return mountReducerImpl(hook, reducer, initialState, initialAction); } - let queue: UpdateQueue | null = (workInProgressHook.queue: any); - if (queue !== null) { - // Already have a queue, so this is an update. - if (numberOfReRenders > 0) { - // This is a re-render. Apply the new render phase updates to the previous - // work-in-progress hook. - const dispatch: Dispatch = (queue.dispatch: any); - if (renderPhaseUpdates !== null) { - // Render phase updates are stored in a map of queue -> linked list - const firstRenderPhaseUpdate = renderPhaseUpdates.get(queue); - if (firstRenderPhaseUpdate !== undefined) { - renderPhaseUpdates.delete(queue); - let newState = workInProgressHook.memoizedState; - let update = firstRenderPhaseUpdate; - do { - // Process this render phase update. We don't have to check the - // priority because it will always be the same as the current - // render's. - const action = update.action; - if (__DEV__) { - enterDisallowedContextReadInDEV(); - isInHookUserCodeInDev = true; - } - newState = reducer(newState, action); - if (__DEV__) { - exitDisallowedContextReadInDEV(); - isInHookUserCodeInDev = false; - } - update = update.next; - } while (update !== null); - - workInProgressHook.memoizedState = newState; - - // Don't persist the state accumlated from the render phase updates to - // the base state unless the queue is empty. - // TODO: Not sure if this is the desired semantics, but it's what we - // do for gDSFP. I can't remember why. - if (workInProgressHook.baseUpdate === queue.last) { - workInProgressHook.baseState = newState; - } - return [newState, dispatch]; + if (numberOfReRenders > 0) { + // This is a re-render. Apply the new render phase updates to the previous + // work-in-progress hook. + const dispatch: Dispatch = (queue.dispatch: any); + if (renderPhaseUpdates !== null) { + // Render phase updates are stored in a map of queue -> linked list + const firstRenderPhaseUpdate = renderPhaseUpdates.get(queue); + if (firstRenderPhaseUpdate !== undefined) { + renderPhaseUpdates.delete(queue); + let newState = workInProgressHook.memoizedState; + let update = firstRenderPhaseUpdate; + do { + // Process this render phase update. We don't have to check the + // priority because it will always be the same as the current + // render's. + const action = update.action; + newState = reducer(newState, action); + update = update.next; + } while (update !== null); + + // Mark that the fiber performed work, but only if the new state is + // different from the current state. + if (newState !== hook.memoizedState) { + markWorkInProgressReceivedUpdate(); } + + hook.memoizedState = newState; + + // Don't persist the state accumlated from the render phase updates to + // the base state unless the queue is empty. + // TODO: Not sure if this is the desired semantics, but it's what we + // do for gDSFP. I can't remember why. + if (hook.baseUpdate === queue.last) { + hook.baseState = newState; + } + + return [newState, dispatch]; } - return [workInProgressHook.memoizedState, dispatch]; } + return [hook.memoizedState, dispatch]; + } - // The last update in the entire queue - const last = queue.last; - // The last update that is part of the base state. - const baseUpdate = workInProgressHook.baseUpdate; - const baseState = workInProgressHook.baseState; - - // Find the first unprocessed update. - let first; - if (baseUpdate !== null) { - if (last !== null) { - // For the first update, the queue is a circular linked list where - // `queue.last.next = queue.first`. Once the first update commits, and - // the `baseUpdate` is no longer empty, we can unravel the list. - last.next = null; - } - first = baseUpdate.next; - } else { - first = last !== null ? last.next : null; + // The last update in the entire queue + const last = queue.last; + // The last update that is part of the base state. + const baseUpdate = hook.baseUpdate; + const baseState = hook.baseState; + + // Find the first unprocessed update. + let first; + if (baseUpdate !== null) { + if (last !== null) { + // For the first update, the queue is a circular linked list where + // `queue.last.next = queue.first`. Once the first update commits, and + // the `baseUpdate` is no longer empty, we can unravel the list. + last.next = null; } - if (first !== null) { - let newState = baseState; - let newBaseState = null; - let newBaseUpdate = null; - let prevUpdate = baseUpdate; - let update = first; - let didSkip = false; - do { - const updateExpirationTime = update.expirationTime; - if (updateExpirationTime < renderExpirationTime) { - // Priority is insufficient. Skip this update. If this is the first - // skipped update, the previous update/state is the new base - // update/state. - if (!didSkip) { - didSkip = true; - newBaseUpdate = prevUpdate; - newBaseState = newState; - } - // Update the remaining priority in the queue. - if (updateExpirationTime > remainingExpirationTime) { - remainingExpirationTime = updateExpirationTime; - } + first = baseUpdate.next; + } else { + first = last !== null ? last.next : null; + } + if (first !== null) { + let newState = baseState; + let newBaseState = null; + let newBaseUpdate = null; + let prevUpdate = baseUpdate; + let update = first; + let didSkip = false; + do { + const updateExpirationTime = update.expirationTime; + if (updateExpirationTime < renderExpirationTime) { + // Priority is insufficient. Skip this update. If this is the first + // skipped update, the previous update/state is the new base + // update/state. + if (!didSkip) { + didSkip = true; + newBaseUpdate = prevUpdate; + newBaseState = newState; + } + // Update the remaining priority in the queue. + if (updateExpirationTime > remainingExpirationTime) { + remainingExpirationTime = updateExpirationTime; + } + } else { + // Process this update. + if (update.eagerReducer === reducer) { + // If this update was processed eagerly, and its reducer matches the + // current reducer, we can use the eagerly computed state. + newState = ((update.eagerState: any): S); } else { - // Process this update. - if (update.eagerReducer === reducer) { - // If this update was processed eagerly, and its reducer matches the - // current reducer, we can use the eagerly computed state. - newState = ((update.eagerState: any): S); - } else { - const action = update.action; - if (__DEV__) { - enterDisallowedContextReadInDEV(); - isInHookUserCodeInDev = true; - } - newState = reducer(newState, action); - if (__DEV__) { - exitDisallowedContextReadInDEV(); - isInHookUserCodeInDev = false; - } - } + const action = update.action; + newState = reducer(newState, action); } - prevUpdate = update; - update = update.next; - } while (update !== null && update !== first); - - if (!didSkip) { - newBaseUpdate = prevUpdate; - newBaseState = newState; } + prevUpdate = update; + update = update.next; + } while (update !== null && update !== first); - workInProgressHook.memoizedState = newState; - workInProgressHook.baseUpdate = newBaseUpdate; - workInProgressHook.baseState = newBaseState; - - // Mark that the fiber performed work, but only if the new state is - // different from the current state. - if (newState !== (currentHook: any).memoizedState) { - markWorkInProgressReceivedUpdate(); - } + if (!didSkip) { + newBaseUpdate = prevUpdate; + newBaseState = newState; + } - queue.eagerReducer = reducer; - queue.eagerState = newState; + // Mark that the fiber performed work, but only if the new state is + // different from the current state. + if (newState !== hook.memoizedState) { + markWorkInProgressReceivedUpdate(); } - const dispatch: Dispatch = (queue.dispatch: any); - return [workInProgressHook.memoizedState, dispatch]; + hook.memoizedState = newState; + hook.baseUpdate = newBaseUpdate; + hook.baseState = newBaseState; + + queue.eagerReducer = reducer; + queue.eagerState = newState; } + + const dispatch: Dispatch = (queue.dispatch: any); + return [hook.memoizedState, dispatch]; +} + +function updateReducer( + reducer: (S, A) => S, + initialState: void | S, + initialAction: void | null | A, +): [S, Dispatch] { if (__DEV__) { - enterDisallowedContextReadInDEV(); - isInHookUserCodeInDev = true; + currentHookNameInDev = 'useReducer'; } - // There's no existing queue, so this is the initial render. - if (reducer === basicStateReducer) { - // Special case for `useState`. - if (typeof initialState === 'function') { - initialState = initialState(); - } - } else if (initialAction !== undefined && initialAction !== null) { - initialState = reducer(initialState, initialAction); + const hook = updateWorkInProgressHook(); + return updateReducerImpl(hook, reducer, initialState, initialAction); +} + +function mountState( + initialState: (() => S) | S, +): [S, Dispatch>] { + if (__DEV__) { + currentHookNameInDev = 'useState'; } + const hook = mountWorkInProgressHook(); + return mountReducerImpl(hook, basicStateReducer, initialState); +} + +function updateState( + initialState: (() => S) | S, +): [S, Dispatch>] { if (__DEV__) { - exitDisallowedContextReadInDEV(); - isInHookUserCodeInDev = false; + currentHookNameInDev = 'useState'; } - workInProgressHook.memoizedState = workInProgressHook.baseState = initialState; - queue = workInProgressHook.queue = { - last: null, - dispatch: null, - eagerReducer: reducer, - eagerState: initialState, - }; - const dispatch: Dispatch = (queue.dispatch = (dispatchAction.bind( - null, - currentlyRenderingFiber, - queue, - ): any)); - return [workInProgressHook.memoizedState, dispatch]; + const hook = updateWorkInProgressHook(); + return updateReducerImpl(hook, basicStateReducer, initialState); } function pushEffect(tag, create, destroy, deps) { @@ -822,62 +850,57 @@ function pushEffect(tag, create, destroy, deps) { return effect; } -function useRef(initialValue: T): {current: T} { - currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); +function mountRef(initialValue: T): {current: T} { if (__DEV__) { currentHookNameInDev = 'useRef'; } - workInProgressHook = createWorkInProgressHook(); + const hook = mountWorkInProgressHook(); + const ref = {current: initialValue}; if (__DEV__) { - currentHookNameInDev = null; + Object.seal(ref); } - let ref; + hook.memoizedState = ref; + return ref; +} - if (workInProgressHook.memoizedState === null) { +function updateRef(initialValue: T): {current: T} { + if (__DEV__) { + currentHookNameInDev = 'useRef'; + } + const hook = updateWorkInProgressHook(); + let ref = hook.memoizedState; + if (ref === null) { ref = {current: initialValue}; if (__DEV__) { Object.seal(ref); } - workInProgressHook.memoizedState = ref; - } else { - ref = workInProgressHook.memoizedState; + hook.memoizedState = ref; } return ref; } -function useLayoutEffect( - create: () => mixed, - deps: Array | void | null, +function mountEffectImpl( + hook, + fiberEffectTag, + hookEffectTag, + create, + deps, ): void { - if (__DEV__) { - if (currentHookNameInDev !== 'useImperativeHandle') { - currentHookNameInDev = 'useLayoutEffect'; - } - } - useEffectImpl(UpdateEffect, UnmountMutation | MountLayout, create, deps); + const nextDeps = deps === undefined ? null : deps; + sideEffectTag |= fiberEffectTag; + hook.memoizedState = pushEffect(hookEffectTag, create, null, nextDeps); } -function useEffect( - create: () => mixed, - deps: Array | void | null, +function updateEffectImpl( + hook, + fiberEffectTag, + hookEffectTag, + create, + deps, ): void { - if (__DEV__) { - currentHookNameInDev = 'useEffect'; - } - useEffectImpl( - UpdateEffect | PassiveEffect, - UnmountPassive | MountPassive, - create, - deps, - ); -} - -function useEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { - currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(); - const nextDeps = deps === undefined ? null : deps; let destroy = null; + if (currentHook !== null) { const prevEffect = currentHook.memoizedState; destroy = prevEffect.destroy; @@ -893,19 +916,108 @@ function useEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { } } - currentlyRenderingFiber.effectTag |= fiberEffectTag; + sideEffectTag |= fiberEffectTag; workInProgressHook.memoizedState = pushEffect( hookEffectTag, create, destroy, nextDeps, ); +} + +function mountEffect( + create: () => mixed, + deps: Array | void | null, +): void { if (__DEV__) { - currentHookNameInDev = null; + currentHookNameInDev = 'useEffect'; } + const hook = mountWorkInProgressHook(); + return mountEffectImpl( + hook, + UpdateEffect | PassiveEffect, + UnmountPassive | MountPassive, + create, + deps, + ); +} + +function updateEffect( + create: () => mixed, + deps: Array | void | null, +): void { + if (__DEV__) { + currentHookNameInDev = 'useEffect'; + } + const hook = updateWorkInProgressHook(); + return updateEffectImpl( + hook, + UpdateEffect | PassiveEffect, + UnmountPassive | MountPassive, + create, + deps, + ); +} + +function mountLayoutEffect( + create: () => mixed, + deps: Array | void | null, +): void { + if (__DEV__) { + currentHookNameInDev = 'useLayoutEffect'; + } + const hook = mountWorkInProgressHook(); + return mountEffectImpl( + hook, + UpdateEffect, + UnmountMutation | MountLayout, + create, + deps, + ); } -function useImperativeHandle( +function updateLayoutEffect( + create: () => mixed, + deps: Array | void | null, +): void { + if (__DEV__) { + currentHookNameInDev = 'useLayoutEffect'; + } + const hook = updateWorkInProgressHook(); + return updateEffectImpl( + hook, + UpdateEffect, + UnmountMutation | MountLayout, + create, + deps, + ); +} + +function imperativeHandleEffect(create, ref) { + if (typeof ref === 'function') { + const refCallback = ref; + const inst = create(); + refCallback(inst); + return () => refCallback(null); + } else if (ref !== null && ref !== undefined) { + const refObject = ref; + if (__DEV__) { + warning( + refObject.hasOwnProperty('current'), + 'Expected useImperativeHandle() first argument to either be a ' + + 'ref callback or React.createRef() object. Instead received: %s.', + 'an object with keys {' + Object.keys(refObject).join(', ') + '}', + ); + } + const inst = create(); + refObject.current = inst; + return () => { + refObject.current = null; + }; + } +} + +function mountImperativeHandle( ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, create: () => T, deps: Array | void | null, @@ -919,61 +1031,79 @@ function useImperativeHandle( create !== null ? typeof create : 'null', ); } + const hook = mountWorkInProgressHook(); + // TODO: If deps are provided, should we skip comparing the ref itself? - const nextDeps = + const effectDeps = deps !== null && deps !== undefined ? deps.concat([ref]) : [ref]; - // 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. - useLayoutEffect(() => { - if (typeof ref === 'function') { - const refCallback = ref; - const inst = create(); - refCallback(inst); - return () => refCallback(null); - } else if (ref !== null && ref !== undefined) { - const refObject = ref; - if (__DEV__) { - warning( - refObject.hasOwnProperty('current'), - 'Expected useImperativeHandle() first argument to either be a ' + - 'ref callback or React.createRef() object. Instead received: %s.', - 'an object with keys {' + Object.keys(refObject).join(', ') + '}', - ); - } - const inst = create(); - refObject.current = inst; - return () => { - refObject.current = null; - }; - } - }, nextDeps); + return mountEffectImpl( + hook, + UpdateEffect, + UnmountMutation | MountLayout, + imperativeHandleEffect.bind(null, create, ref), + effectDeps, + ); } -function useDebugValue(value: any, formatterFn: ?(value: any) => any): void { +function updateImperativeHandle( + ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, + create: () => T, + deps: Array | void | null, +): void { if (__DEV__) { - currentHookNameInDev = 'useDebugValue'; + currentHookNameInDev = 'useImperativeHandle'; + warning( + typeof create === 'function', + 'Expected useImperativeHandle() second argument to be a function ' + + 'that creates a handle. Instead received: %s.', + create !== null ? typeof create : 'null', + ); } + const hook = updateWorkInProgressHook(); - // This will trigger a warning if the hook is used in a non-Function component. - resolveCurrentlyRenderingFiber(); + // TODO: If deps are provided, should we skip comparing the ref itself? + const effectDeps = + deps !== null && deps !== undefined ? deps.concat([ref]) : [ref]; + + return updateEffectImpl( + hook, + UpdateEffect, + UnmountMutation | MountLayout, + imperativeHandleEffect.bind(null, create, ref), + effectDeps, + ); +} + +function mountDebugValue(value: any, formatterFn: ?(value: any) => any): void { + if (__DEV__) { + currentHookNameInDev = 'useDebugValue'; + } // This hook is normally a no-op. // The react-debug-hooks package injects its own implementation // so that e.g. DevTools can display custom hook values. } -function useCallback(callback: T, deps: Array | void | null): T { +const updateDebugValue = mountDebugValue; + +function mountCallback(callback: T, deps: Array | void | null): T { if (__DEV__) { currentHookNameInDev = 'useCallback'; } - currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(); - + const hook = mountWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; + hook.memoizedState = [callback, nextDeps]; + return callback; +} - const prevState = workInProgressHook.memoizedState; +function updateCallback(callback: T, deps: Array | void | null): T { + if (__DEV__) { + currentHookNameInDev = 'useCallback'; + } + const hook = updateWorkInProgressHook(); + const nextDeps = deps === undefined ? null : deps; + const prevState = hook.memoizedState; if (prevState !== null) { if (nextDeps !== null) { const prevDeps: Array | null = prevState[1]; @@ -983,23 +1113,37 @@ function useCallback(callback: T, deps: Array | void | null): T { } } } - workInProgressHook.memoizedState = [callback, nextDeps]; + hook.memoizedState = [callback, nextDeps]; if (__DEV__) { currentHookNameInDev = null; } return callback; } -function useMemo(nextCreate: () => T, deps: Array | void | null): T { +function mountMemo( + nextCreate: () => T, + deps: Array | void | null, +): T { if (__DEV__) { currentHookNameInDev = 'useMemo'; } - currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); - workInProgressHook = createWorkInProgressHook(); - + const hook = mountWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; + const nextValue = nextCreate(); + hook.memoizedState = [nextValue, nextDeps]; + return nextValue; +} - const prevState = workInProgressHook.memoizedState; +function updateMemo( + nextCreate: () => T, + deps: Array | void | null, +): T { + if (__DEV__) { + currentHookNameInDev = 'useMemo'; + } + const hook = updateWorkInProgressHook(); + const nextDeps = deps === undefined ? null : deps; + const prevState = hook.memoizedState; if (prevState !== null) { // Assume these are defined. If they're not, areHookInputsEqual will warn. if (nextDeps !== null) { @@ -1012,20 +1156,8 @@ function useMemo(nextCreate: () => T, deps: Array | void | null): T { } } } - - if (__DEV__) { - enterDisallowedContextReadInDEV(); - isInHookUserCodeInDev = true; - } const nextValue = nextCreate(); - if (__DEV__) { - exitDisallowedContextReadInDEV(); - isInHookUserCodeInDev = false; - } - workInProgressHook.memoizedState = [nextValue, nextDeps]; - if (__DEV__) { - currentHookNameInDev = null; - } + hook.memoizedState = [nextValue, nextDeps]; return nextValue; } @@ -1117,17 +1249,12 @@ function dispatchAction( // same as the current state, we may be able to bail out entirely. const eagerReducer = queue.eagerReducer; if (eagerReducer !== null) { + if (__DEV__) { + ReactCurrentDispatcher.current = InvalidHooksDispatcherOnUpdateInDEV; + } try { const currentState: S = (queue.eagerState: any); - if (__DEV__) { - enterDisallowedContextReadInDEV(); - isInHookUserCodeInDev = true; - } const eagerState = eagerReducer(currentState, action); - if (__DEV__) { - exitDisallowedContextReadInDEV(); - isInHookUserCodeInDev = false; - } // Stash the eagerly computed state, and the reducer used to compute // it, on the update object. If the reducer hasn't changed by the // time we enter the render phase, then the eager state can be used @@ -1145,8 +1272,7 @@ function dispatchAction( // Suppress the error. It will throw again in the render phase. } finally { if (__DEV__) { - exitDisallowedContextReadInDEV(); - isInHookUserCodeInDev = false; + ReactCurrentDispatcher.current = InvalidHooksDispatcherOnUpdateInDEV; } } } @@ -1170,17 +1296,234 @@ export const ContextOnlyDispatcher: Dispatcher = { useDebugValue: throwInvalidHookError, }; -export const HooksDispatcher: Dispatcher = { +const HooksDispatcherOnMount: Dispatcher = { readContext, - useCallback, - useContext, - useEffect, - useImperativeHandle, - useLayoutEffect, - useMemo, - useReducer, - useRef, - useState, - useDebugValue, + useCallback: mountCallback, + useContext: __DEV__ ? mountContext : readContext, + useEffect: mountEffect, + useImperativeHandle: mountImperativeHandle, + useLayoutEffect: mountLayoutEffect, + useMemo: mountMemo, + useReducer: mountReducer, + useRef: mountRef, + useState: mountState, + useDebugValue: mountDebugValue, }; + +const HooksDispatcherOnMountInDEV: Dispatcher = { + readContext, + + useCallback: mountCallback, + useContext: __DEV__ ? mountContext : readContext, + useEffect: mountEffect, + useImperativeHandle: mountImperativeHandle, + useLayoutEffect: mountLayoutEffect, + useMemo(create, deps) { + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidHooksDispatcherOnMountInDEV; + try { + return mountMemo(create, deps); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useReducer(reducer, initialState, initialAction) { + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidHooksDispatcherOnMountInDEV; + try { + return mountReducer(reducer, initialState, initialAction); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useRef: mountRef, + useState(initialState) { + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidHooksDispatcherOnMountInDEV; + try { + return mountState(initialState); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useDebugValue: mountDebugValue, +}; + +const HooksDispatcherOnUpdate: Dispatcher = { + readContext, + + useCallback: updateCallback, + useContext: __DEV__ ? updateContext : readContext, + useEffect: updateEffect, + useImperativeHandle: updateImperativeHandle, + useLayoutEffect: updateLayoutEffect, + useMemo: updateMemo, + useReducer: updateReducer, + useRef: updateRef, + useState: updateState, + useDebugValue: updateDebugValue, +}; + +const HooksDispatcherOnUpdateInDEV: Dispatcher = { + readContext, + + useCallback: updateCallback, + useContext: __DEV__ ? updateContext : readContext, + useEffect: updateEffect, + useImperativeHandle: updateImperativeHandle, + useLayoutEffect: updateLayoutEffect, + useMemo(create, deps) { + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidHooksDispatcherOnUpdateInDEV; + try { + return updateMemo(create, deps); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useReducer(reducer, initialState, initialAction) { + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidHooksDispatcherOnUpdateInDEV; + try { + return updateReducer(reducer, initialState, initialAction); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useRef: updateRef, + useState(initialState) { + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidHooksDispatcherOnUpdateInDEV; + try { + return updateState(initialState); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useDebugValue: updateDebugValue, +}; + +let InvalidHooksDispatcherOnMountInDEV: Dispatcher | null; +let InvalidHooksDispatcherOnUpdateInDEV: Dispatcher | null; + +if (__DEV__) { + const warnInvalidHookAccess = () => { + warning( + false, + 'Hooks can only be called inside the body of a function component. ' + + 'Do not call Hooks inside other Hooks. For more information, see ' + + 'https://fb.me/rules-of-hooks', + ); + }; + + InvalidHooksDispatcherOnMountInDEV = { + readContext(context, observedBits) { + warning( + false, + 'Context can only be read while React is rendering. ' + + 'In classes, you can read it in the render method or getDerivedStateFromProps. ' + + 'In function components, you can read it directly in the function body, but not ' + + 'inside Hooks like useReducer() or useMemo().', + ); + + return readContext(context, observedBits); + }, + + useCallback(callback, deps) { + warnInvalidHookAccess(); + return mountCallback(callback, deps); + }, + useContext(context, observedBits) { + warnInvalidHookAccess(); + return mountContext(context, observedBits); + }, + useEffect(create, deps) { + warnInvalidHookAccess(); + return mountEffect(create, deps); + }, + useImperativeHandle(ref, create, deps) { + warnInvalidHookAccess(); + return mountImperativeHandle(ref, create, deps); + }, + useLayoutEffect(create, deps) { + warnInvalidHookAccess(); + return mountLayoutEffect(create, deps); + }, + useMemo(create, deps) { + warnInvalidHookAccess(); + return mountMemo(create, deps); + }, + useReducer(reducer, initialState, initialAction) { + warnInvalidHookAccess(); + return mountReducer(reducer, initialState, initialAction); + }, + useRef(initialValue) { + warnInvalidHookAccess(); + return mountReducer(initialValue); + }, + useState(initialState) { + warnInvalidHookAccess(); + return mountState(initialState); + }, + useDebugValue(value, formatterFn) { + warnInvalidHookAccess(); + return mountDebugValue(value, formatterFn); + }, + }; + + InvalidHooksDispatcherOnUpdateInDEV = { + readContext(context, observedBits) { + warning( + false, + 'Context can only be read while React is rendering. ' + + 'In classes, you can read it in the render method or getDerivedStateFromProps. ' + + 'In function components, you can read it directly in the function body, but not ' + + 'inside Hooks like useReducer() or useMemo().', + ); + + return readContext(context, observedBits); + }, + + useCallback(callback, deps) { + warnInvalidHookAccess(); + return updateCallback(callback, deps); + }, + useContext(context, observedBits) { + warnInvalidHookAccess(); + return updateContext(context, observedBits); + }, + useEffect(create, deps) { + warnInvalidHookAccess(); + return updateEffect(create, deps); + }, + useImperativeHandle(ref, create, deps) { + warnInvalidHookAccess(); + return updateImperativeHandle(ref, create, deps); + }, + useLayoutEffect(create, deps) { + warnInvalidHookAccess(); + return updateLayoutEffect(create, deps); + }, + useMemo(create, deps) { + warnInvalidHookAccess(); + return updateMemo(create, deps); + }, + useReducer(reducer, initialState, initialAction) { + warnInvalidHookAccess(); + return updateReducer(reducer, initialState, initialAction); + }, + useRef(initialValue) { + warnInvalidHookAccess(); + return updateReducer(initialValue); + }, + useState(initialState) { + warnInvalidHookAccess(); + return updateState(initialState); + }, + useDebugValue(value, formatterFn) { + warnInvalidHookAccess(); + return updateDebugValue(value, formatterFn); + }, + }; +} diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 862d8fbe2bb2d..8535e977673ff 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -719,7 +719,7 @@ describe('ReactHooks', () => { const root = ReactTestRenderer.create(); expect(() => root.update()).toThrow( // The exact message doesn't matter, just make sure we don't allow this - "Cannot read property 'readContext' of null", + 'Context can only be read while React is rendering', ); }); @@ -740,7 +740,7 @@ describe('ReactHooks', () => { expect(() => ReactTestRenderer.create()).toThrow( // The exact message doesn't matter, just make sure we don't allow this - "Cannot read property 'readContext' of null", + 'Context can only be read while React is rendering', ); }); From eaac7a57b1ba50c9da9f161fa60d518c310e3ee9 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 28 Jan 2019 13:59:37 -0800 Subject: [PATCH 4/9] Remove mount code from update path Deletes mount-specific code from the update path, since it should be unreachable. To continue supporting progressive enhancement (mounting new hooks at the end of the list), we detect when there are no more current hooks and switch back to the mount dispatcher. Progressive enhancement isn't officially supported yet, so it will continue to warn. --- .../react-reconciler/src/ReactFiberHooks.js | 537 +++++++++--------- 1 file changed, 261 insertions(+), 276 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 1848aee5b0b13..b9a08664bb588 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -145,8 +145,10 @@ let currentlyRenderingFiber: Fiber | null = null; // work-in-progress fiber. let firstCurrentHook: Hook | null = null; let currentHook: Hook | null = null; +let nextCurrentHook: Hook | null = null; let firstWorkInProgressHook: Hook | null = null; let workInProgressHook: Hook | null = null; +let nextWorkInProgressHook: Hook | null = null; let remainingExpirationTime: ExpirationTime = NoWork; let componentUpdateQueue: FunctionComponentUpdateQueue | null = null; @@ -317,7 +319,9 @@ export function renderWithHooks( ): any { renderExpirationTime = nextRenderExpirationTime; currentlyRenderingFiber = workInProgress; - firstCurrentHook = current !== null ? current.memoizedState : null; + firstCurrentHook = nextCurrentHook = + current !== null ? current.memoizedState : null; + nextWorkInProgressHook = null; // The following should have already been reset // currentHook = null; @@ -333,29 +337,36 @@ export function renderWithHooks( if (__DEV__) { ReactCurrentDispatcher.current = - current === null + nextCurrentHook === null ? HooksDispatcherOnMountInDEV : HooksDispatcherOnUpdateInDEV; } else { ReactCurrentDispatcher.current = - current === null ? HooksDispatcherOnMount : HooksDispatcherOnUpdate; + nextCurrentHook === null + ? HooksDispatcherOnMount + : HooksDispatcherOnUpdate; } let children = Component(props, refOrContext); if (didScheduleRenderPhaseUpdate) { - ReactCurrentDispatcher.current = __DEV__ - ? HooksDispatcherOnUpdateInDEV - : HooksDispatcherOnUpdate; do { didScheduleRenderPhaseUpdate = false; numberOfReRenders += 1; // Start over from the beginning of the list + firstCurrentHook = nextCurrentHook = + current !== null ? current.memoizedState : null; + nextWorkInProgressHook = firstWorkInProgressHook; + currentHook = null; workInProgressHook = null; componentUpdateQueue = null; + ReactCurrentDispatcher.current = __DEV__ + ? HooksDispatcherOnUpdateInDEV + : HooksDispatcherOnUpdate; + children = Component(props, refOrContext); } while (didScheduleRenderPhaseUpdate); @@ -363,26 +374,15 @@ export function renderWithHooks( numberOfReRenders = 0; } - // We can assume the previous dispatcher is always this one, since we set it - // at the beginning of the render phase and there's no re-entrancy. - ReactCurrentDispatcher.current = ContextOnlyDispatcher; - if (__DEV__) { - if ( - current !== null && - workInProgressHook !== null && - currentHook === null - ) { - warning( - false, - '%s: Rendered more hooks than during the previous render. This is ' + - 'not currently supported and may lead to unexpected behavior.', - getComponentName(Component), - ); - } flushHookMismatchWarnings(); + currentHookNameInDev = null; } + // We can assume the previous dispatcher is always this one, since we set it + // at the beginning of the render phase and there's no re-entrancy. + ReactCurrentDispatcher.current = ContextOnlyDispatcher; + const renderedWork: Fiber = (currentlyRenderingFiber: any); renderedWork.memoizedState = firstWorkInProgressHook; @@ -398,17 +398,15 @@ export function renderWithHooks( firstCurrentHook = null; currentHook = null; + nextCurrentHook = null; firstWorkInProgressHook = null; workInProgressHook = null; + nextWorkInProgressHook = null; remainingExpirationTime = NoWork; componentUpdateQueue = null; sideEffectTag = 0; - if (__DEV__) { - currentHookNameInDev = null; - } - // These were reset above // didScheduleRenderPhaseUpdate = false; // renderPhaseUpdates = null; @@ -452,8 +450,10 @@ export function resetHooks(): void { firstCurrentHook = null; currentHook = null; + nextCurrentHook = null; firstWorkInProgressHook = null; workInProgressHook = null; + nextWorkInProgressHook = null; remainingExpirationTime = NoWork; componentUpdateQueue = null; @@ -469,53 +469,47 @@ export function resetHooks(): void { } function createHook(): Hook { - let hook: Hook = __DEV__ - ? { - _debugType: ((currentHookNameInDev: any): HookType), - memoizedState: null, - - baseState: null, - queue: null, - baseUpdate: null, + const hook: Hook = { + memoizedState: null, - next: null, - } - : { - memoizedState: null, + baseState: null, + queue: null, + baseUpdate: null, - baseState: null, - queue: null, - baseUpdate: null, + next: null, + }; - next: null, - }; + if (__DEV__) { + hook._debugType = ((currentHookNameInDev: any): HookType); + if ( + currentlyRenderingFiber !== null && + currentlyRenderingFiber.alternate !== null + ) { + warning( + false, + '%s: Rendered more hooks than during the previous render. This is ' + + 'not currently supported and may lead to unexpected behavior.', + getComponentName(currentlyRenderingFiber.type), + ); + } + } return hook; } function cloneHook(hook: Hook): Hook { - let nextHook: Hook = __DEV__ - ? { - _debugType: ((currentHookNameInDev: any): HookType), - memoizedState: hook.memoizedState, - - baseState: hook.baseState, - queue: hook.queue, - baseUpdate: hook.baseUpdate, + const nextHook: Hook = { + memoizedState: hook.memoizedState, - next: null, - } - : { - 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, + }; if (__DEV__) { + nextHook._debugType = ((currentHookNameInDev: any): HookType); if (currentHookMismatchInDev === null) { if (currentHookNameInDev !== ((hook: any): HookDev)._debugType) { currentHookMismatchInDev = new Error('tracer').stack @@ -540,45 +534,45 @@ function mountWorkInProgressHook(): Hook { } function updateWorkInProgressHook(): Hook { - if (workInProgressHook === null) { - // This is the first hook in the list - if (firstWorkInProgressHook === null) { - currentHook = firstCurrentHook; - if (currentHook === null) { - // This is a newly mounted hook - workInProgressHook = createHook(); - } else { - // Clone the current hook. - workInProgressHook = cloneHook(currentHook); - } - firstWorkInProgressHook = workInProgressHook; - } else { - // There's already a work-in-progress. Reuse it. - currentHook = firstCurrentHook; - workInProgressHook = firstWorkInProgressHook; + // This function is used both for updates and for re-renders triggered by a + // render phase update. It assumes there is either a current hook we can + // clone, or a work-in-progress hook from a previous render pass that we can + // use as a base. When we reach the end of the base list, we must switch to + // the dispatcher used for mounts. + if (nextWorkInProgressHook !== null) { + // There's already a work-in-progress. Reuse it. + workInProgressHook = nextWorkInProgressHook; + nextWorkInProgressHook = workInProgressHook.next; + + currentHook = nextCurrentHook; + nextCurrentHook = currentHook !== null ? currentHook.next : null; + if (nextCurrentHook === null && nextWorkInProgressHook === null) { + // We've reached the end of both lists. Switch to mount dispatcher. + ReactCurrentDispatcher.current = __DEV__ + ? HooksDispatcherOnMountInDEV + : HooksDispatcherOnMount; } } else { - if (workInProgressHook.next === null) { - let hook; - if (currentHook === null) { - // This is a newly mounted hook - hook = createHook(); - } else { - currentHook = currentHook.next; - if (currentHook === null) { - // This is a newly mounted hook - hook = createHook(); - } else { - // Clone the current hook. - hook = cloneHook(currentHook); - } - } - // Append to the end of the list - workInProgressHook = workInProgressHook.next = hook; + // Clone from the current hook. + invariant( + nextCurrentHook !== null, + 'Should have a current hook. This is likely a bug in React. Please ' + + 'file an issue.', + ); + currentHook = nextCurrentHook; + if (workInProgressHook === null) { + // This is the first hook in the list. + workInProgressHook = firstWorkInProgressHook = cloneHook(currentHook); } else { - // There's already a work-in-progress. Reuse it. - workInProgressHook = workInProgressHook.next; - currentHook = currentHook !== null ? currentHook.next : null; + // Append to the end of the list. + workInProgressHook = workInProgressHook.next = cloneHook(currentHook); + } + nextCurrentHook = currentHook.next; + if (nextCurrentHook === null) { + // Check if we've reached the end of the current list. + ReactCurrentDispatcher.current = __DEV__ + ? HooksDispatcherOnMountInDEV + : HooksDispatcherOnMount; } } return workInProgressHook; @@ -601,7 +595,6 @@ function mountContext( if (__DEV__) { currentHookNameInDev = 'useContext'; mountWorkInProgressHook(); - currentHookNameInDev = null; } return readContext(context, observedBits); } @@ -613,7 +606,6 @@ function updateContext( if (__DEV__) { currentHookNameInDev = 'useContext'; updateWorkInProgressHook(); - currentHookNameInDev = null; } return readContext(context, observedBits); } @@ -666,10 +658,10 @@ function updateReducerImpl( initialAction: void | null | A, ): [S, Dispatch] { const queue = hook.queue; - - if (queue === null) { - return mountReducerImpl(hook, reducer, initialState, initialAction); - } + invariant( + queue !== null, + 'Should have a queue. This is likely a bug in React. Please file an issue.', + ); if (numberOfReRenders > 0) { // This is a re-render. Apply the new render phase updates to the previous @@ -868,15 +860,7 @@ function updateRef(initialValue: T): {current: T} { currentHookNameInDev = 'useRef'; } const hook = updateWorkInProgressHook(); - let ref = hook.memoizedState; - if (ref === null) { - ref = {current: initialValue}; - if (__DEV__) { - Object.seal(ref); - } - hook.memoizedState = ref; - } - return ref; + return hook.memoizedState; } function mountEffectImpl( @@ -1250,7 +1234,8 @@ function dispatchAction( const eagerReducer = queue.eagerReducer; if (eagerReducer !== null) { if (__DEV__) { - ReactCurrentDispatcher.current = InvalidHooksDispatcherOnUpdateInDEV; + hooksAccessForbiddenInDEV = true; + ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV; } try { const currentState: S = (queue.eagerState: any); @@ -1272,7 +1257,8 @@ function dispatchAction( // Suppress the error. It will throw again in the render phase. } finally { if (__DEV__) { - ReactCurrentDispatcher.current = InvalidHooksDispatcherOnUpdateInDEV; + hooksAccessForbiddenInDEV = false; + ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV; } } } @@ -1311,45 +1297,6 @@ const HooksDispatcherOnMount: Dispatcher = { useDebugValue: mountDebugValue, }; -const HooksDispatcherOnMountInDEV: Dispatcher = { - readContext, - - useCallback: mountCallback, - useContext: __DEV__ ? mountContext : readContext, - useEffect: mountEffect, - useImperativeHandle: mountImperativeHandle, - useLayoutEffect: mountLayoutEffect, - useMemo(create, deps) { - const prevDispatcher = ReactCurrentDispatcher.current; - ReactCurrentDispatcher.current = InvalidHooksDispatcherOnMountInDEV; - try { - return mountMemo(create, deps); - } finally { - ReactCurrentDispatcher.current = prevDispatcher; - } - }, - useReducer(reducer, initialState, initialAction) { - const prevDispatcher = ReactCurrentDispatcher.current; - ReactCurrentDispatcher.current = InvalidHooksDispatcherOnMountInDEV; - try { - return mountReducer(reducer, initialState, initialAction); - } finally { - ReactCurrentDispatcher.current = prevDispatcher; - } - }, - useRef: mountRef, - useState(initialState) { - const prevDispatcher = ReactCurrentDispatcher.current; - ReactCurrentDispatcher.current = InvalidHooksDispatcherOnMountInDEV; - try { - return mountState(initialState); - } finally { - ReactCurrentDispatcher.current = prevDispatcher; - } - }, - useDebugValue: mountDebugValue, -}; - const HooksDispatcherOnUpdate: Dispatcher = { readContext, @@ -1365,50 +1312,22 @@ const HooksDispatcherOnUpdate: Dispatcher = { useDebugValue: updateDebugValue, }; -const HooksDispatcherOnUpdateInDEV: Dispatcher = { - readContext, - - useCallback: updateCallback, - useContext: __DEV__ ? updateContext : readContext, - useEffect: updateEffect, - useImperativeHandle: updateImperativeHandle, - useLayoutEffect: updateLayoutEffect, - useMemo(create, deps) { - const prevDispatcher = ReactCurrentDispatcher.current; - ReactCurrentDispatcher.current = InvalidHooksDispatcherOnUpdateInDEV; - try { - return updateMemo(create, deps); - } finally { - ReactCurrentDispatcher.current = prevDispatcher; - } - }, - useReducer(reducer, initialState, initialAction) { - const prevDispatcher = ReactCurrentDispatcher.current; - ReactCurrentDispatcher.current = InvalidHooksDispatcherOnUpdateInDEV; - try { - return updateReducer(reducer, initialState, initialAction); - } finally { - ReactCurrentDispatcher.current = prevDispatcher; - } - }, - useRef: updateRef, - useState(initialState) { - const prevDispatcher = ReactCurrentDispatcher.current; - ReactCurrentDispatcher.current = InvalidHooksDispatcherOnUpdateInDEV; - try { - return updateState(initialState); - } finally { - ReactCurrentDispatcher.current = prevDispatcher; - } - }, - useDebugValue: updateDebugValue, -}; - -let InvalidHooksDispatcherOnMountInDEV: Dispatcher | null; -let InvalidHooksDispatcherOnUpdateInDEV: Dispatcher | null; +let hooksAccessForbiddenInDEV = false; +let warnInvalidContextAccess = null; +let warnInvalidHookAccess = null; if (__DEV__) { - const warnInvalidHookAccess = () => { + warnInvalidContextAccess = () => { + warning( + false, + 'Context can only be read while React is rendering. ' + + 'In classes, you can read it in the render method or getDerivedStateFromProps. ' + + 'In function components, you can read it directly in the function body, but not ' + + 'inside Hooks like useReducer() or useMemo().', + ); + }; + + warnInvalidHookAccess = () => { warning( false, 'Hooks can only be called inside the body of a function component. ' + @@ -1416,114 +1335,180 @@ if (__DEV__) { 'https://fb.me/rules-of-hooks', ); }; +} - InvalidHooksDispatcherOnMountInDEV = { - readContext(context, observedBits) { - warning( - false, - 'Context can only be read while React is rendering. ' + - 'In classes, you can read it in the render method or getDerivedStateFromProps. ' + - 'In function components, you can read it directly in the function body, but not ' + - 'inside Hooks like useReducer() or useMemo().', - ); - - return readContext(context, observedBits); - }, +const HooksDispatcherOnMountInDEV: Dispatcher = { + readContext(context, observedBits) { + if (hooksAccessForbiddenInDEV) { + warnInvalidContextAccess(); + } + return readContext(context, observedBits); + }, - useCallback(callback, deps) { + useCallback(callback, deps) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return mountCallback(callback, deps); - }, - useContext(context, observedBits) { + } + return mountCallback(callback, deps); + }, + useContext(context, observedBits) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return mountContext(context, observedBits); - }, - useEffect(create, deps) { + } + return mountContext(context, observedBits); + }, + useEffect(create, deps) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return mountEffect(create, deps); - }, - useImperativeHandle(ref, create, deps) { + } + return mountEffect(create, deps); + }, + useImperativeHandle(ref, create, deps) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return mountImperativeHandle(ref, create, deps); - }, - useLayoutEffect(create, deps) { + } + return mountImperativeHandle(ref, create, deps); + }, + useLayoutEffect(create, deps) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return mountLayoutEffect(create, deps); - }, - useMemo(create, deps) { + } + return mountLayoutEffect(create, deps); + }, + useMemo(create, deps) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { return mountMemo(create, deps); - }, - useReducer(reducer, initialState, initialAction) { + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useReducer(reducer, initialState, initialAction) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { return mountReducer(reducer, initialState, initialAction); - }, - useRef(initialValue) { + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useRef(initialValue) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return mountReducer(initialValue); - }, - useState(initialState) { + } + return mountRef(initialValue); + }, + useState(initialState) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { return mountState(initialState); - }, - useDebugValue(value, formatterFn) { + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useDebugValue(value, formatterFn) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return mountDebugValue(value, formatterFn); - }, - }; - - InvalidHooksDispatcherOnUpdateInDEV = { - readContext(context, observedBits) { - warning( - false, - 'Context can only be read while React is rendering. ' + - 'In classes, you can read it in the render method or getDerivedStateFromProps. ' + - 'In function components, you can read it directly in the function body, but not ' + - 'inside Hooks like useReducer() or useMemo().', - ); + } + return mountDebugValue(value, formatterFn); + }, +}; - return readContext(context, observedBits); - }, +const HooksDispatcherOnUpdateInDEV: Dispatcher = { + readContext(context, observedBits) { + if (hooksAccessForbiddenInDEV) { + warnInvalidContextAccess(); + } + return readContext(context, observedBits); + }, - useCallback(callback, deps) { + useCallback(callback, deps) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return updateCallback(callback, deps); - }, - useContext(context, observedBits) { + } + return updateCallback(callback, deps); + }, + useContext(context, observedBits) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return updateContext(context, observedBits); - }, - useEffect(create, deps) { + } + return updateContext(context, observedBits); + }, + useEffect(create, deps) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return updateEffect(create, deps); - }, - useImperativeHandle(ref, create, deps) { + } + return updateEffect(create, deps); + }, + useImperativeHandle(ref, create, deps) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return updateImperativeHandle(ref, create, deps); - }, - useLayoutEffect(create, deps) { + } + return updateImperativeHandle(ref, create, deps); + }, + useLayoutEffect(create, deps) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return updateLayoutEffect(create, deps); - }, - useMemo(create, deps) { + } + return updateLayoutEffect(create, deps); + }, + useMemo(create, deps) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { return updateMemo(create, deps); - }, - useReducer(reducer, initialState, initialAction) { + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useReducer(reducer, initialState, initialAction) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { return updateReducer(reducer, initialState, initialAction); - }, - useRef(initialValue) { + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useRef(initialValue) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return updateReducer(initialValue); - }, - useState(initialState) { + } + return updateRef(initialValue); + }, + useState(initialState) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { return updateState(initialState); - }, - useDebugValue(value, formatterFn) { + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useDebugValue(value, formatterFn) { + if (hooksAccessForbiddenInDEV) { warnInvalidHookAccess(); - return updateDebugValue(value, formatterFn); - }, - }; -} + } + return updateDebugValue(value, formatterFn); + }, +}; From 1f4c337056e71af3b923bc1447b51d8aeb97e1f6 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 28 Jan 2019 14:47:32 -0800 Subject: [PATCH 5/9] Factoring nits --- .../react-reconciler/src/ReactFiberHooks.js | 149 +++++++----------- 1 file changed, 59 insertions(+), 90 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index b9a08664bb588..d61366dd58797 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -468,7 +468,7 @@ export function resetHooks(): void { numberOfReRenders = 0; } -function createHook(): Hook { +function mountWorkInProgressHook(): Hook { const hook: Hook = { memoizedState: null, @@ -493,42 +493,12 @@ function createHook(): Hook { ); } } - - return hook; -} - -function cloneHook(hook: Hook): Hook { - const nextHook: Hook = { - memoizedState: hook.memoizedState, - - baseState: hook.baseState, - queue: hook.queue, - baseUpdate: hook.baseUpdate, - - next: null, - }; - - if (__DEV__) { - nextHook._debugType = ((currentHookNameInDev: any): HookType); - if (currentHookMismatchInDev === null) { - if (currentHookNameInDev !== ((hook: any): HookDev)._debugType) { - currentHookMismatchInDev = new Error('tracer').stack - .split('\n') - .slice(4) - .join('\n'); - } - } - } - return nextHook; -} - -function mountWorkInProgressHook(): Hook { if (workInProgressHook === null) { // This is the first hook in the list - firstWorkInProgressHook = workInProgressHook = createHook(); + firstWorkInProgressHook = workInProgressHook = hook; } else { // Append to the end of the list - workInProgressHook = workInProgressHook.next = createHook(); + workInProgressHook = workInProgressHook.next = hook; } return workInProgressHook; } @@ -560,12 +530,35 @@ function updateWorkInProgressHook(): Hook { 'file an issue.', ); currentHook = nextCurrentHook; + + const newHook: Hook = { + memoizedState: currentHook.memoizedState, + + baseState: currentHook.baseState, + queue: currentHook.queue, + baseUpdate: currentHook.baseUpdate, + + next: null, + }; + + if (__DEV__) { + newHook._debugType = ((currentHookNameInDev: any): HookType); + if (currentHookMismatchInDev === null) { + if (currentHookNameInDev !== ((currentHook: any): HookDev)._debugType) { + currentHookMismatchInDev = new Error('tracer').stack + .split('\n') + .slice(4) + .join('\n'); + } + } + } + if (workInProgressHook === null) { // This is the first hook in the list. - workInProgressHook = firstWorkInProgressHook = cloneHook(currentHook); + workInProgressHook = firstWorkInProgressHook = newHook; } else { // Append to the end of the list. - workInProgressHook = workInProgressHook.next = cloneHook(currentHook); + workInProgressHook = workInProgressHook.next = newHook; } nextCurrentHook = currentHook.next; if (nextCurrentHook === null) { @@ -610,18 +603,17 @@ function updateContext( return readContext(context, observedBits); } -function mountReducerImpl( - hook: Hook, +function mountReducer( reducer: (S, A) => S, initialState: void | S, initialAction: void | null | A, ): [S, Dispatch] { - if (reducer === basicStateReducer) { - // Special case for `useState`. - if (typeof initialState === 'function') { - initialState = initialState(); - } - } else if (initialAction !== undefined && initialAction !== null) { + if (__DEV__) { + currentHookNameInDev = 'useReducer'; + } + const hook = mountWorkInProgressHook(); + // TODO: Lazy init API will change before release. + if (initialAction !== undefined && initialAction !== null) { initialState = reducer(initialState, initialAction); } hook.memoizedState = hook.baseState = initialState; @@ -639,18 +631,6 @@ function mountReducerImpl( return [hook.memoizedState, dispatch]; } -function mountReducer( - reducer: (S, A) => S, - initialState: void | S, - initialAction: void | null | A, -): [S, Dispatch] { - if (__DEV__) { - currentHookNameInDev = 'useReducer'; - } - const hook = mountWorkInProgressHook(); - return mountReducerImpl(hook, reducer, initialState, initialAction); -} - function updateReducerImpl( hook: Hook, reducer: (S, A) => S, @@ -803,7 +783,23 @@ function mountState( currentHookNameInDev = 'useState'; } const hook = mountWorkInProgressHook(); - return mountReducerImpl(hook, basicStateReducer, initialState); + // TODO: Lazy init API will change before release. + if (typeof initialState === 'function') { + initialState = initialState(); + } + hook.memoizedState = hook.baseState = initialState; + const queue = (hook.queue = { + last: null, + dispatch: null, + eagerReducer: basicStateReducer, + eagerState: initialState, + }); + const dispatch: Dispatch = (queue.dispatch = (dispatchAction.bind( + null, + currentlyRenderingFiber, + queue, + ): any)); + return [hook.memoizedState, dispatch]; } function updateState( @@ -863,25 +859,15 @@ function updateRef(initialValue: T): {current: T} { return hook.memoizedState; } -function mountEffectImpl( - hook, - fiberEffectTag, - hookEffectTag, - create, - deps, -): void { +function mountEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { + const hook = mountWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; sideEffectTag |= fiberEffectTag; hook.memoizedState = pushEffect(hookEffectTag, create, null, nextDeps); } -function updateEffectImpl( - hook, - fiberEffectTag, - hookEffectTag, - create, - deps, -): void { +function updateEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { + const hook = updateWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; let destroy = null; @@ -901,12 +887,7 @@ function updateEffectImpl( } sideEffectTag |= fiberEffectTag; - workInProgressHook.memoizedState = pushEffect( - hookEffectTag, - create, - destroy, - nextDeps, - ); + hook.memoizedState = pushEffect(hookEffectTag, create, destroy, nextDeps); } function mountEffect( @@ -916,9 +897,7 @@ function mountEffect( if (__DEV__) { currentHookNameInDev = 'useEffect'; } - const hook = mountWorkInProgressHook(); return mountEffectImpl( - hook, UpdateEffect | PassiveEffect, UnmountPassive | MountPassive, create, @@ -933,9 +912,7 @@ function updateEffect( if (__DEV__) { currentHookNameInDev = 'useEffect'; } - const hook = updateWorkInProgressHook(); return updateEffectImpl( - hook, UpdateEffect | PassiveEffect, UnmountPassive | MountPassive, create, @@ -950,9 +927,7 @@ function mountLayoutEffect( if (__DEV__) { currentHookNameInDev = 'useLayoutEffect'; } - const hook = mountWorkInProgressHook(); return mountEffectImpl( - hook, UpdateEffect, UnmountMutation | MountLayout, create, @@ -967,9 +942,7 @@ function updateLayoutEffect( if (__DEV__) { currentHookNameInDev = 'useLayoutEffect'; } - const hook = updateWorkInProgressHook(); return updateEffectImpl( - hook, UpdateEffect, UnmountMutation | MountLayout, create, @@ -1015,14 +988,12 @@ function mountImperativeHandle( create !== null ? typeof create : 'null', ); } - const hook = mountWorkInProgressHook(); // TODO: If deps are provided, should we skip comparing the ref itself? const effectDeps = deps !== null && deps !== undefined ? deps.concat([ref]) : [ref]; return mountEffectImpl( - hook, UpdateEffect, UnmountMutation | MountLayout, imperativeHandleEffect.bind(null, create, ref), @@ -1044,14 +1015,12 @@ function updateImperativeHandle( create !== null ? typeof create : 'null', ); } - const hook = updateWorkInProgressHook(); // TODO: If deps are provided, should we skip comparing the ref itself? const effectDeps = deps !== null && deps !== undefined ? deps.concat([ref]) : [ref]; return updateEffectImpl( - hook, UpdateEffect, UnmountMutation | MountLayout, imperativeHandleEffect.bind(null, create, ref), @@ -1286,7 +1255,7 @@ const HooksDispatcherOnMount: Dispatcher = { readContext, useCallback: mountCallback, - useContext: __DEV__ ? mountContext : readContext, + useContext: readContext, useEffect: mountEffect, useImperativeHandle: mountImperativeHandle, useLayoutEffect: mountLayoutEffect, @@ -1301,7 +1270,7 @@ const HooksDispatcherOnUpdate: Dispatcher = { readContext, useCallback: updateCallback, - useContext: __DEV__ ? updateContext : readContext, + useContext: readContext, useEffect: updateEffect, useImperativeHandle: updateImperativeHandle, useLayoutEffect: updateLayoutEffect, From 0e602b9f61c4a7d93fde857c5fe0c8969fe2351d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 28 Jan 2019 16:23:07 -0800 Subject: [PATCH 6/9] Fix Flow Had to cheat more than I would like --- .../react-reconciler/src/ReactFiberHooks.js | 525 +++++++++--------- 1 file changed, 263 insertions(+), 262 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index d61366dd58797..c4422098ebf05 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -44,6 +44,10 @@ import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork'; const {ReactCurrentDispatcher} = ReactSharedInternals; export type Dispatcher = { + readContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T, useState(initialState: (() => S) | S): [S, Dispatch>], useReducer( reducer: (S, A) => S, @@ -64,7 +68,7 @@ export type Dispatcher = { create: () => T, deps: Array | void | null, ): void, - useDebugValue(value: any, formatterFn: ?(value: any) => any): void, + useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void, }; type Update = { @@ -321,7 +325,6 @@ export function renderWithHooks( currentlyRenderingFiber = workInProgress; firstCurrentHook = nextCurrentHook = current !== null ? current.memoizedState : null; - nextWorkInProgressHook = null; // The following should have already been reset // currentHook = null; @@ -480,7 +483,7 @@ function mountWorkInProgressHook(): Hook { }; if (__DEV__) { - hook._debugType = ((currentHookNameInDev: any): HookType); + (hook: any)._debugType = (currentHookNameInDev: any); if ( currentlyRenderingFiber !== null && currentlyRenderingFiber.alternate !== null @@ -542,7 +545,7 @@ function updateWorkInProgressHook(): Hook { }; if (__DEV__) { - newHook._debugType = ((currentHookNameInDev: any): HookType); + (newHook: any)._debugType = (currentHookNameInDev: any); if (currentHookMismatchInDev === null) { if (currentHookNameInDev !== ((currentHook: any): HookDev)._debugType) { currentHookMismatchInDev = new Error('tracer').stack @@ -586,7 +589,6 @@ function mountContext( observedBits: void | number | boolean, ): T { if (__DEV__) { - currentHookNameInDev = 'useContext'; mountWorkInProgressHook(); } return readContext(context, observedBits); @@ -597,7 +599,6 @@ function updateContext( observedBits: void | number | boolean, ): T { if (__DEV__) { - currentHookNameInDev = 'useContext'; updateWorkInProgressHook(); } return readContext(context, observedBits); @@ -608,12 +609,10 @@ function mountReducer( initialState: void | S, initialAction: void | null | A, ): [S, Dispatch] { - if (__DEV__) { - currentHookNameInDev = 'useReducer'; - } const hook = mountWorkInProgressHook(); // TODO: Lazy init API will change before release. if (initialAction !== undefined && initialAction !== null) { + // $FlowFixMe - Must express with overloading. initialState = reducer(initialState, initialAction); } hook.memoizedState = hook.baseState = initialState; @@ -625,18 +624,20 @@ function mountReducer( }); const dispatch: Dispatch = (queue.dispatch = (dispatchAction.bind( null, - currentlyRenderingFiber, + // Flow doesn't know this is non-null, but we do. + // $FlowFixMe – I have no idea why Flow is still complaining about this. + ((currentlyRenderingFiber: any): Fiber), queue, ): any)); return [hook.memoizedState, dispatch]; } -function updateReducerImpl( - hook: Hook, +function updateReducer( reducer: (S, A) => S, initialState: void | S, initialAction: void | null | A, ): [S, Dispatch] { + const hook = updateWorkInProgressHook(); const queue = hook.queue; invariant( queue !== null, @@ -652,7 +653,7 @@ function updateReducerImpl( const firstRenderPhaseUpdate = renderPhaseUpdates.get(queue); if (firstRenderPhaseUpdate !== undefined) { renderPhaseUpdates.delete(queue); - let newState = workInProgressHook.memoizedState; + let newState = hook.memoizedState; let update = firstRenderPhaseUpdate; do { // Process this render phase update. We don't have to check the @@ -764,24 +765,9 @@ function updateReducerImpl( return [hook.memoizedState, dispatch]; } -function updateReducer( - reducer: (S, A) => S, - initialState: void | S, - initialAction: void | null | A, -): [S, Dispatch] { - if (__DEV__) { - currentHookNameInDev = 'useReducer'; - } - const hook = updateWorkInProgressHook(); - return updateReducerImpl(hook, reducer, initialState, initialAction); -} - function mountState( initialState: (() => S) | S, ): [S, Dispatch>] { - if (__DEV__) { - currentHookNameInDev = 'useState'; - } const hook = mountWorkInProgressHook(); // TODO: Lazy init API will change before release. if (typeof initialState === 'function') { @@ -794,9 +780,12 @@ function mountState( eagerReducer: basicStateReducer, eagerState: initialState, }); - const dispatch: Dispatch = (queue.dispatch = (dispatchAction.bind( + const dispatch: Dispatch< + BasicStateAction, + > = (queue.dispatch = (dispatchAction.bind( null, - currentlyRenderingFiber, + // Flow doesn't know this is non-null, but we do. + ((currentlyRenderingFiber: any): Fiber), queue, ): any)); return [hook.memoizedState, dispatch]; @@ -805,11 +794,7 @@ function mountState( function updateState( initialState: (() => S) | S, ): [S, Dispatch>] { - if (__DEV__) { - currentHookNameInDev = 'useState'; - } - const hook = updateWorkInProgressHook(); - return updateReducerImpl(hook, basicStateReducer, initialState); + return updateReducer(basicStateReducer, (initialState: any)); } function pushEffect(tag, create, destroy, deps) { @@ -839,9 +824,6 @@ function pushEffect(tag, create, destroy, deps) { } function mountRef(initialValue: T): {current: T} { - if (__DEV__) { - currentHookNameInDev = 'useRef'; - } const hook = mountWorkInProgressHook(); const ref = {current: initialValue}; if (__DEV__) { @@ -852,9 +834,6 @@ function mountRef(initialValue: T): {current: T} { } function updateRef(initialValue: T): {current: T} { - if (__DEV__) { - currentHookNameInDev = 'useRef'; - } const hook = updateWorkInProgressHook(); return hook.memoizedState; } @@ -878,9 +857,6 @@ function updateEffectImpl(fiberEffectTag, hookEffectTag, create, deps): void { const prevDeps = prevEffect.deps; if (areHookInputsEqual(nextDeps, prevDeps)) { pushEffect(NoHookEffect, create, destroy, nextDeps); - if (__DEV__) { - currentHookNameInDev = null; - } return; } } @@ -894,9 +870,6 @@ function mountEffect( create: () => mixed, deps: Array | void | null, ): void { - if (__DEV__) { - currentHookNameInDev = 'useEffect'; - } return mountEffectImpl( UpdateEffect | PassiveEffect, UnmountPassive | MountPassive, @@ -909,9 +882,6 @@ function updateEffect( create: () => mixed, deps: Array | void | null, ): void { - if (__DEV__) { - currentHookNameInDev = 'useEffect'; - } return updateEffectImpl( UpdateEffect | PassiveEffect, UnmountPassive | MountPassive, @@ -924,9 +894,6 @@ function mountLayoutEffect( create: () => mixed, deps: Array | void | null, ): void { - if (__DEV__) { - currentHookNameInDev = 'useLayoutEffect'; - } return mountEffectImpl( UpdateEffect, UnmountMutation | MountLayout, @@ -939,9 +906,6 @@ function updateLayoutEffect( create: () => mixed, deps: Array | void | null, ): void { - if (__DEV__) { - currentHookNameInDev = 'useLayoutEffect'; - } return updateEffectImpl( UpdateEffect, UnmountMutation | MountLayout, @@ -950,7 +914,10 @@ function updateLayoutEffect( ); } -function imperativeHandleEffect(create, ref) { +function imperativeHandleEffect( + create: () => T, + ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, +) { if (typeof ref === 'function') { const refCallback = ref; const inst = create(); @@ -974,13 +941,12 @@ function imperativeHandleEffect(create, ref) { } } -function mountImperativeHandle( +function mountImperativeHandle( ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, create: () => T, deps: Array | void | null, ): void { if (__DEV__) { - currentHookNameInDev = 'useImperativeHandle'; warning( typeof create === 'function', 'Expected useImperativeHandle() second argument to be a function ' + @@ -1001,13 +967,12 @@ function mountImperativeHandle( ); } -function updateImperativeHandle( +function updateImperativeHandle( ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, create: () => T, deps: Array | void | null, ): void { if (__DEV__) { - currentHookNameInDev = 'useImperativeHandle'; warning( typeof create === 'function', 'Expected useImperativeHandle() second argument to be a function ' + @@ -1028,11 +993,7 @@ function updateImperativeHandle( ); } -function mountDebugValue(value: any, formatterFn: ?(value: any) => any): void { - if (__DEV__) { - currentHookNameInDev = 'useDebugValue'; - } - +function mountDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { // This hook is normally a no-op. // The react-debug-hooks package injects its own implementation // so that e.g. DevTools can display custom hook values. @@ -1041,9 +1002,6 @@ function mountDebugValue(value: any, formatterFn: ?(value: any) => any): void { const updateDebugValue = mountDebugValue; function mountCallback(callback: T, deps: Array | void | null): T { - if (__DEV__) { - currentHookNameInDev = 'useCallback'; - } const hook = mountWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; hook.memoizedState = [callback, nextDeps]; @@ -1051,9 +1009,6 @@ function mountCallback(callback: T, deps: Array | void | null): T { } function updateCallback(callback: T, deps: Array | void | null): T { - if (__DEV__) { - currentHookNameInDev = 'useCallback'; - } const hook = updateWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; const prevState = hook.memoizedState; @@ -1061,15 +1016,11 @@ function updateCallback(callback: T, deps: Array | void | null): T { if (nextDeps !== null) { const prevDeps: Array | null = prevState[1]; if (areHookInputsEqual(nextDeps, prevDeps)) { - currentHookNameInDev = null; return prevState[0]; } } } hook.memoizedState = [callback, nextDeps]; - if (__DEV__) { - currentHookNameInDev = null; - } return callback; } @@ -1077,9 +1028,6 @@ function mountMemo( nextCreate: () => T, deps: Array | void | null, ): T { - if (__DEV__) { - currentHookNameInDev = 'useMemo'; - } const hook = mountWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; const nextValue = nextCreate(); @@ -1091,9 +1039,6 @@ function updateMemo( nextCreate: () => T, deps: Array | void | null, ): T { - if (__DEV__) { - currentHookNameInDev = 'useMemo'; - } const hook = updateWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; const prevState = hook.memoizedState; @@ -1102,9 +1047,6 @@ function updateMemo( if (nextDeps !== null) { const prevDeps: Array | null = prevState[1]; if (areHookInputsEqual(nextDeps, prevDeps)) { - if (__DEV__) { - currentHookNameInDev = null; - } return prevState[0]; } } @@ -1282,11 +1224,12 @@ const HooksDispatcherOnUpdate: Dispatcher = { }; let hooksAccessForbiddenInDEV = false; -let warnInvalidContextAccess = null; -let warnInvalidHookAccess = null; + +let HooksDispatcherOnMountInDEV: Dispatcher | null = null; +let HooksDispatcherOnUpdateInDEV: Dispatcher | null = null; if (__DEV__) { - warnInvalidContextAccess = () => { + const warnInvalidContextAccess = () => { warning( false, 'Context can only be read while React is rendering. ' + @@ -1296,7 +1239,7 @@ if (__DEV__) { ); }; - warnInvalidHookAccess = () => { + const warnInvalidHookAccess = () => { warning( false, 'Hooks can only be called inside the body of a function component. ' + @@ -1304,180 +1247,238 @@ if (__DEV__) { 'https://fb.me/rules-of-hooks', ); }; -} -const HooksDispatcherOnMountInDEV: Dispatcher = { - readContext(context, observedBits) { - if (hooksAccessForbiddenInDEV) { - warnInvalidContextAccess(); - } - return readContext(context, observedBits); - }, + HooksDispatcherOnMountInDEV = { + readContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + if (hooksAccessForbiddenInDEV) { + warnInvalidContextAccess(); + } + return readContext(context, observedBits); + }, - useCallback(callback, deps) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return mountCallback(callback, deps); - }, - useContext(context, observedBits) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return mountContext(context, observedBits); - }, - useEffect(create, deps) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return mountEffect(create, deps); - }, - useImperativeHandle(ref, create, deps) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return mountImperativeHandle(ref, create, deps); - }, - useLayoutEffect(create, deps) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return mountLayoutEffect(create, deps); - }, - useMemo(create, deps) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; - try { - return mountMemo(create, deps); - } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; - } - }, - useReducer(reducer, initialState, initialAction) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; - try { - return mountReducer(reducer, initialState, initialAction); - } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; - } - }, - useRef(initialValue) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return mountRef(initialValue); - }, - useState(initialState) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; - try { - return mountState(initialState); - } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; - } - }, - useDebugValue(value, formatterFn) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return mountDebugValue(value, formatterFn); - }, -}; + useCallback(callback: T, deps: Array | void | null): T { + currentHookNameInDev = 'useCallback'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return mountCallback(callback, deps); + }, + useContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + currentHookNameInDev = 'useContext'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return mountContext(context, observedBits); + }, + useEffect(create: () => mixed, deps: Array | void | null): void { + currentHookNameInDev = 'useEffect'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return mountEffect(create, deps); + }, + useImperativeHandle( + ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, + create: () => T, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useImperativeHandle'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return mountImperativeHandle(ref, create, deps); + }, + useLayoutEffect( + create: () => mixed, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useLayoutEffect'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return mountLayoutEffect(create, deps); + }, + useMemo(create: () => T, deps: Array | void | null): T { + currentHookNameInDev = 'useMemo'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { + return mountMemo(create, deps); + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useReducer( + reducer: (S, A) => S, + initialState: S, + initialAction: A | void | null, + ): [S, Dispatch] { + currentHookNameInDev = 'useReducer'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { + return mountReducer(reducer, initialState, initialAction); + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useRef(initialValue: T): {current: T} { + currentHookNameInDev = 'useRef'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return mountRef(initialValue); + }, + useState( + initialState: (() => S) | S, + ): [S, Dispatch>] { + currentHookNameInDev = 'useState'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { + return mountState(initialState); + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { + currentHookNameInDev = 'useDebugValue'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return mountDebugValue(value, formatterFn); + }, + }; -const HooksDispatcherOnUpdateInDEV: Dispatcher = { - readContext(context, observedBits) { - if (hooksAccessForbiddenInDEV) { - warnInvalidContextAccess(); - } - return readContext(context, observedBits); - }, + HooksDispatcherOnUpdateInDEV = { + readContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + if (hooksAccessForbiddenInDEV) { + warnInvalidContextAccess(); + } + return readContext(context, observedBits); + }, - useCallback(callback, deps) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return updateCallback(callback, deps); - }, - useContext(context, observedBits) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return updateContext(context, observedBits); - }, - useEffect(create, deps) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return updateEffect(create, deps); - }, - useImperativeHandle(ref, create, deps) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return updateImperativeHandle(ref, create, deps); - }, - useLayoutEffect(create, deps) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return updateLayoutEffect(create, deps); - }, - useMemo(create, deps) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; - try { - return updateMemo(create, deps); - } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; - } - }, - useReducer(reducer, initialState, initialAction) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; - try { - return updateReducer(reducer, initialState, initialAction); - } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; - } - }, - useRef(initialValue) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return updateRef(initialValue); - }, - useState(initialState) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; - try { - return updateState(initialState); - } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; - } - }, - useDebugValue(value, formatterFn) { - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - return updateDebugValue(value, formatterFn); - }, -}; + useCallback(callback: T, deps: Array | void | null): T { + currentHookNameInDev = 'useCallback'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return updateCallback(callback, deps); + }, + useContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + currentHookNameInDev = 'useContext'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return updateContext(context, observedBits); + }, + useEffect(create: () => mixed, deps: Array | void | null): void { + currentHookNameInDev = 'useEffect'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return updateEffect(create, deps); + }, + useImperativeHandle( + ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, + create: () => T, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useImperativeHandle'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return updateImperativeHandle(ref, create, deps); + }, + useLayoutEffect( + create: () => mixed, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useLayoutEffect'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return updateLayoutEffect(create, deps); + }, + useMemo(create: () => T, deps: Array | void | null): T { + currentHookNameInDev = 'useMemo'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { + return updateMemo(create, deps); + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useReducer( + reducer: (S, A) => S, + initialState: S, + initialAction: A | void | null, + ): [S, Dispatch] { + currentHookNameInDev = 'useReducer'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { + return updateReducer(reducer, initialState, initialAction); + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useRef(initialValue: T): {current: T} { + currentHookNameInDev = 'useRef'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return updateRef(initialValue); + }, + useState( + initialState: (() => S) | S, + ): [S, Dispatch>] { + currentHookNameInDev = 'useState'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; + hooksAccessForbiddenInDEV = true; + try { + return updateState(initialState); + } finally { + hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + } + }, + useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { + currentHookNameInDev = 'useDebugValue'; + if (hooksAccessForbiddenInDEV) { + warnInvalidHookAccess(); + } + return updateDebugValue(value, formatterFn); + }, + }; +} From 5a739725cd126e9cb1353279c64d46617712b3b7 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 29 Jan 2019 10:40:40 -0800 Subject: [PATCH 7/9] More Flow nits --- .../react-reconciler/src/ReactFiberHooks.js | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index c4422098ebf05..7bef51f42d0da 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -620,23 +620,18 @@ function mountReducer( last: null, dispatch: null, eagerReducer: reducer, - eagerState: initialState, + eagerState: (initialState: any), }); const dispatch: Dispatch = (queue.dispatch = (dispatchAction.bind( null, // Flow doesn't know this is non-null, but we do. - // $FlowFixMe – I have no idea why Flow is still complaining about this. ((currentlyRenderingFiber: any): Fiber), queue, ): any)); return [hook.memoizedState, dispatch]; } -function updateReducer( - reducer: (S, A) => S, - initialState: void | S, - initialAction: void | null | A, -): [S, Dispatch] { +function updateReducer(reducer: (S, A) => S): [S, Dispatch] { const hook = updateWorkInProgressHook(); const queue = hook.queue; invariant( @@ -778,7 +773,7 @@ function mountState( last: null, dispatch: null, eagerReducer: basicStateReducer, - eagerState: initialState, + eagerState: (initialState: any), }); const dispatch: Dispatch< BasicStateAction, @@ -791,10 +786,8 @@ function mountState( return [hook.memoizedState, dispatch]; } -function updateState( - initialState: (() => S) | S, -): [S, Dispatch>] { - return updateReducer(basicStateReducer, (initialState: any)); +function updateState(): [S, Dispatch>] { + return updateReducer(basicStateReducer); } function pushEffect(tag, create, destroy, deps) { @@ -1446,7 +1439,7 @@ if (__DEV__) { const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; hooksAccessForbiddenInDEV = true; try { - return updateReducer(reducer, initialState, initialAction); + return updateReducer(reducer); } finally { hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; } @@ -1468,7 +1461,7 @@ if (__DEV__) { const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; hooksAccessForbiddenInDEV = true; try { - return updateState(initialState); + return updateState(); } finally { hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; } From f4afcbb785cd7b8b5286dd4a49c88aeab10eff28 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 29 Jan 2019 16:02:32 -0800 Subject: [PATCH 8/9] Switch back to using a special dispatcher for nested hooks in DEV In order for this strategy to work, I had to revert progressive enhancement support (appending hooks to the end). It was previously a warning but now it results in an error. We'll reconsider later. --- ...DOMServerIntegrationHooks-test.internal.js | 30 +- .../src/server/ReactPartialRendererHooks.js | 3 + .../react-reconciler/src/ReactFiberHooks.js | 315 ++++++++++++------ .../src/__tests__/ReactHooks-test.internal.js | 19 +- ...eactHooksWithNoopRenderer-test.internal.js | 42 ++- 5 files changed, 268 insertions(+), 141 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js index d3a7dae6122ce..5fa5c14f49ad9 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js @@ -432,21 +432,25 @@ describe('ReactDOMServerHooks', () => { expect(domNode.textContent).toEqual('hi'); }); - itRenders('with a warning for useRef inside useReducer', async render => { - function App() { - const [value, dispatch] = useReducer((state, action) => { - useRef(0); - return state + 1; - }, 0); - if (value === 0) { - dispatch(); + itThrowsWhenRendering( + 'with a warning for useRef inside useReducer', + async render => { + function App() { + const [value, dispatch] = useReducer((state, action) => { + useRef(0); + return state + 1; + }, 0); + if (value === 0) { + dispatch(); + } + return value; } - return value; - } - const domNode = await render(, 1); - expect(domNode.textContent).toEqual('1'); - }); + const domNode = await render(, 1); + expect(domNode.textContent).toEqual('1'); + }, + 'Rendered more hooks than during the previous render', + ); itRenders('with a warning for useRef inside useState', async render => { function App() { diff --git a/packages/react-dom/src/server/ReactPartialRendererHooks.js b/packages/react-dom/src/server/ReactPartialRendererHooks.js index 749437e1dd3da..c462487e380ff 100644 --- a/packages/react-dom/src/server/ReactPartialRendererHooks.js +++ b/packages/react-dom/src/server/ReactPartialRendererHooks.js @@ -113,6 +113,9 @@ function areHookInputsEqual( } function createHook(): Hook { + if (numberOfReRenders > 0) { + invariant(false, 'Rendered more hooks than during the previous render'); + } return { memoizedState: null, queue: null, diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 7bef51f42d0da..50546d4bb9e4f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -519,18 +519,11 @@ function updateWorkInProgressHook(): Hook { currentHook = nextCurrentHook; nextCurrentHook = currentHook !== null ? currentHook.next : null; - if (nextCurrentHook === null && nextWorkInProgressHook === null) { - // We've reached the end of both lists. Switch to mount dispatcher. - ReactCurrentDispatcher.current = __DEV__ - ? HooksDispatcherOnMountInDEV - : HooksDispatcherOnMount; - } } else { // Clone from the current hook. invariant( nextCurrentHook !== null, - 'Should have a current hook. This is likely a bug in React. Please ' + - 'file an issue.', + 'Rendered more hooks than during the previous render.', ); currentHook = nextCurrentHook; @@ -564,12 +557,6 @@ function updateWorkInProgressHook(): Hook { workInProgressHook = workInProgressHook.next = newHook; } nextCurrentHook = currentHook.next; - if (nextCurrentHook === null) { - // Check if we've reached the end of the current list. - ReactCurrentDispatcher.current = __DEV__ - ? HooksDispatcherOnMountInDEV - : HooksDispatcherOnMount; - } } return workInProgressHook; } @@ -1137,9 +1124,10 @@ function dispatchAction( // same as the current state, we may be able to bail out entirely. const eagerReducer = queue.eagerReducer; if (eagerReducer !== null) { + let prevDispatcher; if (__DEV__) { - hooksAccessForbiddenInDEV = true; - ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV; + prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; } try { const currentState: S = (queue.eagerState: any); @@ -1161,8 +1149,7 @@ function dispatchAction( // Suppress the error. It will throw again in the render phase. } finally { if (__DEV__) { - hooksAccessForbiddenInDEV = false; - ReactCurrentDispatcher.current = HooksDispatcherOnUpdateInDEV; + ReactCurrentDispatcher.current = prevDispatcher; } } } @@ -1216,10 +1203,10 @@ const HooksDispatcherOnUpdate: Dispatcher = { useDebugValue: updateDebugValue, }; -let hooksAccessForbiddenInDEV = false; - let HooksDispatcherOnMountInDEV: Dispatcher | null = null; let HooksDispatcherOnUpdateInDEV: Dispatcher | null = null; +let InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher | null = null; +let InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher | null = null; if (__DEV__) { const warnInvalidContextAccess = () => { @@ -1246,17 +1233,11 @@ if (__DEV__) { context: ReactContext, observedBits: void | number | boolean, ): T { - if (hooksAccessForbiddenInDEV) { - warnInvalidContextAccess(); - } return readContext(context, observedBits); }, useCallback(callback: T, deps: Array | void | null): T { currentHookNameInDev = 'useCallback'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } return mountCallback(callback, deps); }, useContext( @@ -1264,16 +1245,10 @@ if (__DEV__) { observedBits: void | number | boolean, ): T { currentHookNameInDev = 'useContext'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } return mountContext(context, observedBits); }, useEffect(create: () => mixed, deps: Array | void | null): void { currentHookNameInDev = 'useEffect'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } return mountEffect(create, deps); }, useImperativeHandle( @@ -1282,9 +1257,6 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useImperativeHandle'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } return mountImperativeHandle(ref, create, deps); }, useLayoutEffect( @@ -1292,22 +1264,16 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useLayoutEffect'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } return mountLayoutEffect(create, deps); }, useMemo(create: () => T, deps: Array | void | null): T { currentHookNameInDev = 'useMemo'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { return mountMemo(create, deps); } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + ReactCurrentDispatcher.current = prevDispatcher; } }, useReducer( @@ -1316,44 +1282,32 @@ if (__DEV__) { initialAction: A | void | null, ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { return mountReducer(reducer, initialState, initialAction); } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + ReactCurrentDispatcher.current = prevDispatcher; } }, useRef(initialValue: T): {current: T} { currentHookNameInDev = 'useRef'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } return mountRef(initialValue); }, useState( initialState: (() => S) | S, ): [S, Dispatch>] { currentHookNameInDev = 'useState'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; try { return mountState(initialState); } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + ReactCurrentDispatcher.current = prevDispatcher; } }, useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { currentHookNameInDev = 'useDebugValue'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } return mountDebugValue(value, formatterFn); }, }; @@ -1363,17 +1317,192 @@ if (__DEV__) { context: ReactContext, observedBits: void | number | boolean, ): T { - if (hooksAccessForbiddenInDEV) { - warnInvalidContextAccess(); + return readContext(context, observedBits); + }, + + useCallback(callback: T, deps: Array | void | null): T { + currentHookNameInDev = 'useCallback'; + return updateCallback(callback, deps); + }, + useContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + currentHookNameInDev = 'useContext'; + return updateContext(context, observedBits); + }, + useEffect(create: () => mixed, deps: Array | void | null): void { + currentHookNameInDev = 'useEffect'; + return updateEffect(create, deps); + }, + useImperativeHandle( + ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, + create: () => T, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useImperativeHandle'; + return updateImperativeHandle(ref, create, deps); + }, + useLayoutEffect( + create: () => mixed, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useLayoutEffect'; + return updateLayoutEffect(create, deps); + }, + useMemo(create: () => T, deps: Array | void | null): T { + currentHookNameInDev = 'useMemo'; + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; + try { + return updateMemo(create, deps); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; } + }, + useReducer( + reducer: (S, A) => S, + initialState: S, + initialAction: A | void | null, + ): [S, Dispatch] { + currentHookNameInDev = 'useReducer'; + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; + try { + return updateReducer(reducer); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useRef(initialValue: T): {current: T} { + currentHookNameInDev = 'useRef'; + return updateRef(initialValue); + }, + useState( + initialState: (() => S) | S, + ): [S, Dispatch>] { + currentHookNameInDev = 'useState'; + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; + try { + return updateState(); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { + currentHookNameInDev = 'useDebugValue'; + return updateDebugValue(value, formatterFn); + }, + }; + + InvalidNestedHooksDispatcherOnMountInDEV = { + readContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + warnInvalidContextAccess(); return readContext(context, observedBits); }, useCallback(callback: T, deps: Array | void | null): T { currentHookNameInDev = 'useCallback'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); + warnInvalidHookAccess(); + return mountCallback(callback, deps); + }, + useContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + currentHookNameInDev = 'useContext'; + warnInvalidHookAccess(); + return mountContext(context, observedBits); + }, + useEffect(create: () => mixed, deps: Array | void | null): void { + currentHookNameInDev = 'useEffect'; + warnInvalidHookAccess(); + return mountEffect(create, deps); + }, + useImperativeHandle( + ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, + create: () => T, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useImperativeHandle'; + warnInvalidHookAccess(); + return mountImperativeHandle(ref, create, deps); + }, + useLayoutEffect( + create: () => mixed, + deps: Array | void | null, + ): void { + currentHookNameInDev = 'useLayoutEffect'; + warnInvalidHookAccess(); + return mountLayoutEffect(create, deps); + }, + useMemo(create: () => T, deps: Array | void | null): T { + currentHookNameInDev = 'useMemo'; + warnInvalidHookAccess(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; + try { + return mountMemo(create, deps); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useReducer( + reducer: (S, A) => S, + initialState: S, + initialAction: A | void | null, + ): [S, Dispatch] { + currentHookNameInDev = 'useReducer'; + warnInvalidHookAccess(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; + try { + return mountReducer(reducer, initialState, initialAction); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; + } + }, + useRef(initialValue: T): {current: T} { + currentHookNameInDev = 'useRef'; + warnInvalidHookAccess(); + return mountRef(initialValue); + }, + useState( + initialState: (() => S) | S, + ): [S, Dispatch>] { + currentHookNameInDev = 'useState'; + warnInvalidHookAccess(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnMountInDEV; + try { + return mountState(initialState); + } finally { + ReactCurrentDispatcher.current = prevDispatcher; } + }, + useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { + currentHookNameInDev = 'useDebugValue'; + warnInvalidHookAccess(); + return mountDebugValue(value, formatterFn); + }, + }; + + InvalidNestedHooksDispatcherOnUpdateInDEV = { + readContext( + context: ReactContext, + observedBits: void | number | boolean, + ): T { + warnInvalidContextAccess(); + return readContext(context, observedBits); + }, + + useCallback(callback: T, deps: Array | void | null): T { + currentHookNameInDev = 'useCallback'; + warnInvalidHookAccess(); return updateCallback(callback, deps); }, useContext( @@ -1381,16 +1510,12 @@ if (__DEV__) { observedBits: void | number | boolean, ): T { currentHookNameInDev = 'useContext'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } + warnInvalidHookAccess(); return updateContext(context, observedBits); }, useEffect(create: () => mixed, deps: Array | void | null): void { currentHookNameInDev = 'useEffect'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } + warnInvalidHookAccess(); return updateEffect(create, deps); }, useImperativeHandle( @@ -1399,9 +1524,7 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useImperativeHandle'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } + warnInvalidHookAccess(); return updateImperativeHandle(ref, create, deps); }, useLayoutEffect( @@ -1409,22 +1532,18 @@ if (__DEV__) { deps: Array | void | null, ): void { currentHookNameInDev = 'useLayoutEffect'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } + warnInvalidHookAccess(); return updateLayoutEffect(create, deps); }, useMemo(create: () => T, deps: Array | void | null): T { currentHookNameInDev = 'useMemo'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; + warnInvalidHookAccess(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { return updateMemo(create, deps); } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + ReactCurrentDispatcher.current = prevDispatcher; } }, useReducer( @@ -1433,44 +1552,36 @@ if (__DEV__) { initialAction: A | void | null, ): [S, Dispatch] { currentHookNameInDev = 'useReducer'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; + warnInvalidHookAccess(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { return updateReducer(reducer); } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + ReactCurrentDispatcher.current = prevDispatcher; } }, useRef(initialValue: T): {current: T} { currentHookNameInDev = 'useRef'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } + warnInvalidHookAccess(); return updateRef(initialValue); }, useState( initialState: (() => S) | S, ): [S, Dispatch>] { currentHookNameInDev = 'useState'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } - const prevHooksAccessForbiddenInDEV = hooksAccessForbiddenInDEV; - hooksAccessForbiddenInDEV = true; + warnInvalidHookAccess(); + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { return updateState(); } finally { - hooksAccessForbiddenInDEV = prevHooksAccessForbiddenInDEV; + ReactCurrentDispatcher.current = prevDispatcher; } }, useDebugValue(value: T, formatterFn: ?(value: T) => mixed): void { currentHookNameInDev = 'useDebugValue'; - if (hooksAccessForbiddenInDEV) { - warnInvalidHookAccess(); - } + warnInvalidHookAccess(); return updateDebugValue(value, formatterFn); }, }; diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 8535e977673ff..0a37db0d37b89 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -803,7 +803,10 @@ describe('ReactHooks', () => { }); it('warns when calling hooks inside useReducer', () => { - const {useReducer, useRef} = React; + const {useReducer, useState, useRef} = React; + + spyOnDev(console, 'error'); + function App() { const [value, dispatch] = useReducer((state, action) => { useRef(0); @@ -812,11 +815,19 @@ describe('ReactHooks', () => { if (value === 0) { dispatch('foo'); } + useState(); return value; } - expect(() => ReactTestRenderer.create()).toWarnDev( - 'Hooks can only be called inside the body of a function component', - ); + expect(() => { + ReactTestRenderer.create(); + }).toThrow('Rendered more hooks than during the previous render.'); + + if (__DEV__) { + expect(console.error).toHaveBeenCalledTimes(3); + expect(console.error.calls.argsFor(0)[0]).toContain( + 'Hooks can only be called inside the body of a function component', + ); + } }); it("throws when calling hooks inside useState's initialize function", () => { diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index fe67d1ab6234a..363ef3bd42393 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -1596,11 +1596,11 @@ describe('ReactHooksWithNoopRenderer', () => { }); }); - describe('progressive enhancement', () => { + describe('progressive enhancement (not supported)', () => { it('mount additional state', () => { let updateA; let updateB; - let updateC; + // let updateC; function App(props) { const [A, _updateA] = useState(0); @@ -1610,9 +1610,7 @@ describe('ReactHooksWithNoopRenderer', () => { let C; if (props.loadC) { - const [_C, _updateC] = useState(0); - C = _C; - updateC = _updateC; + useState(0); } else { C = '[not loaded]'; } @@ -1636,14 +1634,14 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); expect(() => { expect(ReactNoop.flush()).toEqual(['A: 2, B: 3, C: 0']); - }).toWarnDev([ - 'App: Rendered more hooks than during the previous render', - ]); - expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 0')]); + }).toThrow('Rendered more hooks than during the previous render'); - updateC(4); - expect(ReactNoop.flush()).toEqual(['A: 2, B: 3, C: 4']); - expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 4')]); + // Uncomment if/when we support this again + // expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 0')]); + + // updateC(4); + // expect(ReactNoop.flush()).toEqual(['A: 2, B: 3, C: 4']); + // expect(ReactNoop.getChildren()).toEqual([span('A: 2, B: 3, C: 4')]); }); it('unmount state', () => { @@ -1714,17 +1712,17 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); expect(() => { expect(ReactNoop.flush()).toEqual([]); - }).toWarnDev([ - 'App: Rendered more hooks than during the previous render', - ]); - flushPassiveEffects(); - expect(ReactNoop.clearYields()).toEqual(['Mount B']); + }).toThrow('Rendered more hooks than during the previous render'); - ReactNoop.render(); - expect(() => ReactNoop.flush()).toThrow( - 'Rendered fewer hooks than expected. This may be caused by an ' + - 'accidental early return statement.', - ); + // Uncomment if/when we support this again + // flushPassiveEffects(); + // expect(ReactNoop.clearYields()).toEqual(['Mount B']); + + // ReactNoop.render(); + // expect(() => ReactNoop.flush()).toThrow( + // 'Rendered fewer hooks than expected. This may be caused by an ' + + // 'accidental early return statement.', + // ); }); }); }); From 5a76a40f315e06850119f0055a47490d0fe43518 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 29 Jan 2019 16:23:29 -0800 Subject: [PATCH 9/9] Always pass args to updateState and updateReducer Even though the extra args are only used on mount, to ensure type consistency. --- .../react-reconciler/src/ReactFiberHooks.js | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 50546d4bb9e4f..2955a0e55e82a 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -618,7 +618,11 @@ function mountReducer( return [hook.memoizedState, dispatch]; } -function updateReducer(reducer: (S, A) => S): [S, Dispatch] { +function updateReducer( + reducer: (S, A) => S, + initialState: void | S, + initialAction: void | null | A, +): [S, Dispatch] { const hook = updateWorkInProgressHook(); const queue = hook.queue; invariant( @@ -773,8 +777,10 @@ function mountState( return [hook.memoizedState, dispatch]; } -function updateState(): [S, Dispatch>] { - return updateReducer(basicStateReducer); +function updateState( + initialState: (() => S) | S, +): [S, Dispatch>] { + return updateReducer(basicStateReducer, (initialState: any)); } function pushEffect(tag, create, destroy, deps) { @@ -1369,7 +1375,7 @@ if (__DEV__) { const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { - return updateReducer(reducer); + return updateReducer(reducer, initialState, initialAction); } finally { ReactCurrentDispatcher.current = prevDispatcher; } @@ -1385,7 +1391,7 @@ if (__DEV__) { const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { - return updateState(); + return updateState(initialState); } finally { ReactCurrentDispatcher.current = prevDispatcher; } @@ -1556,7 +1562,7 @@ if (__DEV__) { const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { - return updateReducer(reducer); + return updateReducer(reducer, initialState, initialAction); } finally { ReactCurrentDispatcher.current = prevDispatcher; } @@ -1574,7 +1580,7 @@ if (__DEV__) { const prevDispatcher = ReactCurrentDispatcher.current; ReactCurrentDispatcher.current = InvalidNestedHooksDispatcherOnUpdateInDEV; try { - return updateState(); + return updateState(initialState); } finally { ReactCurrentDispatcher.current = prevDispatcher; }