From 98d410f5005988644d01c9ec79b7181c3dd6c847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 10 Apr 2020 13:32:12 -0700 Subject: [PATCH] Build Component Stacks from Native Stack Frames (#18561) * Implement component stack extraction hack * Normalize errors in tests This drops the requirement to include owner to pass the test. * Special case tests * 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. * 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. * Fixture * Add Reflect to lint * Log if out of range. * 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. --- 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 +++++ .../src/__tests__/console-test.js | 7 +- .../src/__tests__/ReactComponent-test.js | 7 +- .../src/__tests__/ReactDOMComponent-test.js | 37 +++- ...DOMServerIntegrationHooks-test.internal.js | 4 +- .../ReactErrorBoundaries-test.internal.js | 8 +- .../__tests__/ReactServerRendering-test.js | 7 +- .../src/__tests__/ReactUpdates-test.js | 2 +- .../ReactNativeError-test.internal.js | 28 ++- .../src/__tests__/ReactFragment-test.js | 24 ++- ...tIncrementalErrorHandling-test.internal.js | 93 +++++---- .../ReactIncrementalErrorLogging-test.js | 22 +- packages/react/src/ReactDebugCurrentFrame.js | 25 +-- packages/react/src/ReactElementValidator.js | 23 ++- .../src/__tests__/ReactElementClone-test.js | 4 +- .../src/__tests__/ReactElementJSX-test.js | 4 +- .../ReactElementValidator-test.internal.js | 4 +- .../__tests__/ReactProfiler-test.internal.js | 4 +- .../src/__tests__/forwardRef-test.internal.js | 3 +- .../react/src/jsx/ReactJSXElementValidator.js | 31 ++- packages/shared/ConsolePatchingDev.js | 49 +++-- packages/shared/ReactComponentStackFrame.js | 192 +++++++++++++++--- packages/shared/ReactFeatureFlags.js | 2 +- .../__tests__/describeComponentFrame-test.js | 6 + scripts/jest/matchers/toWarnDev.js | 14 +- 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 + 35 files changed, 594 insertions(+), 179 deletions(-) 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)
+ + 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..01bde495d3a7 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(() => { @@ -1719,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-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-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__/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/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 4ba335869e8d..b9b5836e7096 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', () => { @@ -57,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!'); } @@ -151,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!'); } @@ -341,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 ( <> @@ -381,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'); } @@ -391,7 +396,7 @@ describe('ReactIncrementalErrorHandling', () => { return text; } - function App() { + function App({unused}) { return ( <> @@ -525,7 +530,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender(props) { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -571,7 +576,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender(props) { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -618,7 +623,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender(props) { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -659,7 +664,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -698,7 +703,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -739,7 +744,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -779,7 +784,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -857,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; } @@ -882,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([ @@ -1449,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; } @@ -1514,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!'); } @@ -1562,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!'); } @@ -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 { @@ -1708,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-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/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/__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/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) => ( 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..e1fdc0f27df2 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,40 @@ 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 */ + } + if (disabledDepth < 0) { + console.error( + 'disabledDepth fell below zero. ' + + 'This is a bug in React. Please file an issue.', + ); + } } } diff --git a/packages/shared/ReactComponentStackFrame.js b/packages/shared/ReactComponentStackFrame.js index 76043711fbeb..f3c80bc1ce0c 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,151 @@ 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; +let componentFrameCache; +if (__DEV__) { + const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map; + componentFrameCache = new PossiblyWeakMap(); +} + +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 ''; + } + + if (__DEV__) { + const frame = componentFrameCache.get(fn); + if (frame !== undefined) { + return frame; + } + } + + 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]) { + // 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; + } + } + } + } + } 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 : ''; + const syntheticFrame = name ? describeBuiltInComponentFrame(name) : ''; + if (__DEV__) { + if (typeof fn === 'function') { + componentFrameCache.set(fn, syntheticFrame); + } + } + return syntheticFrame; +} + const BEFORE_SLASH_RE = /^(.*)[\\\/]/; function describeComponentFrame( @@ -49,24 +196,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 +213,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 +245,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; 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() { 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) => 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,