Refactor Debug Frames to Enable Renderers to Provide Custom Logic (#10105)

* Extract the top element frame from ReactDebugCurrentFrame

This is part of a larger refactor to decouple stack addendums. All
renderers have their own way of getting the stack of the currently
executing components.

There is one special case in Element Validator that adds an additional line
for the element being validated. This commit moves that special case in
into the validator.

There is another case where it looked like this was used in shallow
renderer but this is actually something different. It is part of the
component stack. It just happens to be that shallow renderer has a simpler
implementation of the component stack that just happens to be a single
element.

This will let us decouple the implementation to get a stack from
ReactDebugCurrentFrame and put that in each renderer.

* Stop using ReactComponentTreeHook for Fiber

Currently we fall back to ReactCurrentOwner in ReactComponentTreeHook for
stack addendums. We shouldn't need to because we should use
ReactDebugCurrrentFiber.

Ensure we always set both ReactDebugCurrentFiber and ReactDebugCurrentFrame
so that we can rely on these for all stacks.

* Make ReactDebugCurrentFrame implementation independent

Introduced ReactDebugCurrentStack for the Stack renderer which does the
same thing as ReactDebugCurrentFiber.

ReactDebugCurrentFrame no longer keeps track of the current fiber/debug id.
That's handled by the individual renderers.

Instead, it is now used to keep track of the current *implementation* of
the current stack frame. That way it is decoupled from the specifics of
the renderers. There can be multiple renderers in a context. What matters
is which one is currently executing a debuggable context (such as a render
function).

* Add debug frames to ReactPartialRenderer (ssr)

Basic functionality.

* Add shared modules to shallow renderer

This is now needed because we share describeComponentFrame.

Author: sebmarkbage

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