Skip to content

Commit e1cd83e

Browse files
author
Sunil Pai
authored
Throw an error when using hooks inside useMemo/useState/useReducer, or .memo's comparator (#14608)
* hooks inside useMemo/.memo - failing tests * throw an error when using hooks inside useMemo * throw when using hooks inside .memo's compare fn * faster/better/stronger * same logic for useReducer, tests for the server, etc * Update ReactDOMServerIntegrationHooks-test.internal.js ack lint * nits * whitespace * whitespace * stray semi * Tweak comment * stray unmatched fiber reset * nit
1 parent be457ca commit e1cd83e

File tree

4 files changed

+148
-5
lines changed

4 files changed

+148
-5
lines changed

packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js

+45
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,51 @@ describe('ReactDOMServerHooks', () => {
419419
expect(domNode.textContent).toEqual('HELLO, WORLD.');
420420
},
421421
);
422+
423+
itThrowsWhenRendering(
424+
'a hook inside useMemo',
425+
async render => {
426+
function App() {
427+
useMemo(() => {
428+
useState();
429+
return 0;
430+
});
431+
return null;
432+
}
433+
return render(<App />);
434+
},
435+
'Hooks can only be called inside the body of a function component.',
436+
);
437+
438+
itThrowsWhenRendering(
439+
'a hook inside useReducer',
440+
async render => {
441+
function App() {
442+
const [value, dispatch] = useReducer((state, action) => {
443+
useRef(0);
444+
return state;
445+
}, 0);
446+
dispatch('foo');
447+
return value;
448+
}
449+
return render(<App />);
450+
},
451+
'Hooks can only be called inside the body of a function component.',
452+
);
453+
454+
itThrowsWhenRendering(
455+
'a hook inside useState',
456+
async render => {
457+
function App() {
458+
useState(() => {
459+
useRef(0);
460+
return 0;
461+
});
462+
}
463+
return render(<App />);
464+
},
465+
'Hooks can only be called inside the body of a function component.',
466+
);
422467
});
423468

