Skip to content

Commit

Permalink
Move Hook mismatch warning to first mismatch site
Browse files Browse the repository at this point in the history 
Allows us to localize the warning logic in one place.
  • Loading branch information
@acdlite
acdlite committed Jan 29, 2019
1 parent 9d483dc commit 4022317
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 108 deletions.
157 changes: 51 additions & 106 deletions packages/react-reconciler/src/ReactFiberHooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ type HookType =
| 'useImperativeHandle'
| 'useDebugValue';

// 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;
Expand Down Expand Up @@ -210,95 +207,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
// the first mismatch is coming from
if (__DEV__) {
if (currentHookMismatchInDev !== null) {
let componentName = getComponentName(
((currentlyRenderingFiber: any): Fiber).type,
);
if (!didWarnAboutMismatchedHooksForComponent.has(componentName)) {
didWarnAboutMismatchedHooksForComponent.add(componentName);
const hookStackDiff = [];
let current = firstCurrentHook;
const previousOrder = [];
while (current !== null) {
previousOrder.push(((current: any): HookDev)._debugType);
current = current.next;
}
let workInProgress = firstWorkInProgressHook;
const nextOrder = [];
while (workInProgress !== null) {
nextOrder.push(((workInProgress: any): HookDev)._debugType);
workInProgress = workInProgress.next;
}
// some bookkeeping for formatting the output table
const columnLength = Math.max.apply(
null,
previousOrder
.map(hook => hook.length)
.concat(' Previous render'.length),
);

let hookStackHeader =
((padEndSpaces(' Previous render', columnLength): any): string) +
' Next render\n';
const hookStackWidth = hookStackHeader.length;
hookStackHeader += ' ' + new Array(hookStackWidth - 2).join('-');
const hookStackFooter = ' ' + new Array(hookStackWidth - 2).join('^');

const hookStackLength = Math.max(
previousOrder.length,
nextOrder.length,
);
for (let i = 0; i < hookStackLength; i++) {
hookStackDiff.push(
((padEndSpaces(i + 1 + '. ', 3): any): string) +
((padEndSpaces(previousOrder[i], columnLength): any): string) +
' ' +
nextOrder[i],
);
if (previousOrder[i] !== nextOrder[i]) {
break;
}
}
warning(
false,
'React has detected a change in the order of Hooks called by %s. ' +
'This will lead to bugs and errors if not fixed. ' +
'For more information, read the Rules of Hooks: https://fb.me/rules-of-hooks\n\n' +
'%s\n' +
'%s\n' +
'%s\n' +
'The first Hook type mismatch occured at:\n' +
'%s\n\n' +
'This error occurred in the following component:',
componentName,
hookStackHeader,
hookStackDiff.join('\n'),
hookStackFooter,
currentHookMismatchInDev,
);
}
currentHookMismatchInDev = null;
}
}
}

export function renderWithHooks(
current: Fiber | null,
workInProgress: Fiber,
Expand Down Expand Up @@ -352,7 +260,6 @@ export function renderWithHooks(
getComponentName(Component),
);
}
flushHookMismatchWarnings();
}
} while (didScheduleRenderPhaseUpdate);

Expand Down Expand Up @@ -411,7 +318,6 @@ export function bailoutHooks(

export function resetHooks(): void {
if (__DEV__) {
flushHookMismatchWarnings();
exitDisallowedContextReadInDEV();
isInHookUserCodeInDev = false;
}
Expand Down Expand Up @@ -465,7 +371,7 @@ function createHook(): Hook {
}

function cloneHook(hook: Hook): Hook {
let nextHook: Hook = __DEV__
let newHook: Hook = __DEV__
? {
_debugType: ((currentHookNameInDev: any): HookType),
memoizedState: hook.memoizedState,
Expand All @@ -486,17 +392,7 @@ function cloneHook(hook: Hook): Hook {
next: null,
};

if (__DEV__) {
if (currentHookMismatchInDev === null) {
if (currentHookNameInDev !== ((hook: any): HookDev)._debugType) {
currentHookMismatchInDev = new Error('tracer').stack
.split('\n')
.slice(4)
.join('\n');
}
}
}
return nextHook;
return newHook;
}

function createWorkInProgressHook(): Hook {
Expand Down Expand Up @@ -541,6 +437,55 @@ function createWorkInProgressHook(): Hook {
currentHook = currentHook !== null ? currentHook.next : null;
}
}

if (__DEV__) {
if (
currentHook !== null &&
((currentHook: any): HookDev)._debugType !== currentHookNameInDev
) {
const componentName = getComponentName(
((currentlyRenderingFiber: any): Fiber).type,
);
if (!didWarnAboutMismatchedHooksForComponent.has(componentName)) {
didWarnAboutMismatchedHooksForComponent.add(componentName);

let table =
' Previous render Next render\n' +
' -------------------------------\n';

let prevHook: HookDev | null = (firstCurrentHook: any);
let nextHook: HookDev | null = (firstWorkInProgressHook: any);
let n = 1;
while (prevHook !== null && nextHook !== null) {
const oldHookName = prevHook._debugType;
const newHookName = nextHook._debugType;

let row = `${n}. ${oldHookName}`;
// TODO: Can we use String.repeat? Idk.
row += ' '.repeat(22 - row.length);
row += `${newHookName}\n`;

table += row;
prevHook = (prevHook.next: any);
nextHook = (nextHook.next: any);
n++;
}

table += ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^';

warning(
false,
'React has detected a change in the order of Hooks called by %s. ' +
'This will lead to bugs and errors if not fixed. ' +
'For more information, read the Rules of Hooks: https://fb.me/rules-of-hooks\n\n' +
'%s\n',
componentName,
table,
);
}
}
}

return workInProgressHook;
}

Expand Down
8 changes: 6 additions & 2 deletions packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,7 @@ describe('ReactHooks', () => {
});

it('warns on using differently ordered hooks on subsequent renders', () => {
const {useState, useReducer} = React;
const {useState, useReducer, useRef} = React;
function useCustomHook() {
return useState(0);
}
Expand All @@ -1161,6 +1161,9 @@ describe('ReactHooks', () => {
useReducer((s, a) => a, 0);
useCustomHook(0);
}
// This should not appear in the warning message because it occurs after
// the first mismatch
const ref = useRef(null);
return null;
/* eslint-enable no-unused-vars */
}
Expand All @@ -1174,7 +1177,8 @@ describe('ReactHooks', () => {
' Previous render Next render\n' +
' -------------------------------\n' +
'1. useReducer useState\n' +
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^',
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' +
' in App (at **)',
]);

// further warnings for this component are silenced
Expand Down

0 comments on commit 4022317

Please sign in to comment.