@samitouri / QOS-React-2 / commits / 70d4075832

Move Hook mismatch warning to first mismatch site (#14720)

* Move Hook mismatch warning to first mismatch site Allows us to localize the warning logic in one place. * Nit

Andrew Clark committed Jan 30, 2019 at 08:11 UTC 70d4075832cebc8bd3eb19f1089701096b483bd5
2 files changed +63 -107
packages/react-reconciler/src/ReactFiberHooks.js
+57 -105
@@ -98,10 +98,6 @@ type HookType =
98 | 'useImperativeHandle'
99 | 'useDebugValue';
100
101 -// the first instance of a hook mismatch in a component,
102 -// represented by a portion of its stacktrace
103 -let currentHookMismatchInDev = null;
104 -
101 let didWarnAboutMismatchedHooksForComponent;
102 if (__DEV__) {
103 didWarnAboutMismatchedHooksForComponent = new Set();
@@ -180,6 +176,56 @@ const RE_RENDER_LIMIT = 25;
176 // In DEV, this is the name of the currently executing primitive hook
177 let currentHookNameInDev: ?HookType = null;
178
179 +function warnOnHookMismatchInDev() {
180 + if (__DEV__) {
181 + const componentName = getComponentName(
182 + ((currentlyRenderingFiber: any): Fiber).type,
183 + );
184 + if (!didWarnAboutMismatchedHooksForComponent.has(componentName)) {
185 + didWarnAboutMismatchedHooksForComponent.add(componentName);
186 +
187 + const secondColumnStart = 22;
188 +
189 + let table = '';
190 + let prevHook: HookDev | null = (firstCurrentHook: any);
191 + let nextHook: HookDev | null = (firstWorkInProgressHook: any);
192 + let n = 1;
193 + while (prevHook !== null && nextHook !== null) {
194 + const oldHookName = prevHook._debugType;
195 + const newHookName = nextHook._debugType;
196 +
197 + let row = `${n}. ${oldHookName}`;
198 +
199 + // Extra space so second column lines up
200 + // lol @ IE not supporting String#repeat
201 + while (row.length < secondColumnStart) {
202 + row += ' ';
203 + }
204 +
205 + row += newHookName + '\n';
206 +
207 + table += row;
208 + prevHook = (prevHook.next: any);
209 + nextHook = (nextHook.next: any);
210 + n++;
211 + }
212 +
213 + warning(
214 + false,
215 + 'React has detected a change in the order of Hooks called by %s. ' +
216 + 'This will lead to bugs and errors if not fixed. ' +
217 + 'For more information, read the Rules of Hooks: https://fb.me/rules-of-hooks\n\n' +
218 + ' Previous render Next render\n' +
219 + ' -------------------------------\n' +
220 + '%s' +
221 + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n',
222 + componentName,
223 + table,
224 + );
225 + }
226 + }
227 +}
228 +
229 function throwInvalidHookError() {
230 invariant(
231 false,
@@ -229,90 +275,6 @@ function areHookInputsEqual(
275 return true;
276 }
277
232 -function flushHookMismatchWarnings() {
233 - // we'll show the diff of the low level hooks,
234 - // and a stack trace so the dev can locate where
235 - // the first mismatch is coming from
236 - if (__DEV__) {
237 - if (currentHookMismatchInDev !== null) {
238 - let componentName = getComponentName(
239 - ((currentlyRenderingFiber: any): Fiber).type,
240 - );
241 - if (!didWarnAboutMismatchedHooksForComponent.has(componentName)) {
242 - didWarnAboutMismatchedHooksForComponent.add(componentName);
243 - const hookStackDiff = [];
244 - let current = firstCurrentHook;
245 - const previousOrder = [];
246 - while (current !== null) {
247 - previousOrder.push(((current: any): HookDev)._debugType);
248 - current = current.next;
249 - }
250 - let workInProgress = firstWorkInProgressHook;
251 - const nextOrder = [];
252 - while (workInProgress !== null) {
253 - nextOrder.push(((workInProgress: any): HookDev)._debugType);
254 - workInProgress = workInProgress.next;
255 - }
256 - // some bookkeeping for formatting the output table
257 - const columnLength = Math.max.apply(
258 - null,
259 - previousOrder
260 - .map(hook => hook.length)
261 - .concat(' Previous render'.length),
262 - );
263 -
264 - const padEndSpaces = (string, length) => {
265 - if (string.length >= length) {
266 - return string;
267 - }
268 - return string + ' ' + new Array(length - string.length).join(' ');
269 - };
270 -
271 - let hookStackHeader =
272 - ((padEndSpaces(' Previous render', columnLength): any): string) +
273 - ' Next render\n';
274 - const hookStackWidth = hookStackHeader.length;
275 - hookStackHeader += ' ' + new Array(hookStackWidth - 2).join('-');
276 - const hookStackFooter = ' ' + new Array(hookStackWidth - 2).join('^');
277 -
278 - const hookStackLength = Math.max(
279 - previousOrder.length,
280 - nextOrder.length,
281 - );
282 - for (let i = 0; i < hookStackLength; i++) {
283 - hookStackDiff.push(
284 - ((padEndSpaces(i + 1 + '. ', 3): any): string) +
285 - ((padEndSpaces(previousOrder[i], columnLength): any): string) +
286 - ' ' +
287 - nextOrder[i],
288 - );
289 - if (previousOrder[i] !== nextOrder[i]) {
290 - break;
291 - }
292 - }
293 - warning(
294 - false,
295 - 'React has detected a change in the order of Hooks called by %s. ' +
296 - 'This will lead to bugs and errors if not fixed. ' +
297 - 'For more information, read the Rules of Hooks: https://fb.me/rules-of-hooks\n\n' +
298 - '%s\n' +
299 - '%s\n' +
300 - '%s\n' +
301 - 'The first Hook type mismatch occured at:\n' +
302 - '%s\n\n' +
303 - 'This error occurred in the following component:',
304 - componentName,
305 - hookStackHeader,
306 - hookStackDiff.join('\n'),
307 - hookStackFooter,
308 - currentHookMismatchInDev,
309 - );
310 - }
311 - currentHookMismatchInDev = null;
312 - }
313 - }
314 -}
315 -
278 export function renderWithHooks(
279 current: Fiber | null,
280 workInProgress: Fiber,
@@ -378,7 +340,6 @@ export function renderWithHooks(
340 }
341
342 if (__DEV__) {
381 - flushHookMismatchWarnings();
343 currentHookNameInDev = null;
344 }
345
@@ -437,10 +398,6 @@ export function bailoutHooks(
398 }
399
400 export function resetHooks(): void {
440 - if (__DEV__) {
441 - flushHookMismatchWarnings();
442 - }
443 -
401 // We can assume the previous dispatcher is always this one, since we set it
402 // at the beginning of the render phase and there's no re-entrancy.
403 ReactCurrentDispatcher.current = ContextOnlyDispatcher;
@@ -537,18 +494,6 @@ function updateWorkInProgressHook(): Hook {
494 next: null,
495 };
496
540 - if (__DEV__) {
541 - (newHook: any)._debugType = (currentHookNameInDev: any);
542 - if (currentHookMismatchInDev === null) {
543 - if (currentHookNameInDev !== ((currentHook: any): HookDev)._debugType) {
544 - currentHookMismatchInDev = new Error('tracer').stack
545 - .split('\n')
546 - .slice(4)
547 - .join('\n');
548 - }
549 - }
550 - }
551 -
497 if (workInProgressHook === null) {
498 // This is the first hook in the list.
499 workInProgressHook = firstWorkInProgressHook = newHook;
@@ -557,6 +502,13 @@ function updateWorkInProgressHook(): Hook {
502 workInProgressHook = workInProgressHook.next = newHook;
503 }
504 nextCurrentHook = currentHook.next;
505 +
506 + if (__DEV__) {
507 + (newHook: any)._debugType = (currentHookNameInDev: any);
508 + if (currentHookNameInDev !== ((currentHook: any): HookDev)._debugType) {
509 + warnOnHookMismatchInDev();
510 + }
511 + }
512 }
513 return workInProgressHook;
514 }
packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+6 -2
@@ -1159,7 +1159,7 @@ describe('ReactHooks', () => {
1159 });
1160
1161 it('warns on using differently ordered hooks on subsequent renders', () => {
1162 - const {useState, useReducer} = React;
1162 + const {useState, useReducer, useRef} = React;
1163 function useCustomHook() {
1164 return useState(0);
1165 }
@@ -1172,6 +1172,9 @@ describe('ReactHooks', () => {
1172 useReducer((s, a) => a, 0);
1173 useCustomHook(0);
1174 }
1175 + // This should not appear in the warning message because it occurs after
1176 + // the first mismatch
1177 + const ref = useRef(null);
1178 return null;
1179 /* eslint-enable no-unused-vars */
1180 }
@@ -1185,7 +1188,8 @@ describe('ReactHooks', () => {
1188 ' Previous render Next render\n' +
1189 ' -------------------------------\n' +
1190 '1. useReducer useState\n' +
1188 - ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^',
1191 + ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' +
1192 + ' in App (at **)',
1193 ]);
1194
1195 // further warnings for this component are silenced