Skip to content

Commit

Permalink
Don't reset the current fiber during reconciliation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gaearon committed Sep 28, 2017
1 parent cfbb004 commit 4f5a5f8
Showing 1 changed file with 25 additions and 4 deletions.
29 changes: 25 additions & 4 deletions src/renderers/shared/fiber/ReactFiberContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,20 @@ exports.getMaskedContext = function(

if (__DEV__) {
const name = getComponentName(workInProgress) || 'Unknown';
ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null);
if (workInProgress !== ReactDebugCurrentFiber.current) {
warning(
false,
'Expected the work in progress to match the currently processed fiber. ' +
'This error is likely caused by a bug in React. Please file an issue.',
);
}
checkPropTypes(
contextTypes,
context,
'context',
name,
ReactDebugCurrentFiber.getCurrentFiberStackAddendum,
);
ReactDebugCurrentFiber.resetCurrentFiber();
}

// Cache unmasked context so we can avoid recreating masked context unless necessary.
Expand Down Expand Up @@ -184,11 +189,19 @@ function processChildContext(

let childContext;
if (__DEV__) {
// TODO: we only have to store the "previous" fiber and phase and restore them
// because this method can be called outside of reconciliation. We can remove this
// when we stop supporting unstable_renderSubtreeIntoContainer.
const previousCurrentFiber = ReactDebugCurrentFiber.current;
const previousCurrentPhase = ReactDebugCurrentFiber.phase;
ReactDebugCurrentFiber.setCurrentFiber(fiber, 'getChildContext');
startPhaseTimer(fiber, 'getChildContext');
childContext = instance.getChildContext();
stopPhaseTimer();
ReactDebugCurrentFiber.resetCurrentFiber();
ReactDebugCurrentFiber.setCurrentFiber(
previousCurrentFiber,
previousCurrentPhase,
);
} else {
childContext = instance.getChildContext();
}
Expand All @@ -208,6 +221,11 @@ function processChildContext(
// 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;
// TODO: we only have to store the "previous" fiber and phase and restore them
// because this method can be called outside of reconciliation. We can remove this
// when we stop supporting unstable_renderSubtreeIntoContainer.
const previousCurrentFiber = ReactDebugCurrentFiber.current;
const previousCurrentPhase = ReactDebugCurrentFiber.phase;
ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null);
checkPropTypes(
childContextTypes,
Expand All @@ -216,7 +234,10 @@ function processChildContext(
name,
ReactDebugCurrentFiber.getCurrentFiberStackAddendum,
);
ReactDebugCurrentFiber.resetCurrentFiber();
ReactDebugCurrentFiber.setCurrentFiber(
previousCurrentFiber,
previousCurrentPhase,
);
}

return {...parentContext, ...childContext};
Expand Down

0 comments on commit 4f5a5f8

Please sign in to comment.