424469
describe('useRef', () => {

packages/react-dom/src/server/ReactPartialRendererHooks.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ export function useReducer<S, A>(
234234
currentHookNameInDev = 'useReducer';
235235
}
236236
}
237-
currentlyRenderingComponent = resolveCurrentlyRenderingComponent();
237+
let component = (currentlyRenderingComponent = resolveCurrentlyRenderingComponent());
238238
workInProgressHook = createWorkInProgressHook();
239239
if (isReRender) {
240240
// This is a re-render. Apply the new render phase updates to the previous
@@ -253,7 +253,10 @@ export function useReducer<S, A>(
253253
// priority because it will always be the same as the current
254254
// render's.
255255
const action = update.action;
256+
// Temporarily clear to forbid calling Hooks.
257+
currentlyRenderingComponent = null;
256258
newState = reducer(newState, action);
259+
currentlyRenderingComponent = component;
257260
update = update.next;
258261
} while (update !== null);
259262

@@ -264,6 +267,7 @@ export function useReducer<S, A>(
264267
}
265268
return [workInProgressHook.memoizedState, dispatch];
266269
} else {
270+
currentlyRenderingComponent = null;
267271
if (reducer === basicStateReducer) {
268272
// Special case for `useState`.
269273
if (typeof initialState === 'function') {
@@ -272,6 +276,7 @@ export function useReducer<S, A>(
272276
} else if (initialAction !== undefined && initialAction !== null) {
273277
initialState = reducer(initialState, initialAction);
274278
}
279+
currentlyRenderingComponent = component;
275280
workInProgressHook.memoizedState = initialState;
276281
const queue: UpdateQueue<A> = (workInProgressHook.queue = {
277282
last: null,
@@ -287,7 +292,7 @@ export function useReducer<S, A>(
287292
}
288293

289294
function useMemo<T>(nextCreate: () => T, deps: Array<mixed> | void | null): T {
290-
currentlyRenderingComponent = resolveCurrentlyRenderingComponent();
295+
let component = (currentlyRenderingComponent = resolveCurrentlyRenderingComponent());
291296
workInProgressHook = createWorkInProgressHook();
292297

293298
const nextDeps = deps === undefined ? null : deps;
@@ -304,7 +309,10 @@ function useMemo<T>(nextCreate: () => T, deps: Array<mixed> | void | null): T {
304309
}
305310
}
306311

312+
// Temporarily clear to forbid calling Hooks.
313+
currentlyRenderingComponent = null;
307314
const nextValue = nextCreate();
315+
currentlyRenderingComponent = component;
308316
workInProgressHook.memoizedState = [nextValue, nextDeps];
309317
return nextValue;
310318
}

packages/react-reconciler/src/ReactFiberHooks.js

+14-3
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ export function useReducer<S, A>(
420420
currentHookNameInDev = 'useReducer';
421421
}
422422
}
423-
currentlyRenderingFiber = resolveCurrentlyRenderingFiber();
423+
let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber());
424424
workInProgressHook = createWorkInProgressHook();
425425
let queue: UpdateQueue<S, A> | null = (workInProgressHook.queue: any);
426426
if (queue !== null) {
@@ -441,7 +441,10 @@ export function useReducer<S, A>(
441441
// priority because it will always be the same as the current
442442
// render's.
443443
const action = update.action;
444+
// Temporarily clear to forbid calling Hooks in a reducer.
445+
currentlyRenderingFiber = null;
444446
newState = reducer(newState, action);
447+
currentlyRenderingFiber = fiber;
445448
update = update.next;
446449
} while (update !== null);
447450

@@ -510,7 +513,10 @@ export function useReducer<S, A>(
510513
newState = ((update.eagerState: any): S);
511514
} else {
512515
const action = update.action;
516+
// Temporarily clear to forbid calling Hooks in a reducer.
517+
currentlyRenderingFiber = null;
513518
newState = reducer(newState, action);
519+
currentlyRenderingFiber = fiber;
514520
}
515521
}
516522
prevUpdate = update;
@@ -539,7 +545,8 @@ export function useReducer<S, A>(
539545
const dispatch: Dispatch<A> = (queue.dispatch: any);
540546
return [workInProgressHook.memoizedState, dispatch];
541547
}
542-
548+
// Temporarily clear to forbid calling Hooks in a reducer.
549+
currentlyRenderingFiber = null;
543550
// There's no existing queue, so this is the initial render.
544551
if (reducer === basicStateReducer) {
545552
// Special case for `useState`.
@@ -549,6 +556,7 @@ export function useReducer<S, A>(
549556
} else if (initialAction !== undefined && initialAction !== null) {
550557
initialState = reducer(initialState, initialAction);
551558
}
559+
currentlyRenderingFiber = fiber;
552560
workInProgressHook.memoizedState = workInProgressHook.baseState = initialState;
553561
queue = workInProgressHook.queue = {
554562
last: null,
@@ -739,7 +747,7 @@ export function useMemo<T>(
739747
if (__DEV__) {
740748
currentHookNameInDev = 'useMemo';
741749
}
742-
currentlyRenderingFiber = resolveCurrentlyRenderingFiber();
750+
let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber());
743751
workInProgressHook = createWorkInProgressHook();
744752

745753
const nextDeps = deps === undefined ? null : deps;
@@ -755,7 +763,10 @@ export function useMemo<T>(
755763
}
756764
}
757765

766+
// Temporarily clear to forbid calling Hooks.
767+
currentlyRenderingFiber = null;
758768
const nextValue = nextCreate();
769+
currentlyRenderingFiber = fiber;
759770
workInProgressHook.memoizedState = [nextValue, nextDeps];
760771
return nextValue;
761772
}

packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js

+79
Original file line numberDiff line numberDiff line change
@@ -517,4 +517,83 @@ describe('ReactHooks', () => {
517517
const root = ReactTestRenderer.create(<App />);
518518
expect(root.toJSON()).toMatchSnapshot();
519519
});
520+
521+
it("throws when calling hooks inside .memo's compare function", () => {
522+
const {useState} = React;
523+
function App() {
524+
useState(0);
525+
return null;
526+
}
527+
const MemoApp = React.memo(App, () => {
528+
useState(0);
529+
return false;
530+
});
531+
532+
const root = ReactTestRenderer.create(<MemoApp />);
533+
// trying to render again should trigger comparison and throw
534+
expect(() => root.update(<MemoApp />)).toThrow(
535+
'Hooks can only be called inside the body of a function component',
536+
);
537+
// the next round, it does a fresh mount, so should render
538+
expect(() => root.update(<MemoApp />)).not.toThrow(
539+
'Hooks can only be called inside the body of a function component',
540+
);
541+
// and then again, fail
542+
expect(() => root.update(<MemoApp />)).toThrow(
543+
'Hooks can only be called inside the body of a function component',
544+
);
545+
});
546+
547+
it('throws when calling hooks inside useMemo', () => {
548+
const {useMemo, useState} = React;
549+
function App() {
550+
useMemo(() => {
551+
useState(0);
552+
return 1;
553+
});
554+
return null;
555+
}
556+
557+
function Simple() {
558+
const [value] = useState(123);
559+
return value;
560+
}
561+
let root = ReactTestRenderer.create(null);
562+
expect(() => root.update(<App />)).toThrow(
563+
'Hooks can only be called inside the body of a function component',
564+
);
565+
566+
// we want to assure that no hook machinery has broken
567+
// so we render a fresh component with a hook just to be sure
568+
root.update(<Simple />);
569+
expect(root.toJSON()).toEqual('123');
570+
});
571+
572+
it('throws when calling hooks inside useReducer', () => {
573+
const {useReducer, useRef} = React;
574+
function App() {
575+
const [value, dispatch] = useReducer((state, action) => {
576+
useRef(0);
577+
return state;
578+
}, 0);
579+
dispatch('foo');
580+
return value;
581+
}
582+
expect(() => ReactTestRenderer.create(<App />)).toThrow(
583+
'Hooks can only be called inside the body of a function component',
584+
);
585+
});
586+
587+
it("throws when calling hooks inside useState's initialize function", () => {
588+
const {useState, useRef} = React;
589+
function App() {
590+
useState(() => {
591+
useRef(0);
592+
return 0;
593+
});
594+
}
595+
expect(() => ReactTestRenderer.create(<App />)).toThrow(
596+
'Hooks can only be called inside the body of a function component',
597+
);
598+
});
520599
});

0 commit comments

Comments
 (0)