Refactor Debug Frames to Enable Renderers to Provide Custom Logic

scripts/fiber/tests-passing-except-dev.txt

@@ -1,5 +1,2 @@
 src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
-* gives source code refs for unknown prop warning (ssr)
-* gives source code refs for unknown prop warning for exact elements (ssr)
 * gives source code refs for unknown prop warning for exact elements in composition (ssr)
-* should suggest property name if available (ssr)

scripts/fiber/tests-passing.txt

@@ -853,10 +853,13 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
 * should warn about props that are no longer supported
 * should warn about props that are no longer supported (ssr)
 * gives source code refs for unknown prop warning
+* gives source code refs for unknown prop warning (ssr)
 * gives source code refs for unknown prop warning for update render
 * gives source code refs for unknown prop warning for exact elements
+* gives source code refs for unknown prop warning for exact elements (ssr)
 * gives source code refs for unknown prop warning for exact elements in composition
 * should suggest property name if available
+* should suggest property name if available (ssr)
 * renders innerHTML and preserves whitespace
 * render and then updates innerHTML and preserves whitespace
 

scripts/rollup/bundles.js

@@ -381,7 +381,11 @@ const bundles = [
     label: 'shallow-renderer',
     manglePropertiesOnProd: false,
     name: 'react-test-renderer/shallow',
-    paths: ['src/renderers/shared/**/*.js', 'src/renderers/testing/**/*.js'],
+    paths: [
+      'src/renderers/shared/**/*.js',
+      'src/renderers/testing/**/*.js',
+      'src/shared/**/*.js',
+    ],
   },
 
   /******* React Noop Renderer (used only for fixtures/fiber-debugger) *******/

src/isomorphic/children/traverseAllChildren.js

@@ -24,7 +24,7 @@ var REACT_ELEMENT_TYPE =
   0xeac7;
 
 if (__DEV__) {
-  var {getCurrentStackAddendum} = require('ReactComponentTreeHook');
+  var {getStackAddendum} = require('ReactDebugCurrentFrame');
 }
 
 var SEPARATOR = '.';
@@ -171,7 +171,7 @@ function traverseAllChildrenImpl(
             'Using Maps as children is unsupported and will likely yield ' +
               'unexpected results. Convert it to a sequence/iterable of keyed ' +
               'ReactElements instead.%s',
-            getCurrentStackAddendum(),
+            getStackAddendum(),
           );
           didWarnAboutMaps = true;
         }
