Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw an error when using hooks inside useMemo, or .memo's comparison function #14608

Merged
merged 16 commits into from Jan 18, 2019
Expand Up @@ -419,6 +419,51 @@ describe('ReactDOMServerHooks', () => {
expect(domNode.textContent).toEqual('HELLO, WORLD.');
},
);

itThrowsWhenRendering(
'a hook inside useMemo',
async render => {
function App() {
useMemo(() => {
useState();
return 0;
});
return null;
}
return render(<App />);
},
'Hooks can only be called inside the body of a function component.',
);

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(<App />);
},
'Hooks can only be called inside the body of a function component.',
);

itThrowsWhenRendering(
'a hook inside useState',
async render => {
function App() {
useState(() => {
useRef(0);
return 0;
});
}
return render(<App />);
},
'Hooks can only be called inside the body of a function component.',
);
});

describe('useRef', () => {
Expand Down
12 changes: 10 additions & 2 deletions packages/react-dom/src/server/ReactPartialRendererHooks.js
Expand Up @@ -234,7 +234,7 @@ export function useReducer<S, A>(
currentHookNameInDev = 'useReducer';
}
}
currentlyRenderingComponent = resolveCurrentlyRenderingComponent();
let component = (currentlyRenderingComponent = resolveCurrentlyRenderingComponent());
workInProgressHook = createWorkInProgressHook();
if (isReRender) {
// This is a re-render. Apply the new render phase updates to the previous
Expand All @@ -253,7 +253,10 @@ export function useReducer<S, A>(
// 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;
newState = reducer(newState, action);
currentlyRenderingComponent = component;
update = update.next;
threepointone marked this conversation as resolved.
Show resolved Hide resolved
} while (update !== null);

Expand All @@ -264,6 +267,7 @@ export function useReducer<S, A>(
}
return [workInProgressHook.memoizedState, dispatch];
} else {
currentlyRenderingComponent = null;
if (reducer === basicStateReducer) {
// Special case for `useState`.
if (typeof initialState === 'function') {
Expand All @@ -272,6 +276,7 @@ export function useReducer<S, A>(
} else if (initialAction !== undefined && initialAction !== null) {
initialState = reducer(initialState, initialAction);
}
currentlyRenderingComponent = component;
workInProgressHook.memoizedState = initialState;
const queue: UpdateQueue<A> = (workInProgressHook.queue = {
last: null,
Expand All @@ -287,7 +292,7 @@ export function useReducer<S, A>(
}

function useMemo<T>(nextCreate: () => T, deps: Array<mixed> | void | null): T {
currentlyRenderingComponent = resolveCurrentlyRenderingComponent();
let component = (currentlyRenderingComponent = resolveCurrentlyRenderingComponent());
workInProgressHook = createWorkInProgressHook();

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

// Temporarily clear to forbid calling Hooks.
currentlyRenderingComponent = null;
const nextValue = nextCreate();
currentlyRenderingComponent = component;
workInProgressHook.memoizedState = [nextValue, nextDeps];
return nextValue;
}
Expand Down
17 changes: 14 additions & 3 deletions packages/react-reconciler/src/ReactFiberHooks.js
Expand Up @@ -420,7 +420,7 @@ export function useReducer<S, A>(
currentHookNameInDev = 'useReducer';
}
}
currentlyRenderingFiber = resolveCurrentlyRenderingFiber();
let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber());
workInProgressHook = createWorkInProgressHook();
let queue: UpdateQueue<S, A> | null = (workInProgressHook.queue: any);
if (queue !== null) {
Expand All @@ -441,7 +441,10 @@ export function useReducer<S, A>(
// 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;
newState = reducer(newState, action);
currentlyRenderingFiber = fiber;
update = update.next;
} while (update !== null);

Expand Down Expand Up @@ -510,7 +513,10 @@ export function useReducer<S, A>(
newState = ((update.eagerState: any): S);
} else {
const action = update.action;
// Temporarily clear to forbid calling Hooks in a reducer.
currentlyRenderingFiber = null;
newState = reducer(newState, action);
currentlyRenderingFiber = fiber;
}
}
prevUpdate = update;
Expand Down Expand Up @@ -539,7 +545,8 @@ export function useReducer<S, A>(
const dispatch: Dispatch<A> = (queue.dispatch: any);
return [workInProgressHook.memoizedState, dispatch];
}

threepointone marked this conversation as resolved.
Show resolved Hide resolved
// Temporarily clear to forbid calling Hooks in a reducer.
currentlyRenderingFiber = null;
threepointone marked this conversation as resolved.
Show resolved Hide resolved
// There's no existing queue, so this is the initial render.
if (reducer === basicStateReducer) {
// Special case for `useState`.
Expand All @@ -549,6 +556,7 @@ export function useReducer<S, A>(
} else if (initialAction !== undefined && initialAction !== null) {
initialState = reducer(initialState, initialAction);
}
currentlyRenderingFiber = fiber;
workInProgressHook.memoizedState = workInProgressHook.baseState = initialState;
queue = workInProgressHook.queue = {
last: null,
Expand Down Expand Up @@ -739,7 +747,7 @@ export function useMemo<T>(
if (__DEV__) {
currentHookNameInDev = 'useMemo';
}
currentlyRenderingFiber = resolveCurrentlyRenderingFiber();
let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber());
workInProgressHook = createWorkInProgressHook();

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

// Temporarily clear to forbid calling Hooks.
currentlyRenderingFiber = null;
const nextValue = nextCreate();
currentlyRenderingFiber = fiber;
workInProgressHook.memoizedState = [nextValue, nextDeps];
return nextValue;
}
Expand Down
Expand Up @@ -517,4 +517,83 @@ describe('ReactHooks', () => {
const root = ReactTestRenderer.create(<App />);
expect(root.toJSON()).toMatchSnapshot();
});

it("throws when calling hooks inside .memo's compare function", () => {
const {useState} = React;
function App() {
useState(0);
return null;
}
const MemoApp = React.memo(App, () => {
useState(0);
return false;
});

const root = ReactTestRenderer.create(<MemoApp />);
// trying to render again should trigger comparison and throw
expect(() => root.update(<MemoApp />)).toThrow(
'Hooks can only be called inside the body of a function component',
);
// the next round, it does a fresh mount, so should render
expect(() => root.update(<MemoApp />)).not.toThrow(
'Hooks can only be called inside the body of a function component',
);
// and then again, fail
expect(() => root.update(<MemoApp />)).toThrow(
'Hooks can only be called inside the body of a function component',
);
});

it('throws 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(<App />)).toThrow(
'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(<Simple />);
expect(root.toJSON()).toEqual('123');
});

it('throws when calling hooks inside useReducer', () => {
const {useReducer, useRef} = React;
function App() {
const [value, dispatch] = useReducer((state, action) => {
useRef(0);
return state;
}, 0);
dispatch('foo');
return value;
}
expect(() => ReactTestRenderer.create(<App />)).toThrow(
'Hooks can only be called inside the body of a function component',
);
});

it("throws when calling hooks inside useState's initialize function", () => {
const {useState, useRef} = React;
function App() {
useState(() => {
useRef(0);
return 0;
});
}
expect(() => ReactTestRenderer.create(<App />)).toThrow(
'Hooks can only be called inside the body of a function component',
);
});
});