From 579d9194f549510ef77d5a1d0be408380d1ad4cd Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sat, 13 May 2017 22:20:49 +0100 Subject: [PATCH] Include parent stack but mark owner chain as pertinent --- .../classic/element/ReactElementValidator.js | 6 ++++- .../__tests__/ReactElementValidator-test.js | 24 ++++++++++++++----- .../hooks/ReactComponentTreeHook.js | 19 +++++++++++---- .../ReactJSXElementValidator-test.js | 18 ++++++++++---- 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js index f7ee32194eab9..4f17a1967020b 100644 --- a/src/isomorphic/classic/element/ReactElementValidator.js +++ b/src/isomorphic/classic/element/ReactElementValidator.js @@ -228,7 +228,11 @@ var ReactElementValidator = { props.__source !== undefined ? props.__source : null; - ReactComponentTreeHook.pushNonStandardWarningStack(true, currentSource); + ReactComponentTreeHook.pushNonStandardWarningStack( + true, + true, + currentSource, + ); warning( false, 'React.createElement: type is invalid -- expected a string (for ' + diff --git a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js index 80c0b0252de76..0a9e58251e46d 100644 --- a/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js +++ b/src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js @@ -535,7 +535,7 @@ describe('ReactElementValidator', () => { } function App() { - return React.createElement(Foo); + return React.createElement('div', null, React.createElement(Foo)); } try { @@ -557,14 +557,26 @@ describe('ReactElementValidator', () => { var stack = console.stack.mock.calls[0][0]; expect(Array.isArray(stack)).toBe(true); expect(stack.map(frame => frame.functionName)).toEqual([ - 'Foo', - 'App', - null, + 'Foo', // is inside Foo + 'App', // is inside App + 'App', //
is inside App + null, // is outside a component + ]); + expect(stack.map(frame => frame.isPertinent)).toEqual([ + true, // caused the error + true, // caused to render + false, //
is not pertinent + true, // caused to render ]); expect( stack.map(frame => frame.fileName && frame.fileName.slice(-8)), - ).toEqual([null, null, null]); - expect(stack.map(frame => frame.lineNumber)).toEqual([null, null, null]); + ).toEqual([null, null, null, null]); + expect(stack.map(frame => frame.lineNumber)).toEqual([ + null, + null, + null, + null, + ]); } finally { delete console.stack; delete console.stackEnd; diff --git a/src/isomorphic/hooks/ReactComponentTreeHook.js b/src/isomorphic/hooks/ReactComponentTreeHook.js index a935fa71e675b..6a68e27899687 100644 --- a/src/isomorphic/hooks/ReactComponentTreeHook.js +++ b/src/isomorphic/hooks/ReactComponentTreeHook.js @@ -405,6 +405,7 @@ var ReactComponentTreeHook = { pushNonStandardWarningStack( isCreatingElement: boolean, + isOwnerChainPertinent: boolean, currentSource: ?Source, ) { if (typeof console.stack !== 'function') { @@ -414,30 +415,40 @@ var ReactComponentTreeHook = { var stack = []; var currentOwner = ReactCurrentOwner.current; var id = currentOwner && currentOwner._debugID; + var nextIDInOwnerChain = id; try { if (isCreatingElement) { stack.push({ + isPertinent: true, + functionName: id ? ReactComponentTreeHook.getDisplayName(id) : null, fileName: currentSource ? currentSource.fileName : null, lineNumber: currentSource ? currentSource.lineNumber : null, - functionName: id ? ReactComponentTreeHook.getDisplayName(id) : null, }); } while (id) { var element = ReactComponentTreeHook.getElement(id); + var parentID = ReactComponentTreeHook.getParentID(id); var ownerID = ReactComponentTreeHook.getOwnerID(id); var ownerName = ownerID ? ReactComponentTreeHook.getDisplayName(ownerID) : null; var source = element && element._source; + // For some warnings, only the owner chain is pertinent + var isPertintent = isOwnerChainPertinent + ? nextIDInOwnerChain === id || !nextIDInOwnerChain + : true; stack.push({ + isPertinent: isPertintent, + functionName: ownerName, fileName: source ? source.fileName : null, lineNumber: source ? source.lineNumber : null, - functionName: ownerName, }); - // Owner stack is more useful for visual representation - id = ownerID || ReactComponentTreeHook.getParentID(id); + if (isOwnerChainPertinent && isPertintent) { + nextIDInOwnerChain = ownerID; + } + id = parentID; } } catch (err) { // Internal state is messed up. diff --git a/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js b/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js index 2f8b8816b3eb7..3093f8922b01c 100644 --- a/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js +++ b/src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js @@ -410,7 +410,7 @@ describe('ReactJSXElementValidator', () => { } function App() { - return ; + return
; } try { @@ -432,17 +432,25 @@ describe('ReactJSXElementValidator', () => { var stack = console.stack.mock.calls[0][0]; expect(Array.isArray(stack)).toBe(true); expect(stack.map(frame => frame.functionName)).toEqual([ - 'Foo', - 'App', - null, + 'Foo', // is inside Foo + 'App', // is inside App + 'App', //
is inside App + null, // is outside a component + ]); + expect(stack.map(frame => frame.isPertinent)).toEqual([ + true, // caused the error + true, // caused to render + false, //
is unrelated + true, // caused to render ]); expect( stack.map(frame => frame.fileName && frame.fileName.slice(-8)), - ).toEqual(['-test.js', '-test.js', '-test.js']); + ).toEqual(['-test.js', '-test.js', '-test.js', '-test.js']); expect(stack.map(frame => typeof frame.lineNumber)).toEqual([ 'number', 'number', 'number', + 'number', ]); } finally { delete console.stack;