Fix "null" instead of the component stack in a warning (#10915)

* Add a failing test that prints "null" in the warning instead of the stack

* Don't reset the current fiber during reconciliation

It should only be reset when we stop doing work.
Otherwise, any warnings after the reset will lose the component stack.

The reset in getMaskedContext() was completely unnecessary.
It is always called with the same fiber as the current work in progress.
Therefore, I add a DEV-only warning assertion to ensure we don't regress, and remove the reset.

The reset in processChildContext() is necessary because it can be called outside of reconciliation.
Unfortunately, we have to keep this hack in until we can remove unstable_renderSubtreeIntoContainer().
To work around it, I restore the previous fiber instead of resetting.

* Decouple setting the current fiber and phase

These are two distinct actions.
This helps make it clearer when we're actually changing the current pointer.

I'm also removing an overengineered hack I previously added for unstable_renderSubtreeIntoContainer. It's not necessary now that we don't null the pointer all the time.
This makes the code more straightforward.

* Centralize the pointer updates in the scheduler

Author: gaearon

src/renderers/__tests__/ReactStatelessComponent-test.js

@@ -295,6 +295,43 @@ describe('ReactStatelessComponent', () => {
     console.error.calls.reset();
   });
 
+  // This guards against a regression caused by clearing the current debug fiber.
+  // https://github.com/facebook/react/issues/10831
+  it('should warn when giving a function ref with context', () => {
+    spyOn(console, 'error');
+
+    function Child() {
+      return null;
+    }
+    Child.contextTypes = {
+      foo: PropTypes.string,
+    };
+
+    class Parent extends React.Component {
+      static childContextTypes = {
+        foo: PropTypes.string,
+      };
+      getChildContext() {
+        return {
+          foo: 'bar',
+        };
+      }
+      render() {
+        return <Child ref={function() {}} />;
+      }
+    }
+
+    ReactTestUtils.renderIntoDocument(<Parent />);
+    expectDev(console.error.calls.count()).toBe(1);
+    expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
+      'Warning: Stateless function components cannot be given refs. ' +
+        'Attempts to access this ref will fail.\n\nCheck the render method ' +
+        'of `Parent`.\n' +
+        '    in Child (at **)\n' +
+        '    in Parent (at **)',
+    );
+  });
+
   it('should provide a null ref', () => {
     function Child() {
       return <div />;

src/renderers/shared/fiber/ReactDebugCurrentFiber.js

@@ -55,9 +55,13 @@ function resetCurrentFiber() {
   ReactDebugCurrentFiber.phase = null;
 }
 
-function setCurrentFiber(fiber: Fiber | null, phase: LifeCyclePhase | null) {
+function setCurrentFiber(fiber: Fiber) {
   ReactDebugCurrentFrame.getCurrentStack = getCurrentFiberStackAddendum;
   ReactDebugCurrentFiber.current = fiber;
+  ReactDebugCurrentFiber.phase = null;
+}
+
+function setCurrentPhase(phase: LifeCyclePhase | null) {
   ReactDebugCurrentFiber.phase = phase;
 }
 
@@ -66,6 +70,7 @@ var ReactDebugCurrentFiber = {
   phase: (null: LifeCyclePhase | null),
   resetCurrentFiber,
   setCurrentFiber,
+  setCurrentPhase,
   getCurrentFiberOwnerName,
   getCurrentFiberStackAddendum,
 };

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -207,9 +207,9 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
 
     if (__DEV__) {
       ReactCurrentOwner.current = workInProgress;
-      ReactDebugCurrentFiber.setCurrentFiber(workInProgress, 'render');
+      ReactDebugCurrentFiber.setCurrentPhase('render');
       nextChildren = fn(nextProps, context);
-      ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null);
+      ReactDebugCurrentFiber.setCurrentPhase(null);
     } else {
       nextChildren = fn(nextProps, context);
     }
@@ -281,9 +281,9 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
     ReactCurrentOwner.current = workInProgress;
     let nextChildren;
     if (__DEV__) {
-      ReactDebugCurrentFiber.setCurrentFiber(workInProgress, 'render');
+      ReactDebugCurrentFiber.setCurrentPhase('render');
       nextChildren = instance.render();
-      ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null);
+      ReactDebugCurrentFiber.setCurrentPhase(null);
     } else {
       nextChildren = instance.render();
     }
@@ -725,10 +725,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
       return bailoutOnLowPriority(current, workInProgress);
     }
 
-    if (__DEV__) {
-      ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null);
-    }
-
     switch (workInProgress.tag) {
       case IndeterminateComponent:
         return mountIndeterminateComponent(

src/renderers/shared/fiber/ReactFiberCompleteWork.js

@@ -42,10 +42,6 @@ var {
 var {Placement, Ref, Update} = ReactTypeOfSideEffect;
 var {OffscreenPriority} = ReactPriorityLevel;
 
-if (__DEV__) {
-  var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
-}
-
 var invariant = require('fbjs/lib/invariant');
 
 module.exports = function<T, P, I, TI, PI, C, CX, PL>(
@@ -187,10 +183,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
     workInProgress: Fiber,
     renderPriority: PriorityLevel,
   ): Fiber | null {
-    if (__DEV__) {
-      ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null);
-    }
-
     // Get the latest props.
     let newProps = workInProgress.pendingProps;
     if (newProps === null) {

src/renderers/shared/fiber/ReactFiberContext.js

@@ -89,15 +89,13 @@ exports.getMaskedContext = function(
 
   if (__DEV__) {
     const name = getComponentName(workInProgress) || 'Unknown';
-    ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null);
     checkPropTypes(
       contextTypes,
       context,
       'context',
       name,
       ReactDebugCurrentFiber.getCurrentFiberStackAddendum,
     );
-    ReactDebugCurrentFiber.resetCurrentFiber();
   }
 
   // Cache unmasked context so we can avoid recreating masked context unless necessary.
@@ -153,11 +151,7 @@ exports.pushTopLevelContextObject = function(
   push(didPerformWorkStackCursor, didChange, fiber);
 };
 
-function processChildContext(
-  fiber: Fiber,
-  parentContext: Object,
-  isReconciling: boolean,
-): Object {
+function processChildContext(fiber: Fiber, parentContext: Object): Object {
   const instance = fiber.stateNode;
   const childContextTypes = fiber.type.childContextTypes;
 
@@ -184,11 +178,11 @@ function processChildContext(
 
   let childContext;
   if (__DEV__) {
-    ReactDebugCurrentFiber.setCurrentFiber(fiber, 'getChildContext');
+    ReactDebugCurrentFiber.setCurrentPhase('getChildContext');
     startPhaseTimer(fiber, 'getChildContext');
     childContext = instance.getChildContext();
     stopPhaseTimer();
-    ReactDebugCurrentFiber.resetCurrentFiber();
+    ReactDebugCurrentFiber.setCurrentPhase(null);
   } else {
     childContext = instance.getChildContext();
   }
@@ -202,21 +196,18 @@ function processChildContext(
   }
   if (__DEV__) {
     const name = getComponentName(fiber) || 'Unknown';
-    // We can only provide accurate element stacks if we pass work-in-progress tree
-    // during the begin or complete phase. However currently this function is also
-    // called from unstable_renderSubtree legacy implementation. In this case it unsafe to
-    // assume anything about the given fiber. We won't pass it down if we aren't sure.
-    // TODO: remove this hack when we delete unstable_renderSubtree in Fiber.
-    const workInProgress = isReconciling ? fiber : null;
-    ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null);
     checkPropTypes(
       childContextTypes,
       childContext,
       'child context',
       name,
+      // In practice, there is one case in which we won't get a stack. It's when
+      // somebody calls unstable_renderSubtreeIntoContainer() and we process
+      // context from the parent component instance. The stack will be missing
+      // because it's outside of the reconciliation, and so the pointer has not
+      // been set. This is rare and doesn't matter. We'll also remove that API.
       ReactDebugCurrentFiber.getCurrentFiberStackAddendum,
     );
-    ReactDebugCurrentFiber.resetCurrentFiber();
   }
 
   return {...parentContext, ...childContext};
@@ -264,11 +255,7 @@ exports.invalidateContextProvider = function(
     // Merge parent and own context.
     // Skip this if we're not updating due to sCU.
     // This avoids unnecessarily recomputing memoized values.
-    const mergedContext = processChildContext(
-      workInProgress,
-      previousContext,
-      true,
-    );
+    const mergedContext = processChildContext(workInProgress, previousContext);
     instance.__reactInternalMemoizedMergedChildContext = mergedContext;
 
     // Replace the old (or empty) context with the new one.

src/renderers/shared/fiber/ReactFiberReconciler.js

@@ -181,7 +181,7 @@ export type Reconciler<C, I, TI> = {
 getContextForSubtree._injectFiber(function(fiber: Fiber) {
   const parentContext = findCurrentUnmaskedContext(fiber);
   return isContextProvider(fiber)
-    ? processChildContext(fiber, parentContext, false)
+    ? processChildContext(fiber, parentContext)
     : parentContext;
 });
 

src/renderers/shared/fiber/ReactFiberScheduler.js

@@ -317,7 +317,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
   function commitAllHostEffects() {
     while (nextEffect !== null) {
       if (__DEV__) {
-        ReactDebugCurrentFiber.setCurrentFiber(nextEffect, null);
+        ReactDebugCurrentFiber.setCurrentFiber(nextEffect);
         recordEffect();
       }
 
@@ -615,7 +615,13 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
       // means that we don't need an additional field on the work in
       // progress.
       const current = workInProgress.alternate;
+      if (__DEV__) {
+        ReactDebugCurrentFiber.setCurrentFiber(workInProgress);
+      }
       const next = completeWork(current, workInProgress, nextPriorityLevel);
+      if (__DEV__) {
+        ReactDebugCurrentFiber.resetCurrentFiber();
+      }
 
       const returnFiber = workInProgress.return;
       const siblingFiber = workInProgress.sibling;
@@ -706,8 +712,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
     // See if beginning this work spawns more work.
     if (__DEV__) {
       startWorkTimer(workInProgress);
+      ReactDebugCurrentFiber.setCurrentFiber(workInProgress);
     }
     let next = beginWork(current, workInProgress, nextPriorityLevel);
+    if (__DEV__) {
+      ReactDebugCurrentFiber.resetCurrentFiber();
+    }
     if (__DEV__ && ReactFiberInstrumentation.debugTool) {
       ReactFiberInstrumentation.debugTool.onBeginWork(workInProgress);
     }
@@ -718,9 +728,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
     }
 
     ReactCurrentOwner.current = null;
-    if (__DEV__) {
-      ReactDebugCurrentFiber.resetCurrentFiber();
-    }
 
     return next;
   }
@@ -735,8 +742,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
     // See if beginning this work spawns more work.
     if (__DEV__) {
       startWorkTimer(workInProgress);
+      ReactDebugCurrentFiber.setCurrentFiber(workInProgress);
     }
     let next = beginFailedWork(current, workInProgress, nextPriorityLevel);
+    if (__DEV__) {
+      ReactDebugCurrentFiber.resetCurrentFiber();
+    }
     if (__DEV__ && ReactFiberInstrumentation.debugTool) {
       ReactFiberInstrumentation.debugTool.onBeginWork(workInProgress);
     }
@@ -747,9 +758,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
     }
 
     ReactCurrentOwner.current = null;
-    if (__DEV__) {
-      ReactDebugCurrentFiber.resetCurrentFiber();
-    }
 
     return next;
   }