From 8bcc88f2e7a7dc3e950bfcca1d3c6bbd251c4b20 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 24 Jan 2019 19:31:20 +0000 Subject: [PATCH] Make all readContext() and Hook-in-a-Hook checks DEV-only (#14677) * Make readContext() in Hooks DEV-only warning * Warn about readContext() during class render-phase setState() * Warn on readContext() in SSR inside useMemo and useReducer * Make all Hooks-in-Hooks warnings DEV-only * Rename stashContextDependencies * Clean up warning state on errors --- ...DOMServerIntegrationHooks-test.internal.js | 111 +++++++----- .../src/server/ReactPartialRendererHooks.js | 55 ++++-- .../react-reconciler/src/ReactFiberHooks.js | 98 ++++++---- .../src/ReactFiberNewContext.js | 44 ++--- .../react-reconciler/src/ReactUpdateQueue.js | 15 +- .../src/__tests__/ReactHooks-test.internal.js | 169 +++++++++++++++--- .../ReactNewContext-test.internal.js | 25 +++ 7 files changed, 388 insertions(+), 129 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js index d69256c15ad5f..d3a7dae6122ce 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js @@ -419,50 +419,47 @@ describe('ReactDOMServerHooks', () => { }, ); - itThrowsWhenRendering( - 'a hook inside useMemo', - async render => { - function App() { - useMemo(() => { - useState(); - return 0; - }); - return null; - } - return render(); - }, - 'Hooks can only be called inside the body of a function component.', - ); + itRenders('with a warning for useState inside useMemo', async render => { + function App() { + useMemo(() => { + useState(); + return 0; + }); + return 'hi'; + } - itThrowsWhenRendering( - 'a hook inside useReducer', - async render => { - function App() { - const [value, dispatch] = useReducer((state, action) => { - useRef(0); - return state; - }, 0); - dispatch('foo'); - return value; - } - return render(); - }, - 'Hooks can only be called inside the body of a function component.', - ); + const domNode = await render(, 1); + expect(domNode.textContent).toEqual('hi'); + }); - itThrowsWhenRendering( - 'a hook inside useState', - async render => { - function App() { - useState(() => { - useRef(0); - return 0; - }); + 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(); } - return render(); - }, - 'Hooks can only be called inside the body of a function component.', - ); + return value; + } + + const domNode = await render(, 1); + expect(domNode.textContent).toEqual('1'); + }); + + itRenders('with a warning for useRef inside useState', async render => { + function App() { + const [value] = useState(() => { + useRef(0); + return 0; + }); + return value; + } + + const domNode = await render(, 1); + expect(domNode.textContent).toEqual('0'); + }); }); describe('useRef', () => { @@ -716,4 +713,36 @@ describe('ReactDOMServerHooks', () => { expect(domNode.textContent).toEqual('undefined'); }); }); + + describe('readContext', () => { + function readContext(Context, observedBits) { + const dispatcher = + React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED + .ReactCurrentDispatcher.current; + return dispatcher.readContext(Context, observedBits); + } + + itRenders('with a warning inside useMemo and useReducer', async render => { + const Context = React.createContext(42); + + function ReadInMemo(props) { + let count = React.useMemo(() => readContext(Context), []); + return ; + } + + function ReadInReducer(props) { + let [count, dispatch] = React.useReducer(() => readContext(Context)); + if (count !== 42) { + dispatch(); + } + return ; + } + + const domNode1 = await render(, 1); + expect(domNode1.textContent).toEqual('42'); + + const domNode2 = await render(, 1); + expect(domNode2.textContent).toEqual('42'); + }); + }); }); diff --git a/packages/react-dom/src/server/ReactPartialRendererHooks.js b/packages/react-dom/src/server/ReactPartialRendererHooks.js index d8496b8a26294..1689d830c184a 100644 --- a/packages/react-dom/src/server/ReactPartialRendererHooks.js +++ b/packages/react-dom/src/server/ReactPartialRendererHooks.js @@ -49,6 +49,8 @@ let renderPhaseUpdates: Map, Update> | null = null; let numberOfReRenders: number = 0; const RE_RENDER_LIMIT = 25; +let isInHookUserCodeInDev = false; + // In DEV, this is the name of the currently executing primitive hook let currentHookNameInDev: ?string; @@ -57,6 +59,14 @@ function resolveCurrentlyRenderingComponent(): Object { currentlyRenderingComponent !== null, 'Hooks can only be called inside the body of a function component.', ); + if (__DEV__) { + 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 currentlyRenderingComponent; } @@ -137,6 +147,9 @@ function createWorkInProgressHook(): Hook { export function prepareToUseHooks(componentIdentity: Object): void { currentlyRenderingComponent = componentIdentity; + if (__DEV__) { + isInHookUserCodeInDev = false; + } // The following should have already been reset // didScheduleRenderPhaseUpdate = false; @@ -173,6 +186,9 @@ export function finishHooks( numberOfReRenders = 0; renderPhaseUpdates = null; workInProgressHook = null; + if (__DEV__) { + isInHookUserCodeInDev = false; + } // These were reset above // currentlyRenderingComponent = null; @@ -191,6 +207,15 @@ function readContext( ): T { let threadID = currentThreadID; validateContextBounds(context, threadID); + if (__DEV__) { + warning( + !isInHookUserCodeInDev, + '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 context[threadID]; } @@ -234,7 +259,7 @@ export function useReducer( currentHookNameInDev = 'useReducer'; } } - let component = (currentlyRenderingComponent = resolveCurrentlyRenderingComponent()); + currentlyRenderingComponent = resolveCurrentlyRenderingComponent(); workInProgressHook = createWorkInProgressHook(); if (isReRender) { // This is a re-render. Apply the new render phase updates to the previous @@ -253,10 +278,13 @@ export function useReducer( // priority because it will always be the same as the current // render's. const action = update.action; - // Temporarily clear to forbid calling Hooks. - currentlyRenderingComponent = null; + if (__DEV__) { + isInHookUserCodeInDev = true; + } newState = reducer(newState, action); - currentlyRenderingComponent = component; + if (__DEV__) { + isInHookUserCodeInDev = false; + } update = update.next; } while (update !== null); @@ -267,7 +295,9 @@ export function useReducer( } return [workInProgressHook.memoizedState, dispatch]; } else { - currentlyRenderingComponent = null; + if (__DEV__) { + isInHookUserCodeInDev = true; + } if (reducer === basicStateReducer) { // Special case for `useState`. if (typeof initialState === 'function') { @@ -276,7 +306,9 @@ export function useReducer( } else if (initialAction !== undefined && initialAction !== null) { initialState = reducer(initialState, initialAction); } - currentlyRenderingComponent = component; + if (__DEV__) { + isInHookUserCodeInDev = false; + } workInProgressHook.memoizedState = initialState; const queue: UpdateQueue = (workInProgressHook.queue = { last: null, @@ -292,7 +324,7 @@ export function useReducer( } function useMemo(nextCreate: () => T, deps: Array | void | null): T { - let component = (currentlyRenderingComponent = resolveCurrentlyRenderingComponent()); + currentlyRenderingComponent = resolveCurrentlyRenderingComponent(); workInProgressHook = createWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; @@ -309,10 +341,13 @@ function useMemo(nextCreate: () => T, deps: Array | void | null): T { } } - // Temporarily clear to forbid calling Hooks. - currentlyRenderingComponent = null; + if (__DEV__) { + isInHookUserCodeInDev = true; + } const nextValue = nextCreate(); - currentlyRenderingComponent = component; + if (__DEV__) { + isInHookUserCodeInDev = false; + } workInProgressHook.memoizedState = [nextValue, nextDeps]; return nextValue; } diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 00cd8aa94039a..7aaa45926346c 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -15,8 +15,8 @@ import type {HookEffectTag} from './ReactHookEffectTags'; import {NoWork} from './ReactFiberExpirationTime'; import { readContext, - stashContextDependencies, - unstashContextDependencies, + enterDisallowedContextReadInDEV, + exitDisallowedContextReadInDEV, } from './ReactFiberNewContext'; import { Update as UpdateEffect, @@ -72,6 +72,7 @@ type HookType = // the first instance of a hook mismatch in a component, // represented by a portion of it's stacktrace let currentHookMismatchInDev = null; +let isInHookUserCodeInDev = false; let didWarnAboutMismatchedHooksForComponent; if (__DEV__) { @@ -153,6 +154,17 @@ function resolveCurrentlyRenderingFiber(): Fiber { 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; } @@ -299,6 +311,11 @@ 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; @@ -395,6 +412,8 @@ export function bailoutHooks( export function resetHooks(): void { if (__DEV__) { flushHookMismatchWarnings(); + exitDisallowedContextReadInDEV(); + isInHookUserCodeInDev = false; } // This is used to reset the state of this module when a component throws. @@ -573,7 +592,7 @@ export function useReducer( currentHookNameInDev = 'useReducer'; } } - let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber()); + currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); workInProgressHook = createWorkInProgressHook(); if (__DEV__) { currentHookNameInDev = null; @@ -597,12 +616,15 @@ export function useReducer( // priority because it will always be the same as the current // render's. const action = update.action; - // Temporarily clear to forbid calling Hooks in a reducer. - currentlyRenderingFiber = null; - stashContextDependencies(); + if (__DEV__) { + enterDisallowedContextReadInDEV(); + isInHookUserCodeInDev = true; + } newState = reducer(newState, action); - currentlyRenderingFiber = fiber; - unstashContextDependencies(); + if (__DEV__) { + exitDisallowedContextReadInDEV(); + isInHookUserCodeInDev = false; + } update = update.next; } while (update !== null); @@ -671,12 +693,15 @@ export function useReducer( newState = ((update.eagerState: any): S); } else { const action = update.action; - // Temporarily clear to forbid calling Hooks in a reducer. - currentlyRenderingFiber = null; - stashContextDependencies(); + if (__DEV__) { + enterDisallowedContextReadInDEV(); + isInHookUserCodeInDev = true; + } newState = reducer(newState, action); - currentlyRenderingFiber = fiber; - unstashContextDependencies(); + if (__DEV__) { + exitDisallowedContextReadInDEV(); + isInHookUserCodeInDev = false; + } } } prevUpdate = update; @@ -705,9 +730,10 @@ export function useReducer( const dispatch: Dispatch = (queue.dispatch: any); return [workInProgressHook.memoizedState, dispatch]; } - // Temporarily clear to forbid calling Hooks in a reducer. - currentlyRenderingFiber = null; - stashContextDependencies(); + if (__DEV__) { + enterDisallowedContextReadInDEV(); + isInHookUserCodeInDev = true; + } // There's no existing queue, so this is the initial render. if (reducer === basicStateReducer) { // Special case for `useState`. @@ -717,8 +743,10 @@ export function useReducer( } else if (initialAction !== undefined && initialAction !== null) { initialState = reducer(initialState, initialAction); } - currentlyRenderingFiber = fiber; - unstashContextDependencies(); + if (__DEV__) { + exitDisallowedContextReadInDEV(); + isInHookUserCodeInDev = false; + } workInProgressHook.memoizedState = workInProgressHook.baseState = initialState; queue = workInProgressHook.queue = { last: null, @@ -941,7 +969,7 @@ export function useMemo( if (__DEV__) { currentHookNameInDev = 'useMemo'; } - let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber()); + currentlyRenderingFiber = resolveCurrentlyRenderingFiber(); workInProgressHook = createWorkInProgressHook(); const nextDeps = deps === undefined ? null : deps; @@ -960,12 +988,15 @@ export function useMemo( } } - // Temporarily clear to forbid calling Hooks. - currentlyRenderingFiber = null; - stashContextDependencies(); + if (__DEV__) { + enterDisallowedContextReadInDEV(); + isInHookUserCodeInDev = true; + } const nextValue = nextCreate(); - currentlyRenderingFiber = fiber; - unstashContextDependencies(); + if (__DEV__) { + exitDisallowedContextReadInDEV(); + isInHookUserCodeInDev = false; + } workInProgressHook.memoizedState = [nextValue, nextDeps]; if (__DEV__) { currentHookNameInDev = null; @@ -1063,13 +1094,15 @@ function dispatchAction( if (eagerReducer !== null) { try { const currentState: S = (queue.eagerState: any); - // Temporarily clear to forbid calling Hooks in a reducer. - let maybeFiber = currentlyRenderingFiber; // Note: likely null now unlike `fiber` - currentlyRenderingFiber = null; - stashContextDependencies(); + if (__DEV__) { + enterDisallowedContextReadInDEV(); + isInHookUserCodeInDev = true; + } const eagerState = eagerReducer(currentState, action); - currentlyRenderingFiber = maybeFiber; - unstashContextDependencies(); + 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 @@ -1085,6 +1118,11 @@ function dispatchAction( } } catch (error) { // Suppress the error. It will throw again in the render phase. + } finally { + if (__DEV__) { + exitDisallowedContextReadInDEV(); + isInHookUserCodeInDev = false; + } } } } diff --git a/packages/react-reconciler/src/ReactFiberNewContext.js b/packages/react-reconciler/src/ReactFiberNewContext.js index ac09ea2703f33..26530f0362ebd 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.js @@ -52,10 +52,7 @@ let currentlyRenderingFiber: Fiber | null = null; let lastContextDependency: ContextDependency | null = null; let lastContextWithAllBitsObserved: ReactContext | null = null; -// We stash the variables above before entering user code in Hooks. -let stashedCurrentlyRenderingFiber: Fiber | null = null; -let stashedLastContextDependency: ContextDependency | null = null; -let stashedLastContextWithAllBitsObserved: ReactContext | null = null; +let isDisallowedContextReadInDEV: boolean = false; export function resetContextDependences(): void { // This is called right before React yields execution, to ensure `readContext` @@ -63,26 +60,21 @@ export function resetContextDependences(): void { currentlyRenderingFiber = null; lastContextDependency = null; lastContextWithAllBitsObserved = null; - - stashedCurrentlyRenderingFiber = null; - stashedLastContextDependency = null; - stashedLastContextWithAllBitsObserved = null; + if (__DEV__) { + isDisallowedContextReadInDEV = false; + } } -export function stashContextDependencies(): void { - stashedCurrentlyRenderingFiber = currentlyRenderingFiber; - stashedLastContextDependency = lastContextDependency; - stashedLastContextWithAllBitsObserved = lastContextWithAllBitsObserved; - - currentlyRenderingFiber = null; - lastContextDependency = null; - lastContextWithAllBitsObserved = null; +export function enterDisallowedContextReadInDEV(): void { + if (__DEV__) { + isDisallowedContextReadInDEV = true; + } } -export function unstashContextDependencies(): void { - currentlyRenderingFiber = stashedCurrentlyRenderingFiber; - lastContextDependency = stashedLastContextDependency; - lastContextWithAllBitsObserved = stashedLastContextWithAllBitsObserved; +export function exitDisallowedContextReadInDEV(): void { + if (__DEV__) { + isDisallowedContextReadInDEV = false; + } } export function pushProvider(providerFiber: Fiber, nextValue: T): void { @@ -304,6 +296,18 @@ export function readContext( context: ReactContext, observedBits: void | number | boolean, ): T { + if (__DEV__) { + // This warning would fire if you read context inside a Hook like useMemo. + // Unlike the class check below, it's not enforced in production for perf. + warning( + !isDisallowedContextReadInDEV, + '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().', + ); + } + if (lastContextWithAllBitsObserved === context) { // Nothing to do. We already observe everything in this context. } else if (observedBits === false || observedBits === 0) { diff --git a/packages/react-reconciler/src/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index aafc453426089..64fc6a8a5840c 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.js @@ -88,6 +88,10 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; import {NoWork} from './ReactFiberExpirationTime'; +import { + enterDisallowedContextReadInDEV, + exitDisallowedContextReadInDEV, +} from './ReactFiberNewContext'; import {Callback, ShouldCapture, DidCapture} from 'shared/ReactSideEffectTags'; import {ClassComponent} from 'shared/ReactWorkTags'; @@ -348,6 +352,7 @@ function getStateFromUpdate( if (typeof payload === 'function') { // Updater function if (__DEV__) { + enterDisallowedContextReadInDEV(); if ( debugRenderPhaseSideEffects || (debugRenderPhaseSideEffectsForStrictMode && @@ -356,7 +361,11 @@ function getStateFromUpdate( payload.call(instance, prevState, nextProps); } } - return payload.call(instance, prevState, nextProps); + const nextState = payload.call(instance, prevState, nextProps); + if (__DEV__) { + exitDisallowedContextReadInDEV(); + } + return nextState; } // State object return payload; @@ -372,6 +381,7 @@ function getStateFromUpdate( if (typeof payload === 'function') { // Updater function if (__DEV__) { + enterDisallowedContextReadInDEV(); if ( debugRenderPhaseSideEffects || (debugRenderPhaseSideEffectsForStrictMode && @@ -381,6 +391,9 @@ function getStateFromUpdate( } } partialState = payload.call(instance, prevState, nextProps); + if (__DEV__) { + exitDisallowedContextReadInDEV(); + } } else { // Partial state object partialState = payload; diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 76783087a45a4..862d8fbe2bb2d 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -646,32 +646,20 @@ describe('ReactHooks', () => { ); }); - it('throws when calling hooks inside useMemo', () => { + it('warns when calling hooks inside useMemo', () => { const {useMemo, useState} = React; function App() { useMemo(() => { useState(0); - return 1; }); return null; } - - function Simple() { - const [value] = useState(123); - return value; - } - let root = ReactTestRenderer.create(null); - expect(() => root.update()).toThrow( + expect(() => ReactTestRenderer.create()).toWarnDev( 'Hooks can only be called inside the body of a function component', ); - - // we want to assure that no hook machinery has broken - // so we render a fresh component with a hook just to be sure - root.update(); - expect(root.toJSON()).toEqual('123'); }); - it('throws when reading context inside useMemo', () => { + it('warns when reading context inside useMemo', () => { const {useMemo, createContext} = React; const ReactCurrentDispatcher = React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED @@ -684,12 +672,12 @@ describe('ReactHooks', () => { }, []); } - expect(() => ReactTestRenderer.create()).toThrow( + expect(() => ReactTestRenderer.create()).toWarnDev( 'Context can only be read while React is rendering', ); }); - it('throws when reading context inside useMemo after reading outside it', () => { + it('warns when reading context inside useMemo after reading outside it', () => { const {useMemo, createContext} = React; const ReactCurrentDispatcher = React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED @@ -706,13 +694,14 @@ describe('ReactHooks', () => { }, []); } - expect(() => ReactTestRenderer.create()).toThrow( + expect(() => ReactTestRenderer.create()).toWarnDev( 'Context can only be read while React is rendering', ); expect(firstRead).toBe('light'); expect(secondRead).toBe('light'); }); + // Throws because there's no runtime cost for being strict here. it('throws when reading context inside useEffect', () => { const {useEffect, createContext} = React; const ReactCurrentDispatcher = @@ -734,6 +723,7 @@ describe('ReactHooks', () => { ); }); + // Throws because there's no runtime cost for being strict here. it('throws when reading context inside useLayoutEffect', () => { const {useLayoutEffect, createContext} = React; const ReactCurrentDispatcher = @@ -754,7 +744,7 @@ describe('ReactHooks', () => { ); }); - it('throws when reading context inside useReducer', () => { + it('warns when reading context inside useReducer', () => { const {useReducer, createContext} = React; const ReactCurrentDispatcher = React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED @@ -772,13 +762,13 @@ describe('ReactHooks', () => { return null; } - expect(() => ReactTestRenderer.create()).toThrow( + expect(() => ReactTestRenderer.create()).toWarnDev( 'Context can only be read while React is rendering', ); }); // Edge case. - it('throws when reading context inside eager useReducer', () => { + it('warns when reading context inside eager useReducer', () => { const {useState, createContext} = React; const ThemeContext = createContext('light'); @@ -809,20 +799,22 @@ describe('ReactHooks', () => { , ), - ).toThrow('Context can only be read while React is rendering'); + ).toWarnDev('Context can only be read while React is rendering'); }); - it('throws when calling hooks inside useReducer', () => { + it('warns when calling hooks inside useReducer', () => { const {useReducer, useRef} = React; function App() { const [value, dispatch] = useReducer((state, action) => { useRef(0); - return state; + return state + 1; }, 0); - dispatch('foo'); + if (value === 0) { + dispatch('foo'); + } return value; } - expect(() => ReactTestRenderer.create()).toThrow( + expect(() => ReactTestRenderer.create()).toWarnDev( 'Hooks can only be called inside the body of a function component', ); }); @@ -834,12 +826,107 @@ describe('ReactHooks', () => { useRef(0); return 0; }); + return null; } - expect(() => ReactTestRenderer.create()).toThrow( + expect(() => ReactTestRenderer.create()).toWarnDev( 'Hooks can only be called inside the body of a function component', ); }); + it('resets warning internal state when interrupted by an error', () => { + const ReactCurrentDispatcher = + React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED + .ReactCurrentDispatcher; + + const ThemeContext = React.createContext('light'); + function App() { + React.useMemo(() => { + // Trigger warnings + ReactCurrentDispatcher.current.readContext(ThemeContext); + React.useRef(); + // Interrupt exit from a Hook + throw new Error('No.'); + }, []); + } + + class Boundary extends React.Component { + state = {}; + static getDerivedStateFromError(error) { + return {err: true}; + } + render() { + if (this.state.err) { + return 'Oops'; + } + return this.props.children; + } + } + + expect(() => { + ReactTestRenderer.create( + + + , + ); + }).toWarnDev([ + // We see it twice due to replay + 'Context can only be read while React is rendering', + 'Hooks can only be called inside the body of a function component', + 'Context can only be read while React is rendering', + 'Hooks can only be called inside the body of a function component', + ]); + + function Valid() { + React.useState(); + React.useMemo(() => {}); + React.useReducer(() => {}); + React.useEffect(() => {}); + React.useLayoutEffect(() => {}); + React.useCallback(() => {}); + React.useRef(); + React.useImperativeHandle(() => {}, () => {}); + if (__DEV__) { + React.useDebugValue(); + } + return null; + } + // Verify it doesn't think we're still inside a Hook. + // Should have no warnings. + ReactTestRenderer.create(); + + // Verify warnings don't get permanently disabled. + expect(() => { + ReactTestRenderer.create( + + + , + ); + }).toWarnDev([ + // We see it twice due to replay + 'Context can only be read while React is rendering', + 'Hooks can only be called inside the body of a function component', + 'Context can only be read while React is rendering', + 'Hooks can only be called inside the body of a function component', + ]); + }); + + it('warns when reading context inside useMemo', () => { + const {useMemo, createContext} = React; + const ReactCurrentDispatcher = + React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED + .ReactCurrentDispatcher; + + const ThemeContext = createContext('light'); + function App() { + return useMemo(() => { + return ReactCurrentDispatcher.current.readContext(ThemeContext); + }, []); + } + + expect(() => ReactTestRenderer.create()).toWarnDev( + 'Context can only be read while React is rendering', + ); + }); it('double-invokes components with Hooks in Strict Mode', () => { ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = true; @@ -1125,4 +1212,32 @@ describe('ReactHooks', () => { ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^', ]); }); + + // Regression test for #14674 + it('does not swallow original error when updating another component in render phase', () => { + let {useState} = React; + + let _setState; + function A() { + const [, setState] = useState(0); + _setState = setState; + return null; + } + + function B() { + _setState(() => { + throw new Error('Hello'); + }); + return null; + } + + expect(() => + ReactTestRenderer.create( + + + + , + ), + ).toThrow('Hello'); + }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js index 05c59d5f339b6..9a4408598f020 100644 --- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js @@ -1346,6 +1346,31 @@ describe('ReactNewContext', () => { expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']); expect(ReactNoop.getChildren()).toEqual([span('goodbye')]); }); + + it('warns when reading context inside render phase class setState updater', () => { + const ThemeContext = React.createContext('light'); + + class Cls extends React.Component { + state = {}; + render() { + this.setState(() => { + readContext(ThemeContext); + }); + return null; + } + } + + ReactNoop.render(); + expect(ReactNoop.flush).toWarnDev( + [ + 'Context can only be read while React is rendering', + 'Cannot update during an existing state transition', + ], + { + withoutStack: 1, + }, + ); + }); }); describe('useContext', () => {