Skip to content

Commit

Permalink
Move replay to outer catch block
Browse files Browse the repository at this point in the history
This moves it out of the hot path.
  • Loading branch information
acdlite committed Feb 24, 2018
1 parent a4d53d2 commit ddac776
Show file tree
Hide file tree
Showing 12 changed files with 58 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('ReactErrorBoundaries', () => {
jest.resetModules();
PropTypes = require('prop-types');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.replayFailedBeginPhaseWithInvokeGuardedCallback = false;
ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
ReactDOM = require('react-dom');
React = require('react');

Expand Down
125 changes: 47 additions & 78 deletions packages/react-reconciler/src/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
import {
enableUserTimingAPI,
warnAboutDeprecatedLifecycles,
replayFailedBeginPhaseWithInvokeGuardedCallback,
replayFailedUnitOfWorkWithInvokeGuardedCallback,
} from 'shared/ReactFeatureFlags';
import getComponentName from 'shared/getComponentName';
import invariant from 'fbjs/lib/invariant';
Expand Down Expand Up @@ -242,6 +242,42 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
// Used for performance tracking.
let interruptedBy: Fiber | null = null;

let stashedWorkInProgressProperties;
let replayUnitOfWork;
if (__DEV__) {
stashedWorkInProgressProperties = null;
replayUnitOfWork = (failedUnitOfWork: Fiber, isAsync: boolean) => {
// Retore the original state of the work-in-progress
Object.assign(failedUnitOfWork, stashedWorkInProgressProperties);
switch (failedUnitOfWork.tag) {
case HostRoot:
popHostContainer(failedUnitOfWork);
popTopLevelLegacyContextObject(failedUnitOfWork);
break;
case HostComponent:
popHostContext(failedUnitOfWork);
break;
case ClassComponent:
popLegacyContextProvider(failedUnitOfWork);
break;
case HostPortal:
popHostContainer(failedUnitOfWork);
break;
case ContextProvider:
popProvider(failedUnitOfWork);
break;
}
// Replay the begin phase.
invokeGuardedCallback(null, workLoop, null, isAsync);
if (hasCaughtError()) {
clearCaughtError();
} else {
// This should be unreachable because the render phase is
// idempotent
}
};
}

function resetContextStack() {
// Reset the stack
reset();
Expand Down Expand Up @@ -725,78 +761,6 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
return null;
}

let invokeBeginWork = beginWork;
if (__DEV__ && replayFailedBeginPhaseWithInvokeGuardedCallback) {
invokeBeginWork = function beginWorkInDEV(
current,
workInProgress,
expirationTime,
) {
const stashedWorkInProgressProperties = Object.assign({}, workInProgress);
try {
return beginWork(current, workInProgress, expirationTime);
} catch (value) {
let thrownValue = value;
// Check if the thrown value is an error. If it is, we will replay the
// begin phase using invokeGuardedCallback, so that the error is not
// treated as caught by the debugger.
if (
value instanceof Error ||
(typeof value === 'object' &&
value !== null &&
typeof value.message === 'string' &&
typeof value.stack === 'string')
) {
// Retore the original state of the work-in-progress
Object.assign(workInProgress, stashedWorkInProgressProperties);

switch (workInProgress.tag) {
case HostRoot:
popHostContainer(workInProgress);
popTopLevelLegacyContextObject(workInProgress);
break;
case HostComponent:
popHostContext(workInProgress);
break;
case ClassComponent:
popLegacyContextProvider(workInProgress);
break;
case HostPortal:
popHostContainer(workInProgress);
break;
case ContextProvider:
popProvider(workInProgress);
break;
}

// Replay the begin phase.
invokeGuardedCallback(
null,
beginWork,
null,
current,
workInProgress,
expirationTime,
);
if (hasCaughtError()) {
thrownValue = clearCaughtError();
} else {
// This should be unreachable because the render phase is
// idempotent, but even we don't throw the second time, we should
// still rethrow the original thrown value
}
} else {
// If it's not an error, then it's a promise or other algebraic
// effect. We don't want the debugger to treat these as uncaught, so
// we should rethrow it without replaying.
}
// Rethrow to unwind the stack. This maintains the same control flow
// in development and production.
throw thrownValue;
}
};
}

function performUnitOfWork(workInProgress: Fiber): Fiber | null {
// The current, flushed, state of this fiber is the alternate.
// Ideally nothing should rely on this, but relying on it here
Expand All @@ -810,11 +774,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
ReactDebugCurrentFiber.setCurrentFiber(workInProgress);
}

let next = invokeBeginWork(
current,
workInProgress,
nextRenderExpirationTime,
);
if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
stashedWorkInProgressProperties = Object.assign({}, workInProgress);
}
let next = beginWork(current, workInProgress, nextRenderExpirationTime);

if (__DEV__) {
ReactDebugCurrentFiber.resetCurrentFiber();
Expand Down Expand Up @@ -892,6 +855,12 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
onUncaughtError(thrownValue);
break;
}

if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
const failedUnitOfWork = nextUnitOfWork;
replayUnitOfWork(failedUnitOfWork, isAsync);
}

