Add dev-only ReactDebugCurrentFiber for warnings

gaearon · gaearon · commit 6796336f3cf9 · 2016-12-14T22:12:36.000Z
The goal is to use ReactCurrentOwner less and rely on ReactDebugCurrentFiber for warning owner name and stack.
diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt
@@ -21,7 +21,6 @@ src/renderers/dom/shared/__tests__/ReactDOM-test.js
 
 src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
 * should warn for children on void elements
-* should report component containing invalid styles
 * should clean up input value tracking
 * should clean up input textarea tracking
 * gives source code refs for unknown prop warning
diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt
@@ -12,7 +12,6 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
 * should warn for unknown prop
 * should group multiple unknown prop warnings together
 * should warn for onDblClick prop
-* should emit a warning once for a named custom component using shady DOM
 * should not warn when server-side rendering `onScroll`
 * warns on invalid nesting
 * warns on invalid nesting at root
diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt
@@ -653,6 +653,8 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
 * should warn on upper case HTML tags, not SVG nor custom tags
 * should warn against children for void elements
 * should warn against dangerouslySetInnerHTML for void elements
+* should include owner rather than parent in warnings
+* should emit a warning once for a named custom component using shady DOM
 * should emit a warning once for an unnamed custom component using shady DOM
 * should treat menuitem as a void element but still create the closing tag
 * should validate against multiple children props
@@ -671,6 +673,7 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
 * should validate against multiple children props
 * should warn about contentEditable and children
 * should validate against invalid styles
+* should report component containing invalid styles
 * should properly escape text content and attributes values
 * unmounts children before unsetting DOM node info
 * should warn about the `onScroll` issue when unsupported (IE8)
diff --git a/src/isomorphic/hooks/ReactComponentTreeHook.js b/src/isomorphic/hooks/ReactComponentTreeHook.js
@@ -353,25 +353,17 @@ var ReactComponentTreeHook = {
     var currentOwner = ReactCurrentOwner.current;
     if (currentOwner) {
       if (typeof currentOwner.tag === 'number') {
-        const fiber = ((currentOwner : any) : Fiber);
-        info += ReactComponentTreeHook.getStackAddendumByFiber(fiber);
+        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 += ReactComponentTreeHook.getStackAddendumByWorkInProgressFiber(workInProgress);
       } else if (typeof currentOwner._debugID === 'number') {
         info += ReactComponentTreeHook.getStackAddendumByID(currentOwner._debugID);
       }
     }
     return info;
   },
 
