Include parent stack but mark owner chain as pertinent

facebook · May 13, 2017 · 579d919 · 579d919
1 parent e403f30
commit 579d919
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 16 deletions.
diff --git 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
@@ -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', // <Bad> is inside Foo
+        'App', // <Foo> is inside App
+        'App', // <div> is inside App
+        null, // <App> is outside a component
+      ]);
+      expect(stack.map(frame => frame.isPertinent)).toEqual([
+        true, // <Bad> caused the error
+        true, // <Foo> caused <Bad> to render
+        false, // <div> is not pertinent
+        true, // <App> caused <Foo> 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
@@ -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
@@ -410,7 +410,7 @@ describe('ReactJSXElementValidator', () => {
     }
 
     function App() {
-      return <Foo />;
+      return <div><Foo /></div>;
     }
 
     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', // <Bad> is inside Foo
+        'App', // <Foo> is inside App
+        'App', // <div> is inside App
+        null, // <App> is outside a component
+      ]);
+      expect(stack.map(frame => frame.isPertinent)).toEqual([
+        true, // <Bad> caused the error
+        true, // <Foo> caused <Bad> to render
+        false, // <div> is unrelated
+        true, // <App> caused <Foo> 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;