const sourceFiber: Fiber = nextUnitOfWork;
let returnFiber = sourceFiber.return;
if (returnFiber === null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('ReactIncrementalErrorHandling', () => {
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.enableGetDerivedStateFromCatch = true;
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
ReactFeatureFlags.replayFailedBeginPhaseWithInvokeGuardedCallback = false;
ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
PropTypes = require('prop-types');
React = require('react');
ReactNoop = require('react-noop-renderer');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('ReactIncrementalErrorLogging', () => {
jest.resetModules();
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
ReactFeatureFlags.replayFailedBeginPhaseWithInvokeGuardedCallback = false;
ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
React = require('react');
ReactNoop = require('react-noop-renderer');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ describe('ReactDebugFiberPerf', () => {
global.performance = createUserTimingPolyfill();

require('shared/ReactFeatureFlags').enableUserTimingAPI = true;
require('shared/ReactFeatureFlags').replayFailedUnitOfWorkWithInvokeGuardedCallback = false;

// Import after the polyfill is set up:
React = require('react');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
'use strict';

const ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.replayFailedBeginPhaseWithInvokeGuardedCallback = false;
ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
const React = require('react');
const ReactTestRenderer = require('react-test-renderer');
const prettyFormat = require('pretty-format');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('ReactElementValidator', () => {

PropTypes = require('prop-types');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.replayFailedBeginPhaseWithInvokeGuardedCallback = false;
ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
React = require('react');
ReactDOM = require('react-dom');
ReactTestUtils = require('react-dom/test-utils');
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const debugRenderPhaseSideEffectsForStrictMode = __DEV__;

// To preserve the "Pause on caught exceptions" behavior of the debugger, we
// replay the begin phase of a failed component inside invokeGuardedCallback.
export const replayFailedBeginPhaseWithInvokeGuardedCallback = __DEV__;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;

// Warn about deprecated, async-unsafe lifecycles; relates to RFC #6:
export const warnAboutDeprecatedLifecycles = false;
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const enableCreateRoot = false;
export const enableUserTimingAPI = __DEV__;
export const enableGetDerivedStateFromCatch = false;
export const warnAboutDeprecatedLifecycles = false;
export const replayFailedBeginPhaseWithInvokeGuardedCallback = __DEV__;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;

// React Fabric uses persistent reconciler.
export const enableMutatingReconciler = false;
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const {
debugRenderPhaseSideEffects,
debugRenderPhaseSideEffectsForStrictMode,
warnAboutDeprecatedLifecycles,
replayFailedBeginPhaseWithInvokeGuardedCallback,
replayFailedUnitOfWorkWithInvokeGuardedCallback,
} = require('ReactFeatureFlags');

// The rest of the flags are static for better dead code elimination.
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.persistent.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const enableCreateRoot = false;
export const enableUserTimingAPI = __DEV__;
export const enableGetDerivedStateFromCatch = false;
export const warnAboutDeprecatedLifecycles = false;
export const replayFailedBeginPhaseWithInvokeGuardedCallback = __DEV__;
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;

// react-reconciler/persistent entry point
// uses a persistent reconciler.
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const {
debugRenderPhaseSideEffects,
debugRenderPhaseSideEffectsForStrictMode,
warnAboutDeprecatedLifecycles,
replayFailedBeginPhaseWithInvokeGuardedCallback,
replayFailedUnitOfWorkWithInvokeGuardedCallback,
} = require('ReactFeatureFlags');

// The rest of the flags are static for better dead code elimination.
Expand Down

0 comments on commit ddac776

Please sign in to comment.