@@ -196,7 +196,7 @@ function traverseAllChildrenImpl(
         addendum =
           ' If you meant to render a collection of children, use an array ' +
           'instead.' +
-          getCurrentStackAddendum();
+          getStackAddendum();
       }
       var childrenString = '' + children;
       invariant(

src/isomorphic/classic/element/ReactDebugCurrentFrame.js

@@ -12,46 +12,18 @@
 
 'use strict';
 
-import type {Fiber} from 'ReactFiber';
-import type {DebugID} from 'ReactInstanceType';
-
 const ReactDebugCurrentFrame = {};
 
 if (__DEV__) {
-  var {
-    getStackAddendumByID,
-    getCurrentStackAddendum,
-  } = require('ReactComponentTreeHook');
-  var {
-    getStackAddendumByWorkInProgressFiber,
-  } = require('ReactFiberComponentTreeHook');
-
   // Component that is being worked on
-  ReactDebugCurrentFrame.current = (null: Fiber | DebugID | null);
-
-  // Element that is being cloned or created
-  ReactDebugCurrentFrame.element = (null: *);
+  ReactDebugCurrentFrame.getCurrentStack = (null: null | (() => string | null));
 
   ReactDebugCurrentFrame.getStackAddendum = function(): string | null {
-    let stack = null;
-    const current = ReactDebugCurrentFrame.current;
-    const element = ReactDebugCurrentFrame.element;
-    if (current !== null) {
-      if (typeof current === 'number') {
-        // DebugID from Stack.
-        const debugID = current;
-        stack = getStackAddendumByID(debugID);
-      } else if (typeof current.tag === 'number') {
-        // This is a Fiber.
-        // The stack will only be correct if this is a work in progress
-        // version and we're calling it during reconciliation.
-        const workInProgress = current;
-        stack = getStackAddendumByWorkInProgressFiber(workInProgress);
-      }
-    } else if (element !== null) {
-      stack = getCurrentStackAddendum(element);
+    const impl = ReactDebugCurrentFrame.getCurrentStack;
+    if (impl) {
+      return impl();
     }
-    return stack;
+    return null;
   };
 }
 

src/isomorphic/classic/element/ReactElementValidator.js

@@ -21,14 +21,42 @@
 var ReactCurrentOwner = require('ReactCurrentOwner');
 var ReactElement = require('ReactElement');
 
-var getComponentName = require('getComponentName');
-
 if (__DEV__) {
   var checkPropTypes = require('prop-types/checkPropTypes');
   var lowPriorityWarning = require('lowPriorityWarning');
   var ReactDebugCurrentFrame = require('ReactDebugCurrentFrame');
   var warning = require('fbjs/lib/warning');
-  var {getCurrentStackAddendum} = require('ReactComponentTreeHook');
+  var describeComponentFrame = require('describeComponentFrame');
+  var getComponentName = require('getComponentName');
+
+  var currentlyValidatingElement = null;
+
+  var getDisplayName = function(element: ?ReactElement): string {
+    if (element == null) {
+      return '#empty';
+    } else if (typeof element === 'string' || typeof element === 'number') {
+      return '#text';
+    } else if (typeof element.type === 'string') {
+      return element.type;
+    } else {
+      return element.type.displayName || element.type.name || 'Unknown';
+    }
+  };
+
+  var getStackAddendum = function(): string {
+    var stack = '';
+    if (currentlyValidatingElement) {
+      var name = getDisplayName(currentlyValidatingElement);
+      var owner = currentlyValidatingElement._owner;
+      stack += describeComponentFrame(
+        name,
+        currentlyValidatingElement._source,
+        owner && getComponentName(owner),
+      );
+    }
+    stack += ReactDebugCurrentFrame.getStackAddendum() || '';
+    return stack;
+  };
 }
 
 var ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator;
@@ -115,14 +143,16 @@ function validateExplicitKey(element, parentType) {
     childOwner = ` It was passed a child from ${getComponentName(element._owner)}.`;
   }
 
+  currentlyValidatingElement = element;
   warning(
     false,
     'Each child in an array or iterator should have a unique "key" prop.' +
       '%s%s See https://fb.me/react-warning-keys for more information.%s',
     currentComponentErrorInfo,
     childOwner,
-    getCurrentStackAddendum(element),
+    getStackAddendum(),
   );
+  currentlyValidatingElement = null;
 }
 
 /**
@@ -193,13 +223,9 @@ function validatePropTypes(element) {
     : componentClass.propTypes;
 
   if (propTypes) {
-    checkPropTypes(
-      propTypes,
-      element.props,
-      'prop',
-      name,
-      ReactDebugCurrentFrame.getStackAddendum,
-    );
+    currentlyValidatingElement = element;
+    checkPropTypes(propTypes, element.props, 'prop', name, getStackAddendum);
+    currentlyValidatingElement = null;
   }
   if (typeof componentClass.getDefaultProps === 'function') {
     warning(
@@ -235,7 +261,7 @@ var ReactElementValidator = {
         info += getDeclarationErrorAddendum();
       }
 
-      info += getCurrentStackAddendum();
+      info += ReactDebugCurrentFrame.getStackAddendum() || '';
 
       warning(
         false,
@@ -255,10 +281,6 @@ var ReactElementValidator = {
       return element;
     }
 
-    if (__DEV__) {
-      ReactDebugCurrentFrame.element = element;
-    }
-
     // Skip key warning if the type isn't valid since our key validation logic
     // doesn't expect a non-string/function type and can throw confusing errors.
     // We don't want exception behavior to differ between dev and prod.
@@ -272,10 +294,6 @@ var ReactElementValidator = {
 
     validatePropTypes(element);
 
-    if (__DEV__) {
-      ReactDebugCurrentFrame.element = null;
-    }
-
     return element;
   },
 
@@ -306,16 +324,10 @@ var ReactElementValidator = {
 
   cloneElement: function(element, props, children) {
     var newElement = ReactElement.cloneElement.apply(this, arguments);
-    if (__DEV__) {
-      ReactDebugCurrentFrame.element = newElement;
-    }
     for (var i = 2; i < arguments.length; i++) {
       validateChildKeys(arguments[i], newElement.type);
     }
     validatePropTypes(newElement);
-    if (__DEV__) {
-      ReactDebugCurrentFrame.element = null;
-    }
     return newElement;
   },
 };

src/isomorphic/hooks/ReactComponentTreeHook.js

@@ -13,17 +13,12 @@
 'use strict';
 
 var ReactCurrentOwner = require('ReactCurrentOwner');
-var {
-  getStackAddendumByWorkInProgressFiber,
-  describeComponentFrame,
-} = require('ReactFiberComponentTreeHook');
 var invariant = require('fbjs/lib/invariant');
 var warning = require('fbjs/lib/warning');
-var getComponentName = require('getComponentName');
+var describeComponentFrame = require('describeComponentFrame');
 
 import type {ReactElement, Source} from 'ReactElementType';
 import type {DebugID} from 'ReactInstanceType';
-import type {Fiber} from 'ReactFiber';
 
 function isNative(fn) {
   // Based on isNative() from Lodash
@@ -313,26 +308,15 @@ var ReactComponentTreeHook = {
     return item ? item.isMounted : false;
   },
 
-  getCurrentStackAddendum(topElement: ?ReactElement): string {
+  getCurrentStackAddendum(): string {
     var info = '';
-    if (topElement) {
-      var name = getDisplayName(topElement);
-      var owner = topElement._owner;
-      info += describeComponentFrame(
-        name,
-        topElement._source,
-        owner && getComponentName(owner),
-      );
-    }
-
     var currentOwner = ReactCurrentOwner.current;
     if (currentOwner) {
-      if (typeof currentOwner.tag === 'number') {
-        const workInProgress = ((currentOwner: any): Fiber);
-        // Safe because if current owner exists, we are reconciling,
-        // and it is guaranteed to be the work-in-progress version.
-        info += getStackAddendumByWorkInProgressFiber(workInProgress);
-      } else if (typeof currentOwner._debugID === 'number') {
+      invariant(
+        typeof currentOwner.tag !== 'number',
+        'Fiber owners should not show up in Stack stack traces.',
+      );
+      if (typeof currentOwner._debugID === 'number') {
         info += ReactComponentTreeHook.getStackAddendumByID(
           currentOwner._debugID,
         );

src/renderers/__tests__/ReactComponentTreeHook-test.js

@@ -20,6 +20,7 @@ describe('ReactComponentTreeHook', () => {
   var ReactDOMServer;
   var ReactInstanceMap;
   var ReactComponentTreeHook;
+  var ReactDebugCurrentFiber;
   var ReactComponentTreeTestUtils;
 
   beforeEach(() => {
@@ -29,6 +30,7 @@ describe('ReactComponentTreeHook', () => {
     ReactDOM = require('react-dom');
     ReactDOMServer = require('react-dom/server');
     ReactInstanceMap = require('ReactInstanceMap');
+    ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
     ReactComponentTreeHook = require('ReactComponentTreeHook');
     ReactComponentTreeTestUtils = require('ReactComponentTreeTestUtils');
   });
@@ -37,7 +39,9 @@ describe('ReactComponentTreeHook', () => {
   describe('stack addenda', () => {
     it('gets created', () => {
       function getAddendum(element) {
-        var addendum = ReactComponentTreeHook.getCurrentStackAddendum(element);
+        var addendum = ReactDOMFeatureFlags.useFiber
+          ? ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || ''
+          : ReactComponentTreeHook.getCurrentStackAddendum();
         return addendum.replace(/\(at .+?:\d+\)/g, '(at **)');
       }
 
@@ -47,23 +51,23 @@ describe('ReactComponentTreeHook', () => {
       Object.defineProperty(Anon, 'name', {
         value: null,
       });
-      function Orange() {
-        return null;
-      }
+      // function Orange() {
+      //   return null;
+      // }
 
       expectDev(getAddendum()).toBe('');
-      expectDev(getAddendum(<div />)).toBe('\n    in div (at **)');
-      expectDev(getAddendum(<Anon />)).toBe('\n    in Unknown (at **)');
-      expectDev(getAddendum(<Orange />)).toBe('\n    in Orange (at **)');
-      expectDev(getAddendum(React.createElement(Orange))).toBe(
-        '\n    in Orange',
-      );
+      // expectDev(getAddendum(<div />)).toBe('\n    in div (at **)');
+      // expectDev(getAddendum(<Anon />)).toBe('\n    in Unknown (at **)');
+      // expectDev(getAddendum(<Orange />)).toBe('\n    in Orange (at **)');
+      // expectDev(getAddendum(React.createElement(Orange))).toBe(
+      //   '\n    in Orange',
+      // );
 
       var renders = 0;
-      var rOwnedByQ;
+      //var rOwnedByQ;
 
       function Q() {
-        return (rOwnedByQ = React.createElement(R));
+        return /*rOwnedByQ =*/ React.createElement(R);
       }
       function R() {
         return <div><S /></div>;
@@ -82,15 +86,15 @@ describe('ReactComponentTreeHook', () => {
               '\n    in Q (at **)',
           );
           expectDev(getAddendum(<span />)).toBe(
-            '\n    in span (at **)' +
-              '\n    in S (at **)' +
+            // '\n    in span (at **)' +
+            '\n    in S (at **)' +
               '\n    in div (at **)' +
               '\n    in R (created by Q)' +
               '\n    in Q (at **)',
           );
           expectDev(getAddendum(React.createElement('span'))).toBe(
-            '\n    in span (created by S)' +
-              '\n    in S (at **)' +
+            // '\n    in span (created by S)' +
+            '\n    in S (at **)' +
               '\n    in div (at **)' +
               '\n    in R (created by Q)' +
               '\n    in Q (at **)',
@@ -103,7 +107,7 @@ describe('ReactComponentTreeHook', () => {
       expectDev(renders).toBe(2);
 
       // Make sure owner is fetched for the top element too.
-      expectDev(getAddendum(rOwnedByQ)).toBe('\n    in R (created by Q)');
+      // expectDev(getAddendum(rOwnedByQ)).toBe('\n    in R (created by Q)');
     });
 
     // These are features and regression tests that only affect