Skip to content

Commit

Permalink
Make all Hooks-in-Hooks warnings DEV-only
Browse files Browse the repository at this point in the history
  • Loading branch information
@gaearon
gaearon committed Jan 23, 2019
1 parent 63bf091 commit 3e4949d
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 96 deletions.
79 changes: 38 additions & 41 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,50 +420,47 @@ describe('ReactDOMServerHooks', () => {
},
);

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

const domNode = await render(<App />, 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(<App />, 1);
expect(domNode.textContent).toEqual('0');
});
});

describe('useRef', () => {
Expand Down
42 changes: 24 additions & 18 deletions packages/react-dom/src/server/ReactPartialRendererHooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ let renderPhaseUpdates: Map<UpdateQueue<any>, Update<any>> | null = null;
let numberOfReRenders: number = 0;
const RE_RENDER_LIMIT = 25;

let shouldWarnAboutReadingContextInDEV = false;
let isInHookUserCodeInDev = false;

// In DEV, this is the name of the currently executing primitive hook
let currentHookNameInDev: ?string;
Expand All @@ -59,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;
}

Expand Down Expand Up @@ -140,7 +148,7 @@ function createWorkInProgressHook(): Hook {
export function prepareToUseHooks(componentIdentity: Object): void {
currentlyRenderingComponent = componentIdentity;
if (__DEV__) {
shouldWarnAboutReadingContextInDEV = false;
isInHookUserCodeInDev = false;
}

// The following should have already been reset
Expand Down Expand Up @@ -179,7 +187,7 @@ export function finishHooks(
renderPhaseUpdates = null;
workInProgressHook = null;
if (__DEV__) {
shouldWarnAboutReadingContextInDEV = false;
isInHookUserCodeInDev = false;
}

// These were reset above
Expand All @@ -201,7 +209,7 @@ function readContext<T>(
validateContextBounds(context, threadID);
if (__DEV__) {
warning(
!shouldWarnAboutReadingContextInDEV,
!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 ' +
Expand Down Expand Up @@ -251,7 +259,7 @@ export function useReducer<S, A>(
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
Expand All @@ -270,16 +278,13 @@ 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;
if (__DEV__) {
shouldWarnAboutReadingContextInDEV = true;
isInHookUserCodeInDev = true;
}
newState = reducer(newState, action);
if (__DEV__) {
shouldWarnAboutReadingContextInDEV = false;
isInHookUserCodeInDev = false;
}
currentlyRenderingComponent = component;
update = update.next;
} while (update !== null);

Expand All @@ -290,7 +295,9 @@ export function useReducer<S, A>(
}
return [workInProgressHook.memoizedState, dispatch];
} else {
currentlyRenderingComponent = null;
if (__DEV__) {
isInHookUserCodeInDev = true;
}
if (reducer === basicStateReducer) {
// Special case for `useState`.
if (typeof initialState === 'function') {
Expand All @@ -299,7 +306,9 @@ export function useReducer<S, A>(
} else if (initialAction !== undefined && initialAction !== null) {
initialState = reducer(initialState, initialAction);
}
currentlyRenderingComponent = component;
if (__DEV__) {
isInHookUserCodeInDev = false;
}
workInProgressHook.memoizedState = initialState;
const queue: UpdateQueue<A> = (workInProgressHook.queue = {
last: null,
Expand All @@ -315,7 +324,7 @@ export function useReducer<S, A>(
}

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

const nextDeps = deps === undefined ? null : deps;
Expand All @@ -332,15 +341,12 @@ function useMemo<T>(nextCreate: () => T, deps: Array<mixed> | void | null): T {
}
}

// Temporarily clear to forbid calling Hooks.
currentlyRenderingComponent = null;
if (__DEV__) {
shouldWarnAboutReadingContextInDEV = true;
isInHookUserCodeInDev = true;
}
const nextValue = nextCreate();
currentlyRenderingComponent = component;
if (__DEV__) {
shouldWarnAboutReadingContextInDEV = false;
isInHookUserCodeInDev = false;
}
workInProgressHook.memoizedState = [nextValue, nextDeps];
return nextValue;
Expand Down
46 changes: 28 additions & 18 deletions packages/react-reconciler/src/ReactFiberHooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,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__) {
Expand Down Expand Up @@ -154,6 +155,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;
}

Expand Down Expand Up @@ -580,7 +592,7 @@ export function useReducer<S, A>(
currentHookNameInDev = 'useReducer';
}
}
let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber());
currentlyRenderingFiber = resolveCurrentlyRenderingFiber();
workInProgressHook = createWorkInProgressHook();
if (__DEV__) {
currentHookNameInDev = null;
Expand All @@ -604,15 +616,14 @@ 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;
if (__DEV__) {
stashContextDependenciesInDEV();
isInHookUserCodeInDev = true;
}
newState = reducer(newState, action);
currentlyRenderingFiber = fiber;
if (__DEV__) {
unstashContextDependenciesInDEV();
isInHookUserCodeInDev = false;
}
update = update.next;
} while (update !== null);
Expand Down Expand Up @@ -682,15 +693,14 @@ 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;
if (__DEV__) {
stashContextDependenciesInDEV();
isInHookUserCodeInDev = true;
}
newState = reducer(newState, action);
currentlyRenderingFiber = fiber;
if (__DEV__) {
unstashContextDependenciesInDEV();
isInHookUserCodeInDev = false;
}
}
}
Expand Down Expand Up @@ -720,10 +730,9 @@ export function useReducer<S, A>(
const dispatch: Dispatch<A> = (queue.dispatch: any);
return [workInProgressHook.memoizedState, dispatch];
}
// Temporarily clear to forbid calling Hooks in a reducer.
currentlyRenderingFiber = null;
if (__DEV__) {
stashContextDependenciesInDEV();
isInHookUserCodeInDev = true;
}
// There's no existing queue, so this is the initial render.
if (reducer === basicStateReducer) {
Expand All @@ -734,9 +743,9 @@ export function useReducer<S, A>(
} else if (initialAction !== undefined && initialAction !== null) {
initialState = reducer(initialState, initialAction);
}
currentlyRenderingFiber = fiber;
if (__DEV__) {
unstashContextDependenciesInDEV();
isInHookUserCodeInDev = false;
}
workInProgressHook.memoizedState = workInProgressHook.baseState = initialState;
queue = workInProgressHook.queue = {
Expand Down Expand Up @@ -960,7 +969,7 @@ export function useMemo<T>(
if (__DEV__) {
currentHookNameInDev = 'useMemo';
}
let fiber = (currentlyRenderingFiber = resolveCurrentlyRenderingFiber());
currentlyRenderingFiber = resolveCurrentlyRenderingFiber();
workInProgressHook = createWorkInProgressHook();

const nextDeps = deps === undefined ? null : deps;
Expand All @@ -979,15 +988,14 @@ export function useMemo<T>(
}
}

// Temporarily clear to forbid calling Hooks.
currentlyRenderingFiber = null;
if (__DEV__) {
stashContextDependenciesInDEV();
isInHookUserCodeInDev = true;
}
const nextValue = nextCreate();
currentlyRenderingFiber = fiber;
if (__DEV__) {
unstashContextDependenciesInDEV();
isInHookUserCodeInDev = false;
}
workInProgressHook.memoizedState = [nextValue, nextDeps];
if (__DEV__) {
Expand Down Expand Up @@ -1086,16 +1094,14 @@ function dispatchAction<S, A>(
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;
if (__DEV__) {
stashContextDependenciesInDEV();
isInHookUserCodeInDev = true;
}
const eagerState = eagerReducer(currentState, action);
currentlyRenderingFiber = maybeFiber;
if (__DEV__) {
unstashContextDependenciesInDEV();
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
Expand All @@ -1112,6 +1118,10 @@ function dispatchAction<S, A>(
}
} catch (error) {
// Suppress the error. It will throw again in the render phase.
} finally {
if (__DEV__) {
isInHookUserCodeInDev = false;
}
}
}
}
Expand Down
Loading

0 comments on commit 3e4949d

Please sign in to comment.