From d0710d0d83c57404b52c32057de7df0b25b9dfbf Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 5 Apr 2020 22:10:40 -0700 Subject: [PATCH 1/9] Implement component stack extraction hack --- packages/react/src/ReactDebugCurrentFrame.js | 25 +-- packages/react/src/ReactElementValidator.js | 23 ++- .../react/src/jsx/ReactJSXElementValidator.js | 31 +++- packages/shared/ConsolePatchingDev.js | 43 +++-- packages/shared/ReactComponentStackFrame.js | 161 +++++++++++++++--- packages/shared/ReactFeatureFlags.js | 2 +- 6 files changed, 215 insertions(+), 70 deletions(-) diff --git a/packages/react/src/ReactDebugCurrentFrame.js b/packages/react/src/ReactDebugCurrentFrame.js index cdb2eb9fc2e7..ddaeed70f99b 100644 --- a/packages/react/src/ReactDebugCurrentFrame.js +++ b/packages/react/src/ReactDebugCurrentFrame.js @@ -7,26 +7,20 @@ * @flow */ -import type {ReactElement} from 'shared/ReactElementType'; - -import {describeUnknownElementTypeFrameInDEV} from 'shared/ReactComponentStackFrame'; - const ReactDebugCurrentFrame = {}; -let currentlyValidatingElement = (null: null | ReactElement); +let currentExtraStackFrame = (null: null | string); -export function setCurrentlyValidatingElement(element: null | ReactElement) { +export function setExtraStackFrame(stack: null | string) { if (__DEV__) { - currentlyValidatingElement = element; + currentExtraStackFrame = stack; } } if (__DEV__) { - ReactDebugCurrentFrame.setCurrentlyValidatingElement = function( - element: null | ReactElement, - ) { + ReactDebugCurrentFrame.setExtraStackFrame = function(stack: null | string) { if (__DEV__) { - currentlyValidatingElement = element; + currentExtraStackFrame = stack; } }; // Stack implementation injected by the current renderer. @@ -36,13 +30,8 @@ if (__DEV__) { let stack = ''; // Add an extra top frame while an element is being validated - if (currentlyValidatingElement) { - const owner = currentlyValidatingElement._owner; - stack += describeUnknownElementTypeFrameInDEV( - currentlyValidatingElement.type, - currentlyValidatingElement._source, - owner ? owner.type : null, - ); + if (currentExtraStackFrame) { + stack += currentExtraStackFrame; } // Delegate to the injected renderer-specific implementation diff --git a/packages/react/src/ReactElementValidator.js b/packages/react/src/ReactElementValidator.js index c81ace3c6b2f..43c514649289 100644 --- a/packages/react/src/ReactElementValidator.js +++ b/packages/react/src/ReactElementValidator.js @@ -31,7 +31,24 @@ import { cloneElement, jsxDEV, } from './ReactElement'; -import {setCurrentlyValidatingElement} from './ReactDebugCurrentFrame'; +import {setExtraStackFrame} from './ReactDebugCurrentFrame'; +import {describeUnknownElementTypeFrameInDEV} from 'shared/ReactComponentStackFrame'; + +function setCurrentlyValidatingElement(element) { + if (__DEV__) { + if (element) { + const owner = element._owner; + const stack = describeUnknownElementTypeFrameInDEV( + element.type, + element._source, + owner ? owner.type : null, + ); + setExtraStackFrame(stack); + } else { + setExtraStackFrame(null); + } + } +} let propTypesMisspellWarningShown; @@ -127,16 +144,16 @@ function validateExplicitKey(element, parentType) { )}.`; } - setCurrentlyValidatingElement(element); if (__DEV__) { + setCurrentlyValidatingElement(element); console.error( 'Each child in a list should have a unique "key" prop.' + '%s%s See https://fb.me/react-warning-keys for more information.', currentComponentErrorInfo, childOwner, ); + setCurrentlyValidatingElement(null); } - setCurrentlyValidatingElement(null); } /** diff --git a/packages/react/src/jsx/ReactJSXElementValidator.js b/packages/react/src/jsx/ReactJSXElementValidator.js index a96225a61d87..004c3354ed10 100644 --- a/packages/react/src/jsx/ReactJSXElementValidator.js +++ b/packages/react/src/jsx/ReactJSXElementValidator.js @@ -24,11 +24,30 @@ import { import {warnAboutSpreadingKeyToJSX} from 'shared/ReactFeatureFlags'; import {jsxDEV} from './ReactJSXElement'; + +import {describeUnknownElementTypeFrameInDEV} from 'shared/ReactComponentStackFrame'; + import ReactSharedInternals from 'shared/ReactSharedInternals'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; const ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame; +function setCurrentlyValidatingElement(element) { + if (__DEV__) { + if (element) { + const owner = element._owner; + const stack = describeUnknownElementTypeFrameInDEV( + element.type, + element._source, + owner ? owner.type : null, + ); + ReactDebugCurrentFrame.setExtraStackFrame(stack); + } else { + ReactDebugCurrentFrame.setExtraStackFrame(null); + } + } +} + let propTypesMisspellWarningShown; if (__DEV__) { @@ -140,14 +159,14 @@ function validateExplicitKey(element, parentType) { )}.`; } - ReactDebugCurrentFrame.setCurrentlyValidatingElement(element); + setCurrentlyValidatingElement(element); console.error( 'Each child in a list should have a unique "key" prop.' + '%s%s See https://fb.me/react-warning-keys for more information.', currentComponentErrorInfo, childOwner, ); - ReactDebugCurrentFrame.setCurrentlyValidatingElement(null); + setCurrentlyValidatingElement(null); } } @@ -224,9 +243,9 @@ function validatePropTypes(element) { return; } if (propTypes) { - ReactDebugCurrentFrame.setCurrentlyValidatingElement(element); + setCurrentlyValidatingElement(element); checkPropTypes(propTypes, element.props, 'prop', name); - ReactDebugCurrentFrame.setCurrentlyValidatingElement(null); + setCurrentlyValidatingElement(null); } else if (type.PropTypes !== undefined && !propTypesMisspellWarningShown) { propTypesMisspellWarningShown = true; console.error( @@ -252,7 +271,7 @@ function validatePropTypes(element) { */ function validateFragmentProps(fragment) { if (__DEV__) { - ReactDebugCurrentFrame.setCurrentlyValidatingElement(fragment); + setCurrentlyValidatingElement(fragment); const keys = Object.keys(fragment.props); for (let i = 0; i < keys.length; i++) { @@ -271,7 +290,7 @@ function validateFragmentProps(fragment) { console.error('Invalid attribute `ref` supplied to `React.Fragment`.'); } - ReactDebugCurrentFrame.setCurrentlyValidatingElement(null); + setCurrentlyValidatingElement(null); } } diff --git a/packages/shared/ConsolePatchingDev.js b/packages/shared/ConsolePatchingDev.js index 41c16a761d24..2d651ed4ede4 100644 --- a/packages/shared/ConsolePatchingDev.js +++ b/packages/shared/ConsolePatchingDev.js @@ -12,6 +12,7 @@ // lazily which won't cover if the log function was extracted eagerly. // We could also eagerly patch the method. +let disabledDepth = 0; let prevLog; let prevInfo; let prevWarn; @@ -21,28 +22,34 @@ function disabledLog() {} export function disableLogs(): void { if (__DEV__) { - /* eslint-disable react-internal/no-production-logging */ - prevLog = console.log; - prevInfo = console.info; - prevWarn = console.warn; - prevError = console.error; - // $FlowFixMe Flow thinks console is immutable. - console.log = console.info = console.warn = console.error = disabledLog; - /* eslint-enable react-internal/no-production-logging */ + if (disabledDepth === 0) { + /* eslint-disable react-internal/no-production-logging */ + prevLog = console.log; + prevInfo = console.info; + prevWarn = console.warn; + prevError = console.error; + // $FlowFixMe Flow thinks console is immutable. + console.log = console.info = console.warn = console.error = disabledLog; + /* eslint-enable react-internal/no-production-logging */ + } + disabledDepth++; } } export function reenableLogs(): void { if (__DEV__) { - /* eslint-disable react-internal/no-production-logging */ - // $FlowFixMe Flow thinks console is immutable. - console.log = prevLog; - // $FlowFixMe Flow thinks console is immutable. - console.info = prevInfo; - // $FlowFixMe Flow thinks console is immutable. - console.warn = prevWarn; - // $FlowFixMe Flow thinks console is immutable. - console.error = prevError; - /* eslint-enable react-internal/no-production-logging */ + disabledDepth--; + if (disabledDepth === 0) { + /* eslint-disable react-internal/no-production-logging */ + // $FlowFixMe Flow thinks console is immutable. + console.log = prevLog; + // $FlowFixMe Flow thinks console is immutable. + console.info = prevInfo; + // $FlowFixMe Flow thinks console is immutable. + console.warn = prevWarn; + // $FlowFixMe Flow thinks console is immutable. + console.error = prevError; + /* eslint-enable react-internal/no-production-logging */ + } } } diff --git a/packages/shared/ReactComponentStackFrame.js b/packages/shared/ReactComponentStackFrame.js index 76043711fbeb..b0408e642fe0 100644 --- a/packages/shared/ReactComponentStackFrame.js +++ b/packages/shared/ReactComponentStackFrame.js @@ -10,6 +10,8 @@ import type {Source} from 'shared/ReactElementType'; import type {LazyComponent} from 'react/src/ReactLazy'; +import {enableComponentStackLocations} from 'shared/ReactFeatureFlags'; + import { REACT_SUSPENSE_TYPE, REACT_SUSPENSE_LIST_TYPE, @@ -19,6 +21,120 @@ import { REACT_LAZY_TYPE, } from 'shared/ReactSymbols'; +import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev'; + +import ReactSharedInternals from 'shared/ReactSharedInternals'; + +const {ReactCurrentDispatcher} = ReactSharedInternals; + +let prefix; +export function describeBuiltInComponentFrame( + name: string, + source: void | null | Source, + ownerFn: void | null | Function, +): string { + if (enableComponentStackLocations) { + if (prefix === undefined) { + // Extract the VM specific prefix used by each line. + const match = Error() + .stack.trim() + .match(/\n( *(at )?)/); + prefix = (match && match[1]) || ''; + } + // We use the prefix to ensure our stacks line up with native stack frames. + return '\n' + prefix + name; + } else { + let ownerName = null; + if (__DEV__ && ownerFn) { + ownerName = ownerFn.displayName || ownerFn.name || null; + } + return describeComponentFrame(name, source, ownerName); + } +} + +let reentry = false; + +export function describeNativeComponentFrame( + fn: Function, + construct: boolean, +): string { + // If something asked for a stack inside a fake render, it should get ignored. + if (!fn || reentry) { + return ''; + } + + const control = Error(); + + reentry = true; + let previousDispatcher; + if (__DEV__) { + previousDispatcher = ReactCurrentDispatcher.current; + // Set the dispatcher in DEV because this might be call in the render function + // for warnings. + ReactCurrentDispatcher.current = null; + disableLogs(); + } + try { + // This should throw. + if (construct) { + // Something should be setting the props in the constructor. + const Fake = function() {}; + // $FlowFixMe + Object.defineProperty(Fake.prototype, 'props', { + set: function() { + // We use a throwing setter instead of frozen or non-writable props + // because that won't throw in a non-strict mode function. + throw Error(); + }, + }); + if (typeof Reflect === 'object' && Reflect.construct) { + Reflect.construct(fn, [], Fake); + } else { + fn.call(new Fake()); + } + } else { + fn(); + } + } catch (sample) { + // This is inlined manually because closure doesn't do it for us. + if (sample && typeof sample.stack === 'string') { + // This extracts the first frame from the sample that isn't also in the control. + // Skipping one frame that we assume is the frame that calls the two. + const sampleLines = sample.stack.split('\n'); + const controlLines = control.stack.split('\n'); + let s = sampleLines.length - 1; + let c = controlLines.length - 1; + while (s >= 1 && c >= 0 && sampleLines[s] !== controlLines[c]) { + // We expect at least one stack frame to be shared. + // Typically this will be the root most one. However, stack frames may be + // cut off due to maximum stack limits. In this case, one maybe cut off + // earlier than the other. We assume that the sample is longer or the same + // and there for cut off earlier. So we should find the root most frame in + // the sample somewhere in the control. + c--; + } + for (; s >= 1 && c >= 0; s--, c--) { + // Next we find the first one that isn't the same which should be the + // frame that called our sample function. + if (sampleLines[s] !== controlLines[c]) { + // Return the line we found. + // V8 adds a "new" prefix for native classes. Let's remove it to make it prettier. + return '\n' + sampleLines[s - 1].replace(' at new ', ' at '); + } + } + } + } finally { + reentry = false; + if (__DEV__) { + ReactCurrentDispatcher.current = previousDispatcher; + reenableLogs(); + } + } + // Fallback to just using the name if we couldn't make it throw. + const name = fn ? fn.displayName || fn.name : ''; + return name ? describeBuiltInComponentFrame(name) : ''; +} + const BEFORE_SLASH_RE = /^(.*)[\\\/]/; function describeComponentFrame( @@ -49,24 +165,16 @@ function describeComponentFrame( return '\n in ' + (name || 'Unknown') + sourceInfo; } -export function describeBuiltInComponentFrame( - name: string, - source: void | null | Source, - ownerFn: void | null | Function, -): string { - let ownerName = null; - if (__DEV__ && ownerFn) { - ownerName = ownerFn.displayName || ownerFn.name || null; - } - return describeComponentFrame(name, source, ownerName); -} - export function describeClassComponentFrame( ctor: Function, source: void | null | Source, ownerFn: void | null | Function, ): string { - return describeFunctionComponentFrame(ctor, source, ownerFn); + if (enableComponentStackLocations) { + return describeNativeComponentFrame(ctor, true); + } else { + return describeFunctionComponentFrame(ctor, source, ownerFn); + } } export function describeFunctionComponentFrame( @@ -74,15 +182,19 @@ export function describeFunctionComponentFrame( source: void | null | Source, ownerFn: void | null | Function, ): string { - if (!fn) { - return ''; - } - const name = fn.displayName || fn.name || null; - let ownerName = null; - if (__DEV__ && ownerFn) { - ownerName = ownerFn.displayName || ownerFn.name || null; + if (enableComponentStackLocations) { + return describeNativeComponentFrame(fn, false); + } else { + if (!fn) { + return ''; + } + const name = fn.displayName || fn.name || null; + let ownerName = null; + if (__DEV__ && ownerFn) { + ownerName = ownerFn.displayName || ownerFn.name || null; + } + return describeComponentFrame(name, source, ownerName); } - return describeComponentFrame(name, source, ownerName); } function shouldConstruct(Component: Function) { @@ -102,10 +214,11 @@ export function describeUnknownElementTypeFrameInDEV( return ''; } if (typeof type === 'function') { - if (shouldConstruct(type)) { - return describeClassComponentFrame(type, source, ownerFn); + if (enableComponentStackLocations) { + return describeNativeComponentFrame(type, shouldConstruct(type)); + } else { + return describeFunctionComponentFrame(type, source, ownerFn); } - return describeFunctionComponentFrame(type, source, ownerFn); } if (typeof type === 'string') { return describeBuiltInComponentFrame(type, source, ownerFn); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 529965c9db6f..db00085a4cdc 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -101,7 +101,7 @@ export const deferPassiveEffectCleanupDuringUnmount = false; // a deprecated pattern we want to get rid of in the future export const warnAboutSpreadingKeyToJSX = false; -export const enableComponentStackLocations = false; +export const enableComponentStackLocations = __EXPERIMENTAL__; // Internal-only attempt to debug a React Native issue. See D20130868. export const throwEarlyForMysteriousError = false; From 91b5f34ff78d2345ca087cbe37160f1fd45bc68e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 9 Apr 2020 21:36:27 -0700 Subject: [PATCH 2/9] Cache the result in DEV In DEV it's somewhat likely that we'll see many logs that add component stacks. This could be slow so we cache the results of previous components. --- packages/shared/ReactComponentStackFrame.js | 28 +++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/shared/ReactComponentStackFrame.js b/packages/shared/ReactComponentStackFrame.js index b0408e642fe0..4b56064cc278 100644 --- a/packages/shared/ReactComponentStackFrame.js +++ b/packages/shared/ReactComponentStackFrame.js @@ -53,6 +53,11 @@ export function describeBuiltInComponentFrame( } let reentry = false; +let componentFrameCache; +if (__DEV__) { + const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map; + componentFrameCache = new PossiblyWeakMap(); +} export function describeNativeComponentFrame( fn: Function, @@ -63,6 +68,13 @@ export function describeNativeComponentFrame( return ''; } + if (__DEV__) { + const frame = componentFrameCache.get(fn); + if (frame !== undefined) { + return frame; + } + } + const control = Error(); reentry = true; @@ -119,7 +131,13 @@ export function describeNativeComponentFrame( if (sampleLines[s] !== controlLines[c]) { // Return the line we found. // V8 adds a "new" prefix for native classes. Let's remove it to make it prettier. - return '\n' + sampleLines[s - 1].replace(' at new ', ' at '); + const frame = '\n' + sampleLines[s - 1].replace(' at new ', ' at '); + if (__DEV__) { + if (typeof fn === 'function') { + componentFrameCache.set(fn, frame); + } + } + return frame; } } } @@ -132,7 +150,13 @@ export function describeNativeComponentFrame( } // Fallback to just using the name if we couldn't make it throw. const name = fn ? fn.displayName || fn.name : ''; - return name ? describeBuiltInComponentFrame(name) : ''; + const syntheticFrame = name ? describeBuiltInComponentFrame(name) : ''; + if (__DEV__) { + if (typeof fn === 'function') { + componentFrameCache.set(fn, syntheticFrame); + } + } + return syntheticFrame; } const BEFORE_SLASH_RE = /^(.*)[\\\/]/; From ffc30063525fee4494f9f78dcc690ac845cdb97f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 10 Apr 2020 11:46:45 -0700 Subject: [PATCH 3/9] Fix special case when the function call throws in V8 In V8 we need to ignore the first line. Normally we would never get there because the stacks would differ before that, but the stacks are the same if we end up throwing at the same place as the control. --- packages/shared/ReactComponentStackFrame.js | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/shared/ReactComponentStackFrame.js b/packages/shared/ReactComponentStackFrame.js index 4b56064cc278..f3c80bc1ce0c 100644 --- a/packages/shared/ReactComponentStackFrame.js +++ b/packages/shared/ReactComponentStackFrame.js @@ -129,15 +129,22 @@ export function describeNativeComponentFrame( // Next we find the first one that isn't the same which should be the // frame that called our sample function. if (sampleLines[s] !== controlLines[c]) { - // Return the line we found. - // V8 adds a "new" prefix for native classes. Let's remove it to make it prettier. - const frame = '\n' + sampleLines[s - 1].replace(' at new ', ' at '); - if (__DEV__) { - if (typeof fn === 'function') { - componentFrameCache.set(fn, frame); + // In V8, the first line is describing the message but other VMs don't. + // If we're about to return the first line, and the control is also on the same + // line, that's a pretty good indicator that our sample threw at same line as + // the control. I.e. before we entered the sample frame. So we ignore this result. + // This can happen if you passed a class to function component, or non-function. + if (s !== 1 || c !== 1) { + // V8 adds a "new" prefix for native classes. Let's remove it to make it prettier. + const frame = '\n' + sampleLines[s - 1].replace(' at new ', ' at '); + if (__DEV__) { + if (typeof fn === 'function') { + componentFrameCache.set(fn, frame); + } } + // Return the line we found. + return frame; } - return frame; } } } From 0f2f008b608932477a7f370ca0745123d3de2fbe Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 10 Apr 2020 11:36:08 -0700 Subject: [PATCH 4/9] Log if out of range. --- packages/shared/ConsolePatchingDev.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/shared/ConsolePatchingDev.js b/packages/shared/ConsolePatchingDev.js index 2d651ed4ede4..e1fdc0f27df2 100644 --- a/packages/shared/ConsolePatchingDev.js +++ b/packages/shared/ConsolePatchingDev.js @@ -51,5 +51,11 @@ export function reenableLogs(): void { console.error = prevError; /* eslint-enable react-internal/no-production-logging */ } + if (disabledDepth < 0) { + console.error( + 'disabledDepth fell below zero. ' + + 'This is a bug in React. Please file an issue.', + ); + } } } From 0575e9f3841dc95a8d1fc91d5d60427765cf8757 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 6 Apr 2020 22:08:54 -0700 Subject: [PATCH 5/9] Normalize errors in tests This drops the requirement to include owner to pass the test. --- .../src/__tests__/console-test.js | 7 ++++- .../src/__tests__/ReactComponent-test.js | 7 ++++- .../src/__tests__/ReactDOMComponent-test.js | 7 ++++- .../__tests__/ReactServerRendering-test.js | 7 ++++- .../src/__tests__/ReactUpdates-test.js | 2 +- .../ReactNativeError-test.internal.js | 28 ++++++++----------- ...tIncrementalErrorHandling-test.internal.js | 17 ++++++----- .../ReactIncrementalErrorLogging-test.js | 22 +++++++-------- .../src/__tests__/ReactElementClone-test.js | 4 +-- .../src/__tests__/ReactElementJSX-test.js | 4 +-- .../ReactElementValidator-test.internal.js | 4 +-- scripts/jest/matchers/toWarnDev.js | 14 +++++++++- 12 files changed, 77 insertions(+), 46 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/console-test.js b/packages/react-devtools-shared/src/__tests__/console-test.js index c858a755f9ee..6d400164a835 100644 --- a/packages/react-devtools-shared/src/__tests__/console-test.js +++ b/packages/react-devtools-shared/src/__tests__/console-test.js @@ -61,7 +61,12 @@ describe('console', () => { }); function normalizeCodeLocInfo(str) { - return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); + return ( + str && + str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) { + return '\n in ' + name + ' (at **)'; + }) + ); } it('should not patch console methods that do not receive component stacks', () => { diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index ca3c0d5203c7..552f4b4ee2c7 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -16,7 +16,12 @@ let ReactTestUtils; describe('ReactComponent', () => { function normalizeCodeLocInfo(str) { - return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); + return ( + str && + str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) { + return '\n in ' + name + ' (at **)'; + }) + ); } beforeEach(() => { diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index 5df5d8b294a0..ca67cd809416 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -17,7 +17,12 @@ describe('ReactDOMComponent', () => { const ReactFeatureFlags = require('shared/ReactFeatureFlags'); function normalizeCodeLocInfo(str) { - return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); + return ( + str && + str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) { + return '\n in ' + name + ' (at **)'; + }) + ); } beforeEach(() => { diff --git a/packages/react-dom/src/__tests__/ReactServerRendering-test.js b/packages/react-dom/src/__tests__/ReactServerRendering-test.js index 26fb497a6b66..fb5858e88114 100644 --- a/packages/react-dom/src/__tests__/ReactServerRendering-test.js +++ b/packages/react-dom/src/__tests__/ReactServerRendering-test.js @@ -18,7 +18,12 @@ const enableSuspenseServerRenderer = require('shared/ReactFeatureFlags') .enableSuspenseServerRenderer; function normalizeCodeLocInfo(str) { - return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); + return ( + str && + str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) { + return '\n in ' + name + ' (at **)'; + }) + ); } describe('ReactDOMServer', () => { diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js index 29bccc13b6e0..993869de6e99 100644 --- a/packages/react-dom/src/__tests__/ReactUpdates-test.js +++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js @@ -1615,7 +1615,7 @@ describe('ReactUpdates', () => { Scheduler.unstable_clearYields(); } expect(error).toContain('Warning: Maximum update depth exceeded.'); - expect(stack).toContain('in NonTerminating'); + expect(stack).toContain(' NonTerminating'); // rethrow error to prevent going into an infinite loop when act() exits throw error; }); diff --git a/packages/react-native-renderer/src/__tests__/ReactNativeError-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactNativeError-test.internal.js index a20ab6b9fc04..3cf8f803a47f 100644 --- a/packages/react-native-renderer/src/__tests__/ReactNativeError-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactNativeError-test.internal.js @@ -16,7 +16,12 @@ let createReactNativeComponentClass; let computeComponentStackForErrorReporting; function normalizeCodeLocInfo(str) { - return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); + return ( + str && + str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) { + return '\n in ' + name + ' (at **)'; + }) + ); } describe('ReactNativeError', () => { @@ -74,20 +79,11 @@ describe('ReactNativeError', () => { computeComponentStackForErrorReporting(reactTag), ); - if (__DEV__) { - expect(componentStack).toBe( - '\n' + - ' in View (at **)\n' + - ' in FunctionComponent (at **)\n' + - ' in ClassComponent (at **)', - ); - } else { - expect(componentStack).toBe( - '\n' + - ' in View\n' + - ' in FunctionComponent\n' + - ' in ClassComponent', - ); - } + expect(componentStack).toBe( + '\n' + + ' in View (at **)\n' + + ' in FunctionComponent (at **)\n' + + ' in ClassComponent (at **)', + ); }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 4ba335869e8d..8ab0ba3ed247 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -10,8 +10,8 @@ 'use strict'; +let ReactFeatureFlags = require('shared/ReactFeatureFlags'); let PropTypes; -let ReactFeatureFlags; let React; let ReactNoop; let Scheduler; @@ -37,7 +37,12 @@ describe('ReactIncrementalErrorHandling', () => { } function normalizeCodeLocInfo(str) { - return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); + return ( + str && + str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) { + return '\n in ' + name + ' (at **)'; + }) + ); } it('recovers from errors asynchronously', () => { @@ -1633,10 +1638,8 @@ describe('ReactIncrementalErrorHandling', () => { expect(ReactNoop.getChildren()).toEqual([ span( 'Caught an error:\n' + - (__DEV__ - ? ' in BrokenRender (at **)\n' - : ' in BrokenRender\n') + - (__DEV__ ? ' in ErrorBoundary (at **).' : ' in ErrorBoundary.'), + ' in BrokenRender (at **)\n' + + ' in ErrorBoundary (at **).', ), ]); }); @@ -1669,7 +1672,7 @@ describe('ReactIncrementalErrorHandling', () => { expect(ReactNoop.getChildren()).toEqual([span('Caught an error: Hello')]); }); - if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { + if (!ReactFeatureFlags.disableModulePatternComponents) { it('handles error thrown inside getDerivedStateFromProps of a module-style context provider', () => { function Provider() { return { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js index e4ade52f4eea..745ca558c9aa 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js @@ -59,9 +59,9 @@ describe('ReactIncrementalErrorLogging', () => { ? expect.stringMatching( new RegExp( 'The above error occurred in the component:\n' + - '\\s+in ErrorThrowingComponent (.*)\n' + - '\\s+in span (.*)\n' + - '\\s+in div (.*)\n\n' + + '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) div(.*)\n\n' + 'Consider adding an error boundary to your tree ' + 'to customize error handling behavior\\.', ), @@ -95,9 +95,9 @@ describe('ReactIncrementalErrorLogging', () => { ? expect.stringMatching( new RegExp( 'The above error occurred in the component:\n' + - '\\s+in ErrorThrowingComponent (.*)\n' + - '\\s+in span (.*)\n' + - '\\s+in div (.*)\n\n' + + '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) div(.*)\n\n' + 'Consider adding an error boundary to your tree ' + 'to customize error handling behavior\\.', ), @@ -134,9 +134,9 @@ describe('ReactIncrementalErrorLogging', () => { ? expect.stringMatching( new RegExp( 'The above error occurred in the component:\n' + - '\\s+in ErrorThrowingComponent (.*)\n' + - '\\s+in span (.*)\n' + - '\\s+in div (.*)\n\n' + + '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) div(.*)\n\n' + 'Consider adding an error boundary to your tree ' + 'to customize error handling behavior\\.', ), @@ -206,8 +206,8 @@ describe('ReactIncrementalErrorLogging', () => { ? expect.stringMatching( new RegExp( 'The above error occurred in the component:\n' + - '\\s+in Foo (.*)\n' + - '\\s+in ErrorBoundary (.*)\n\n' + + '\\s+(in|at) Foo (.*)\n' + + '\\s+(in|at) ErrorBoundary (.*)\n\n' + 'React will try to recreate this component tree from scratch ' + 'using the error boundary you provided, ErrorBoundary.', ), diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js index b428787a7bf7..88cbea84cd7e 100644 --- a/packages/react/src/__tests__/ReactElementClone-test.js +++ b/packages/react/src/__tests__/ReactElementClone-test.js @@ -301,8 +301,8 @@ describe('ReactElementClone', () => { 'Warning: Failed prop type: ' + 'Invalid prop `color` of type `number` supplied to `Component`, ' + 'expected `string`.\n' + - ' in Component (created by GrandParent)\n' + - ' in Parent (created by GrandParent)\n' + + ' in Component (at **)\n' + + ' in Parent (at **)\n' + ' in GrandParent', ); }); diff --git a/packages/react/src/__tests__/ReactElementJSX-test.js b/packages/react/src/__tests__/ReactElementJSX-test.js index 0ce12c86f616..fbf9cc7a3ca2 100644 --- a/packages/react/src/__tests__/ReactElementJSX-test.js +++ b/packages/react/src/__tests__/ReactElementJSX-test.js @@ -364,8 +364,8 @@ describe('ReactElement.jsx', () => { ).toErrorDev( 'Warning: Each child in a list should have a unique "key" prop.\n\n' + 'Check the render method of `Parent`. See https://fb.me/react-warning-keys for more information.\n' + - ' in Child (created by Parent)\n' + - ' in Parent', + ' in Child (at **)\n' + + ' in Parent (at **)', ); }); diff --git a/packages/react/src/__tests__/ReactElementValidator-test.internal.js b/packages/react/src/__tests__/ReactElementValidator-test.internal.js index c563e6ce6fea..c8209952c1cc 100644 --- a/packages/react/src/__tests__/ReactElementValidator-test.internal.js +++ b/packages/react/src/__tests__/ReactElementValidator-test.internal.js @@ -227,8 +227,8 @@ describe('ReactElementValidator', () => { 'Warning: Failed prop type: ' + 'Invalid prop `color` of type `number` supplied to `MyComp`, ' + 'expected `string`.\n' + - ' in MyComp (created by ParentComp)\n' + - ' in ParentComp', + ' in MyComp (at **)\n' + + ' in ParentComp (at **)', ); }); diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js index 44ec4e56ed4a..780ab9c9cd92 100644 --- a/scripts/jest/matchers/toWarnDev.js +++ b/scripts/jest/matchers/toWarnDev.js @@ -5,7 +5,19 @@ const util = require('util'); const shouldIgnoreConsoleError = require('../shouldIgnoreConsoleError'); function normalizeCodeLocInfo(str) { - return str && str.replace(/at .+?:\d+/g, 'at **'); + if (typeof str !== 'string') { + return str; + } + // This special case exists only for the special source location in + // ReactElementValidator. That will go away if we remove source locations. + str = str.replace(/Check your code at .+?:\d+/g, 'Check your code at **'); + // V8 format: + // at Component (/path/filename.js:123:45) + // React format: + // in Component (at filename.js:123) + return str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) { + return '\n in ' + name + ' (at **)'; + }); } const createMatcherFor = (consoleMethod, matcherName) => From 0b317f2a27a6fc30e3260b2f2a3e758c53e14a2d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 7 Apr 2020 20:11:48 -0700 Subject: [PATCH 6/9] Special case tests --- .../src/__tests__/ReactDOMComponent-test.js | 30 ++++++++++++------- .../src/__tests__/ReactFragment-test.js | 24 +++++++++++---- .../__tests__/describeComponentFrame-test.js | 6 ++++ 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index ca67cd809416..01bde495d3a7 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -1724,16 +1724,26 @@ describe('ReactDOMComponent', () => { , ); - }).toErrorDev([ - 'Warning: validateDOMNesting(...): cannot appear as a child of ' + - '
.' + - '\n in tr (at **)' + - '\n in div (at **)', - 'Warning: validateDOMNesting(...): cannot appear as a child of ' + - '
.' + - '\n in tr (at **)' + - '\n in div (at **)', - ]); + }).toErrorDev( + ReactFeatureFlags.enableComponentStackLocations + ? [ + // This warning dedupes since they're in the same component. + 'Warning: validateDOMNesting(...): cannot appear as a child of ' + + '
.' + + '\n in tr (at **)' + + '\n in div (at **)', + ] + : [ + 'Warning: validateDOMNesting(...): cannot appear as a child of ' + + '
.' + + '\n in tr (at **)' + + '\n in div (at **)', + 'Warning: validateDOMNesting(...): cannot appear as a child of ' + + '
.' + + '\n in tr (at **)' + + '\n in div (at **)', + ], + ); }); it('warns on invalid nesting at root', () => { diff --git a/packages/react-reconciler/src/__tests__/ReactFragment-test.js b/packages/react-reconciler/src/__tests__/ReactFragment-test.js index c258b25146ac..7c058e95d288 100644 --- a/packages/react-reconciler/src/__tests__/ReactFragment-test.js +++ b/packages/react-reconciler/src/__tests__/ReactFragment-test.js @@ -10,6 +10,7 @@ 'use strict'; let React; +let ReactFeatureFlags; let ReactNoop; let Scheduler; @@ -18,6 +19,7 @@ describe('ReactFragment', () => { jest.resetModules(); React = require('react'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); }); @@ -900,17 +902,27 @@ describe('ReactFragment', () => { ); ReactNoop.render(); - expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev( - 'Each child in a list should have a unique "key" prop.', - ); + if (ReactFeatureFlags.enableComponentStackLocations) { + // The key warning gets deduped because it's in the same component. + expect(Scheduler).toFlushWithoutYielding(); + } else { + expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev( + 'Each child in a list should have a unique "key" prop.', + ); + } expect(ops).toEqual(['Update Stateful']); expect(ReactNoop.getChildren()).toEqual([span(), div()]); ReactNoop.render(); - expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev( - 'Each child in a list should have a unique "key" prop.', - ); + if (ReactFeatureFlags.enableComponentStackLocations) { + // The key warning gets deduped because it's in the same component. + expect(Scheduler).toFlushWithoutYielding(); + } else { + expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev( + 'Each child in a list should have a unique "key" prop.', + ); + } expect(ops).toEqual(['Update Stateful', 'Update Stateful']); expect(ReactNoop.getChildren()).toEqual([span(), div()]); diff --git a/packages/shared/__tests__/describeComponentFrame-test.js b/packages/shared/__tests__/describeComponentFrame-test.js index 7acd340e4737..3b7b843cae44 100644 --- a/packages/shared/__tests__/describeComponentFrame-test.js +++ b/packages/shared/__tests__/describeComponentFrame-test.js @@ -11,6 +11,7 @@ let React; let ReactDOM; +const ReactFeatureFlags = require('shared/ReactFeatureFlags'); describe('Component stack trace displaying', () => { beforeEach(() => { @@ -18,6 +19,11 @@ describe('Component stack trace displaying', () => { ReactDOM = require('react-dom'); }); + if (ReactFeatureFlags.enableComponentStackLocations) { + it("empty test so Jest doesn't complain", () => {}); + return; + } + it('should provide filenames in stack traces', () => { class Component extends React.Component { render() { From 66e67d5276af804ade4dc2188704793b8c87f211 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 9 Apr 2020 20:25:44 -0700 Subject: [PATCH 7/9] Add destructuring to force toObject which throws before the side-effects This ensures that we don't double call yieldValue or advanceTime in tests. Ideally we could use empty destructuring but ES lint doesn't like it. --- ...DOMServerIntegrationHooks-test.internal.js | 4 +- .../ReactErrorBoundaries-test.internal.js | 8 +- ...tIncrementalErrorHandling-test.internal.js | 76 +++++++++---------- .../__tests__/ReactProfiler-test.internal.js | 4 +- .../src/__tests__/forwardRef-test.internal.js | 3 +- 5 files changed, 48 insertions(+), 47 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js index a910b011f050..359c61a2fada 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js @@ -1097,7 +1097,7 @@ describe('ReactDOMServerHooks', () => { it('useOpaqueIdentifier: ID is not used during hydration but is used in an update', async () => { let _setShow; - function App() { + function App({unused}) { Scheduler.unstable_yieldValue('App'); const id = useOpaqueIdentifier(); const [show, setShow] = useState(false); @@ -1129,7 +1129,7 @@ describe('ReactDOMServerHooks', () => { it('useOpaqueIdentifier: ID is not used during hydration but is used in an update in legacy', async () => { let _setShow; - function App() { + function App({unused}) { Scheduler.unstable_yieldValue('App'); const id = useOpaqueIdentifier(); const [show, setShow] = useState(false); diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 7601a4723097..81427c97eb05 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -497,7 +497,7 @@ describe('ReactErrorBoundaries', () => { } }; - BrokenUseEffect = props => { + BrokenUseEffect = ({children}) => { Scheduler.unstable_yieldValue('BrokenUseEffect render'); React.useEffect(() => { @@ -505,10 +505,10 @@ describe('ReactErrorBoundaries', () => { throw new Error('Hello'); }); - return props.children; + return children; }; - BrokenUseLayoutEffect = props => { + BrokenUseLayoutEffect = ({children}) => { Scheduler.unstable_yieldValue('BrokenUseLayoutEffect render'); React.useLayoutEffect(() => { @@ -518,7 +518,7 @@ describe('ReactErrorBoundaries', () => { throw new Error('Hello'); }); - return props.children; + return children; }; NoopErrorBoundary = class extends React.Component { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 8ab0ba3ed247..b9b5836e7096 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -62,17 +62,17 @@ describe('ReactIncrementalErrorHandling', () => { } } - function ErrorMessage(props) { + function ErrorMessage({error}) { Scheduler.unstable_yieldValue('ErrorMessage'); - return ; + return ; } - function Indirection(props) { + function Indirection({children}) { Scheduler.unstable_yieldValue('Indirection'); - return props.children || null; + return children || null; } - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('throw'); throw new Error('oops!'); } @@ -156,17 +156,17 @@ describe('ReactIncrementalErrorHandling', () => { } } - function ErrorMessage(props) { + function ErrorMessage({error}) { Scheduler.unstable_yieldValue('ErrorMessage'); - return ; + return ; } - function Indirection(props) { + function Indirection({children}) { Scheduler.unstable_yieldValue('Indirection'); - return props.children || null; + return children || null; } - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('throw'); throw new Error('oops!'); } @@ -346,17 +346,17 @@ describe('ReactIncrementalErrorHandling', () => { }); it('retries one more time before handling error', () => { - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('BadRender'); throw new Error('oops'); } - function Sibling() { + function Sibling({unused}) { Scheduler.unstable_yieldValue('Sibling'); return ; } - function Parent() { + function Parent({unused}) { Scheduler.unstable_yieldValue('Parent'); return ( <> @@ -386,7 +386,7 @@ describe('ReactIncrementalErrorHandling', () => { }); it('retries one more time if an error occurs during a render that expires midway through the tree', () => { - function Oops() { + function Oops({unused}) { Scheduler.unstable_yieldValue('Oops'); throw new Error('Oops'); } @@ -396,7 +396,7 @@ describe('ReactIncrementalErrorHandling', () => { return text; } - function App() { + function App({unused}) { return ( <> @@ -530,7 +530,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender(props) { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -576,7 +576,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender(props) { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -623,7 +623,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender(props) { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -664,7 +664,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -703,7 +703,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -744,7 +744,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -784,7 +784,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -862,12 +862,12 @@ describe('ReactIncrementalErrorHandling', () => { }); it('can schedule updates after uncaught error in render on mount', () => { - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } - function Foo() { + function Foo({unused}) { Scheduler.unstable_yieldValue('Foo'); return null; } @@ -887,24 +887,24 @@ describe('ReactIncrementalErrorHandling', () => { }); it('can schedule updates after uncaught error in render on update', () => { - function BrokenRender(props) { + function BrokenRender({shouldThrow}) { Scheduler.unstable_yieldValue('BrokenRender'); - if (props.throw) { + if (shouldThrow) { throw new Error('Hello'); } return null; } - function Foo() { + function Foo({unused}) { Scheduler.unstable_yieldValue('Foo'); return null; } - ReactNoop.render(); + ReactNoop.render(); expect(Scheduler).toFlushAndYield(['BrokenRender']); expect(() => { - ReactNoop.render(); + ReactNoop.render(); expect(Scheduler).toFlushWithoutYielding(); }).toThrow('Hello'); expect(Scheduler).toHaveYielded([ @@ -1454,13 +1454,13 @@ describe('ReactIncrementalErrorHandling', () => { } } - function Indirection(props) { + function Indirection({children}) { Scheduler.unstable_yieldValue('Indirection'); - return props.children; + return children; } const notAnError = {nonStandardMessage: 'oops'}; - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('BadRender'); throw notAnError; } @@ -1519,17 +1519,17 @@ describe('ReactIncrementalErrorHandling', () => { } } - function ErrorMessage(props) { + function ErrorMessage({error}) { Scheduler.unstable_yieldValue('ErrorMessage'); - return ; + return ; } - function BadRenderSibling(props) { + function BadRenderSibling({unused}) { Scheduler.unstable_yieldValue('BadRenderSibling'); return null; } - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('throw'); throw new Error('oops!'); } @@ -1567,7 +1567,7 @@ describe('ReactIncrementalErrorHandling', () => { // This test seems a bit contrived, but it's based on an actual regression // where we checked for the existence of didUpdate instead of didMount, and // didMount was not defined. - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('throw'); throw new Error('oops!'); } @@ -1711,7 +1711,7 @@ describe('ReactIncrementalErrorHandling', () => { it('uncaught errors should be discarded if the render is aborted', async () => { const root = ReactNoop.createRoot(); - function Oops() { + function Oops({unused}) { Scheduler.unstable_yieldValue('Oops'); throw Error('Oops'); } diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index 1fce1caa623c..b3019fc280e5 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -1041,7 +1041,7 @@ describe('Profiler', () => { it('should accumulate actual time after an error handled by componentDidCatch()', () => { const callback = jest.fn(); - const ThrowsError = () => { + const ThrowsError = ({unused}) => { Scheduler.unstable_advanceTime(3); throw Error('expected error'); }; @@ -1120,7 +1120,7 @@ describe('Profiler', () => { it('should accumulate actual time after an error handled by getDerivedStateFromError()', () => { const callback = jest.fn(); - const ThrowsError = () => { + const ThrowsError = ({unused}) => { Scheduler.unstable_advanceTime(10); throw Error('expected error'); }; diff --git a/packages/react/src/__tests__/forwardRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js index d98dfabcad67..ef8c7422f87a 100644 --- a/packages/react/src/__tests__/forwardRef-test.internal.js +++ b/packages/react/src/__tests__/forwardRef-test.internal.js @@ -159,8 +159,9 @@ describe('forwardRef', () => { } function Wrapper(props) { + const forwardedRef = props.forwardedRef; Scheduler.unstable_yieldValue('Wrapper'); - return ; + return ; } const RefForwardingComponent = React.forwardRef((props, ref) => ( From df0ef462f544e113f0aba07554d239a223c814e8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 10 Apr 2020 00:06:22 -0700 Subject: [PATCH 8/9] Add Reflect to lint --- scripts/rollup/validate/eslintrc.cjs.js | 1 + scripts/rollup/validate/eslintrc.fb.js | 1 + scripts/rollup/validate/eslintrc.rn.js | 1 + scripts/rollup/validate/eslintrc.umd.js | 1 + 4 files changed, 4 insertions(+) diff --git a/scripts/rollup/validate/eslintrc.cjs.js b/scripts/rollup/validate/eslintrc.cjs.js index 6e392ed42f49..008cbf1d7827 100644 --- a/scripts/rollup/validate/eslintrc.cjs.js +++ b/scripts/rollup/validate/eslintrc.cjs.js @@ -14,6 +14,7 @@ module.exports = { WeakMap: true, WeakSet: true, Uint16Array: true, + Reflect: true, // Vendor specific MSApp: true, __REACT_DEVTOOLS_GLOBAL_HOOK__: true, diff --git a/scripts/rollup/validate/eslintrc.fb.js b/scripts/rollup/validate/eslintrc.fb.js index f8965a81ff49..b05619d10bd0 100644 --- a/scripts/rollup/validate/eslintrc.fb.js +++ b/scripts/rollup/validate/eslintrc.fb.js @@ -14,6 +14,7 @@ module.exports = { WeakMap: true, WeakSet: true, Uint16Array: true, + Reflect: true, // Vendor specific MSApp: true, __REACT_DEVTOOLS_GLOBAL_HOOK__: true, diff --git a/scripts/rollup/validate/eslintrc.rn.js b/scripts/rollup/validate/eslintrc.rn.js index 9906dfb5b8ab..49b993ed90d7 100644 --- a/scripts/rollup/validate/eslintrc.rn.js +++ b/scripts/rollup/validate/eslintrc.rn.js @@ -13,6 +13,7 @@ module.exports = { Proxy: true, WeakMap: true, WeakSet: true, + Reflect: true, // Vendor specific MSApp: true, __REACT_DEVTOOLS_GLOBAL_HOOK__: true, diff --git a/scripts/rollup/validate/eslintrc.umd.js b/scripts/rollup/validate/eslintrc.umd.js index 9537b41a357a..8dfe4ec5630c 100644 --- a/scripts/rollup/validate/eslintrc.umd.js +++ b/scripts/rollup/validate/eslintrc.umd.js @@ -13,6 +13,7 @@ module.exports = { WeakMap: true, WeakSet: true, Uint16Array: true, + Reflect: true, // Vendor specific MSApp: true, __REACT_DEVTOOLS_GLOBAL_HOOK__: true, From fbb0189fc795f9213187604da55780f32471b0df Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 9 Apr 2020 23:45:29 -0700 Subject: [PATCH 9/9] Fixture --- fixtures/stacks/BabelClass-compiled.js | 25 +++++++++ fixtures/stacks/BabelClass-compiled.js.map | 1 + fixtures/stacks/BabelClass.js | 8 +++ fixtures/stacks/Component.js | 20 ++++++++ fixtures/stacks/Example.js | 59 ++++++++++++++++++++++ fixtures/stacks/babel.config.json | 5 ++ fixtures/stacks/index.html | 51 +++++++++++++++++++ 7 files changed, 169 insertions(+) create mode 100644 fixtures/stacks/BabelClass-compiled.js create mode 100644 fixtures/stacks/BabelClass-compiled.js.map create mode 100644 fixtures/stacks/BabelClass.js create mode 100644 fixtures/stacks/Component.js create mode 100644 fixtures/stacks/Example.js create mode 100644 fixtures/stacks/babel.config.json create mode 100644 fixtures/stacks/index.html diff --git a/fixtures/stacks/BabelClass-compiled.js b/fixtures/stacks/BabelClass-compiled.js new file mode 100644 index 000000000000..dc1424e138e5 --- /dev/null +++ b/fixtures/stacks/BabelClass-compiled.js @@ -0,0 +1,25 @@ +function _inheritsLoose(subClass, superClass) { + subClass.prototype = Object.create(superClass.prototype); + subClass.prototype.constructor = subClass; + subClass.__proto__ = superClass; +} + +// Compile this with Babel. +// babel --config-file ./babel.config.json BabelClass.js --out-file BabelClass-compiled.js --source-maps +let BabelClass = /*#__PURE__*/ (function(_React$Component) { + _inheritsLoose(BabelClass, _React$Component); + + function BabelClass() { + return _React$Component.apply(this, arguments) || this; + } + + var _proto = BabelClass.prototype; + + _proto.render = function render() { + return this.props.children; + }; + + return BabelClass; +})(React.Component); + +//# sourceMappingURL=BabelClass-compiled.js.map diff --git a/fixtures/stacks/BabelClass-compiled.js.map b/fixtures/stacks/BabelClass-compiled.js.map new file mode 100644 index 000000000000..b36937026212 --- /dev/null +++ b/fixtures/stacks/BabelClass-compiled.js.map @@ -0,0 +1 @@ +{"version":3,"sources":["BabelClass.js"],"names":[],"mappings":";;AAAA;AACA;IAEM,U;;;;;;;;;SACJ,M,GAAA,kBAAS;AACP,WAAO,KAAK,KAAL,CAAW,QAAlB;AACD,G;;;EAHsB,KAAK,CAAC,S","file":"BabelClass-compiled.js","sourcesContent":["// Compile this with Babel.\n// babel --config-file ./babel.config.json BabelClass.js --out-file BabelClass-compiled.js --source-maps\n\nclass BabelClass extends React.Component {\n render() {\n return this.props.children;\n }\n}\n"]} \ No newline at end of file diff --git a/fixtures/stacks/BabelClass.js b/fixtures/stacks/BabelClass.js new file mode 100644 index 000000000000..d76637c1ae89 --- /dev/null +++ b/fixtures/stacks/BabelClass.js @@ -0,0 +1,8 @@ +// Compile this with Babel. +// babel --config-file ./babel.config.json BabelClass.js --out-file BabelClass-compiled.js --source-maps + +class BabelClass extends React.Component { + render() { + return this.props.children; + } +} diff --git a/fixtures/stacks/Component.js b/fixtures/stacks/Component.js new file mode 100644 index 000000000000..b4cd8a00123a --- /dev/null +++ b/fixtures/stacks/Component.js @@ -0,0 +1,20 @@ +// Example + +const Throw = React.lazy(() => { + throw new Error('Example'); +}); + +const Component = React.memo(function Component({children}) { + return children; +}); + +function DisplayName({children}) { + return children; +} +DisplayName.displayName = 'Custom Name'; + +class NativeClass extends React.Component { + render() { + return this.props.children; + } +} diff --git a/fixtures/stacks/Example.js b/fixtures/stacks/Example.js new file mode 100644 index 000000000000..086934c0d9e1 --- /dev/null +++ b/fixtures/stacks/Example.js @@ -0,0 +1,59 @@ +// Example + +const x = React.createElement; + +class ErrorBoundary extends React.Component { + static getDerivedStateFromError(error) { + return { + error: error, + }; + } + + componentDidCatch(error, errorInfo) { + console.log(error.message, errorInfo.componentStack); + this.setState({ + componentStack: errorInfo.componentStack, + }); + } + + render() { + if (this.state && this.state.error) { + return x( + 'div', + null, + x('h3', null, this.state.error.message), + x('pre', null, this.state.componentStack) + ); + } + return this.props.children; + } +} + +function Example() { + let state = React.useState(false); + return x( + ErrorBoundary, + null, + x( + DisplayName, + null, + x( + React.SuspenseList, + null, + x( + NativeClass, + null, + x( + BabelClass, + null, + x( + React.Suspense, + null, + x('div', null, x(Component, null, x(Throw))) + ) + ) + ) + ) + ) + ); +} diff --git a/fixtures/stacks/babel.config.json b/fixtures/stacks/babel.config.json new file mode 100644 index 000000000000..1b89e92c4b32 --- /dev/null +++ b/fixtures/stacks/babel.config.json @@ -0,0 +1,5 @@ +{ + "plugins": [ + ["@babel/plugin-transform-classes", {"loose": true}] + ] +} diff --git a/fixtures/stacks/index.html b/fixtures/stacks/index.html new file mode 100644 index 000000000000..f4ee24472a82 --- /dev/null +++ b/fixtures/stacks/index.html @@ -0,0 +1,51 @@ + + + + + Component Stacks + + + +
+

+ To install React, follow the instructions on + GitHub. +

+

+ If you can see this, React is not working right. + If you checked out the source from GitHub make sure to run npm run build. +

+
+ + + + + + +

The above stack should look something like this:

+ 
+
+    at Lazy
+    at Component (/stacks/Component.js:7:50)
+    at div
+    at Suspense
+    at BabelClass (/stacks/BabelClass-compiled.js:13:29)
+    at NativeClass (/stacks/Component.js:16:1)
+    at SuspenseList
+    at Custom Name (/stacks/Component.js:11:23)
+    at ErrorBoundary (/stacks/Example.js:5:1)
+    at Example (/stacks/Example.js:33:21)
+ +