-  getStackAddendumByFiber(fiber : Fiber) : string {
-    var info = '';
-    var node = fiber;
-    do {
-      info += describeFiber(node);
-      node = node.return;
-    } while (node);
-    return info;
-  },
-
   getStackAddendumByID(id: ?DebugID): string {
     var info = '';
     while (id) {
@@ -381,6 +373,20 @@ var ReactComponentTreeHook = {
     return info;
   },
 
+  // This function can only be called with a work-in-progress fiber and
+  // only during begin or complete phase. Do not call it under any other
+  // circumstances.
+  getStackAddendumByWorkInProgressFiber(workInProgress : Fiber) : string {
+    var info = '';
+    var node = workInProgress;
+    do {
+      info += describeFiber(node);
+      // Otherwise this return pointer might point to the wrong tree:
+      node = node.return;
+    } while (node);
+    return info;
+  },
+
   getChildIDs(id: DebugID): Array<DebugID> {
     var item = getItem(id);
     return item ? item.childIDs : [];
diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js
@@ -25,10 +25,10 @@ var ReactDOMFiberInput = require('ReactDOMFiberInput');
 var ReactDOMFiberOption = require('ReactDOMFiberOption');
 var ReactDOMFiberSelect = require('ReactDOMFiberSelect');
 var ReactDOMFiberTextarea = require('ReactDOMFiberTextarea');
+var { getCurrentFiberOwnerName } = require('ReactDebugCurrentFiber');
 
 var emptyFunction = require('emptyFunction');
 var focusNode = require('focusNode');
-var getCurrentOwnerName = require('getCurrentOwnerName');
 var invariant = require('invariant');
 var isEventSupported = require('isEventSupported');
 var setInnerHTML = require('setInnerHTML');
@@ -56,7 +56,7 @@ var DOC_FRAGMENT_TYPE = 11;
 
 
 function getDeclarationErrorAddendum() {
-  var ownerName = getCurrentOwnerName();
+  var ownerName = getCurrentFiberOwnerName();
   if (ownerName) {
     return ' This DOM node was rendered by `' + ownerName + '`.';
   }
@@ -436,7 +436,7 @@ function updateDOMProperties(
     CSSPropertyOperations.setValueForStyles(
       domElement,
       styleUpdates,
-      componentPlaceholder // TODO: Change CSSPropertyOperations to use getCurrentOwnerName.
+      componentPlaceholder // TODO: Change CSSPropertyOperations to use getCurrentFiberOwnerName.
     );
   }
 }
@@ -529,7 +529,7 @@ var ReactDOMFiberComponent = {
           false,
           '%s is using shady DOM. Using shady DOM with React can ' +
           'cause things to break subtly.',
-          getCurrentOwnerName() || 'A component'
+          getCurrentFiberOwnerName() || 'A component'
         );
         didWarnShadyDOM = true;
       }
diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js
@@ -23,8 +23,8 @@ type InputWithWrapperState = HTMLInputElement & {
 var DOMPropertyOperations = require('DOMPropertyOperations');
 var ReactControlledValuePropTypes = require('ReactControlledValuePropTypes');
 var ReactDOMComponentTree = require('ReactDOMComponentTree');
+var { getCurrentFiberOwnerName } = require('ReactDebugCurrentFiber');
 
-var getCurrentOwnerName = require('getCurrentOwnerName');
 var invariant = require('invariant');
 var warning = require('warning');
 
@@ -86,7 +86,7 @@ var ReactDOMInput = {
       ReactControlledValuePropTypes.checkPropTypes(
         'input',
         props,
-        getCurrentOwnerName()
+        getCurrentFiberOwnerName()
       );
 
       if (
@@ -102,7 +102,7 @@ var ReactDOMInput = {
           'both). Decide between using a controlled or uncontrolled input ' +
           'element and remove one of these props. More info: ' +
           'https://fb.me/react-controlled-components',
-          getCurrentOwnerName() || 'A component',
+          getCurrentFiberOwnerName() || 'A component',
           props.type
         );
         didWarnCheckedDefaultChecked = true;
@@ -120,7 +120,7 @@ var ReactDOMInput = {
           'both). Decide between using a controlled or uncontrolled input ' +
           'element and remove one of these props. More info: ' +
           'https://fb.me/react-controlled-components',
-          getCurrentOwnerName() || 'A component',
+          getCurrentFiberOwnerName() || 'A component',
           props.type
         );
         didWarnValueDefaultValue = true;
@@ -151,7 +151,7 @@ var ReactDOMInput = {
           'Input elements should not switch from uncontrolled to controlled (or vice versa). ' +
           'Decide between using a controlled or uncontrolled input ' +
           'element for the lifetime of the component. More info: https://fb.me/react-controlled-components',
-          getCurrentOwnerName() || 'A component',
+          getCurrentFiberOwnerName() || 'A component',
           props.type
         );
         didWarnUncontrolledToControlled = true;
@@ -163,7 +163,7 @@ var ReactDOMInput = {
           'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
           'Decide between using a controlled or uncontrolled input ' +
           'element for the lifetime of the component. More info: https://fb.me/react-controlled-components',
-          getCurrentOwnerName() || 'A component',
+          getCurrentFiberOwnerName() || 'A component',
           props.type
         );
         didWarnControlledToUncontrolled = true;
diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js
@@ -20,14 +20,13 @@ type SelectWithWrapperState = HTMLSelectElement & {
 };
 
 var ReactControlledValuePropTypes = require('ReactControlledValuePropTypes');
-
-var getCurrentOwnerName = require('getCurrentOwnerName');
+var { getCurrentFiberOwnerName } = require('ReactDebugCurrentFiber');
 var warning = require('warning');
 
 var didWarnValueDefaultValue = false;
 
 function getDeclarationErrorAddendum() {
-  var ownerName = getCurrentOwnerName();
+  var ownerName = getCurrentFiberOwnerName();
   if (ownerName) {
     return ' Check the render method of `' + ownerName + '`.';
   }
@@ -43,7 +42,7 @@ function checkSelectPropTypes(props) {
   ReactControlledValuePropTypes.checkPropTypes(
     'select',
     props,
-    getCurrentOwnerName()
+    getCurrentFiberOwnerName()
   );
 
   for (var i = 0; i < valuePropNames.length; i++) {
diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberTextarea.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberTextarea.js
@@ -19,8 +19,8 @@ type TextAreaWithWrapperState = HTMLTextAreaElement & {
 };
 
 var ReactControlledValuePropTypes = require('ReactControlledValuePropTypes');
+var { getCurrentFiberOwnerName } = require('ReactDebugCurrentFiber');
 
-var getCurrentOwnerName = require('getCurrentOwnerName');
 var invariant = require('invariant');
 var warning = require('warning');
 
@@ -69,7 +69,7 @@ var ReactDOMTextarea = {
       ReactControlledValuePropTypes.checkPropTypes(
         'textarea',
         props,
-        getCurrentOwnerName()
+        getCurrentFiberOwnerName()
       );
       if (
         props.value !== undefined &&
diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
@@ -848,6 +848,33 @@ describe('ReactDOMComponent', () => {
       );
     });
 
+    it('should include owner rather than parent in warnings', () => {
+      var container = document.createElement('div');
+
+      function Parent(props) {
+        return props.children;
+      }
+      function Owner() {
+        // We're using the input dangerouslySetInnerHTML invariant but the
+        // exact error doesn't matter as long as we have a way to verify
+        // that warnings and invariants contain owner rather than parent name.
+        return (
+          <Parent>
+            <input dangerouslySetInnerHTML={{__html: 'content'}} />
+          </Parent>
+        );
+      }
+
+      expect(function() {
+        ReactDOM.render(
+          <Owner />,
+          container
+        );
+      }).toThrowError(
+        'This DOM node was rendered by `Owner`.'
+      );
+    });
+
     it('should emit a warning once for a named custom component using shady DOM', () => {
       if (ReactDOMFeatureFlags.useCreateElement) {
         spyOn(console, 'error');
diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js
@@ -38,8 +38,7 @@ var getIteratorFn = require('getIteratorFn');
 var invariant = require('invariant');
 
 if (__DEV__) {
-  var ReactComponentTreeHook = require('ReactComponentTreeHook');
-  var { getStackAddendumByFiber } = ReactComponentTreeHook;
+  var { getCurrentFiberStackAddendum } = require('ReactDebugCurrentFiber');
   var warning = require('warning');
 }
 
@@ -549,7 +548,6 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
 
   function warnOnDuplicateKey(
     child : mixed,
-    returnFiber : Fiber,
     knownKeys : Set<string> | null
   ) : Set<string> | null {
     if (__DEV__) {
@@ -580,7 +578,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
             '`%s`. Child keys must be unique; when two children share a key, ' +
             'only the first child will be used.%s',
             key,
-            getStackAddendumByFiber(returnFiber)
+            getCurrentFiberStackAddendum()
           );
           break;
         default:
@@ -620,7 +618,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
       let knownKeys = null;
       for (let i = 0; i < newChildren.length; i++) {
         const child = newChildren[i];
-        knownKeys = warnOnDuplicateKey(child, returnFiber, knownKeys);
+        knownKeys = warnOnDuplicateKey(child, knownKeys);
       }
     }
 
@@ -776,7 +774,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
       let step = newChildren.next();
       for (; !step.done; step = newChildren.next()) {
         const child = step.value;
-        knownKeys = warnOnDuplicateKey(child, returnFiber, knownKeys);
+        knownKeys = warnOnDuplicateKey(child, knownKeys);
       }
     }
 
diff --git a/src/renderers/shared/fiber/ReactDebugCurrentFiber.js b/src/renderers/shared/fiber/ReactDebugCurrentFiber.js
@@ -0,0 +1,55 @@
+/**
+ * Copyright 2013-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *
+ * @providesModule ReactDebugCurrentFiber
+ * @flow
+ */
+
+'use strict';
+
+import type { Fiber } from 'ReactFiber';
+
+if (__DEV__) {
+  var getComponentName = require('getComponentName');
+  var { getStackAddendumByWorkInProgressFiber } = require('ReactComponentTreeHook');
+}
+
+function getCurrentFiberOwnerName() : string | null {
+  if (__DEV__) {
+    const fiber = ReactDebugCurrentFiber.current;
+    if (fiber == null) {
+      return null;
+    }
+    if (fiber._debugOwner == null) {
+      return null;
+    }
+    return getComponentName(fiber._debugOwner);
+  }
+  return null;
+}
+
+function getCurrentFiberStackAddendum() : string | null {
+  if (__DEV__) {
+    const fiber = ReactDebugCurrentFiber.current;
+    if (fiber == null) {
+      return null;
+    }
+    // Safe because if current fiber exists, we are reconciling,
+    // and it is guaranteed to be the work-in-progress version.
+    return getStackAddendumByWorkInProgressFiber(fiber);
+  }
+  return null;
+}
+
+var ReactDebugCurrentFiber = {
+  current: (null : Fiber | null),
+  getCurrentFiberOwnerName,
+  getCurrentFiberStackAddendum,
+};
+
+module.exports = ReactDebugCurrentFiber;
diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js
@@ -60,6 +60,10 @@ var {
 var ReactCurrentOwner = require('ReactCurrentOwner');
 var ReactFiberClassComponent = require('ReactFiberClassComponent');
 
+if (__DEV__) {
+  var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
+}
+
 module.exports = function<T, P, I, TI, C, CX>(
   config : HostConfig<T, P, I, TI, C, CX>,
   hostContext : HostContext<C, CX>,
@@ -452,6 +456,10 @@ module.exports = function<T, P, I, TI, C, CX>(
       return bailoutOnLowPriority(current, workInProgress);
     }
 
+    if (__DEV__) {
+      ReactDebugCurrentFiber.current = workInProgress;
+    }
+
     // If we don't bail out, we're going be recomputing our children so we need
     // to drop our effect list.
     workInProgress.firstEffect = null;
diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js
diff --git a/src/renderers/shared/fiber/ReactFiberContext.js b/src/renderers/shared/fiber/ReactFiberContext.js
diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js
diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js
diff --git a/src/renderers/shared/fiber/getCurrentOwnerName.js b/src/renderers/shared/fiber/getCurrentOwnerName.js
diff --git a/src/shared/types/checkReactTypeSpec.js b/src/shared/types/checkReactTypeSpec.js
