Skip to content

[WIP] Refactor beginWork replay #14130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+42 −49
Closed

[WIP] Refactor beginWork replay #14130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 42 additions & 48 deletions packages/react-reconciler/src/ReactFiberScheduler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,19 +275,17 @@ let interruptedBy: Fiber | null = null;

let stashedWorkInProgressProperties;
let replayUnitOfWork;
let mayReplayFailedUnitOfWork;
let isReplayingFailedUnitOfWork;
let originalReplayError;
let rethrowOriginalError;
if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
stashedWorkInProgressProperties = null;
mayReplayFailedUnitOfWork = true;
isReplayingFailedUnitOfWork = false;
originalReplayError = null;
replayUnitOfWork = (
failedUnitOfWork: Fiber,
thrownValue: mixed,
isYieldy: boolean,
current, workInProgress, renderExpirationTime
) => {
if (
thrownValue !== null &&
Expand Down Expand Up @@ -340,7 +338,7 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
// Replay the begin phase.
isReplayingFailedUnitOfWork = true;
originalReplayError = thrownValue;
invokeGuardedCallback(null, workLoop, null, isYieldy);
invokeGuardedCallback(null, beginWork, current, workInProgress, renderExpirationTime);
isReplayingFailedUnitOfWork = false;
originalReplayError = null;
if (hasCaughtError()) {
Expand Down Expand Up @@ -954,10 +952,6 @@ function completeUnitOfWork(workInProgress: Fiber): Fiber | null {
const siblingFiber = workInProgress.sibling;

if ((workInProgress.effectTag & Incomplete) === NoEffect) {
if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
// Don't replay if it fails during completion phase.
mayReplayFailedUnitOfWork = false;
}
// This fiber completed.
// Remember we're completing this unit so we can find a boundary if it fails.
nextUnitOfWork = workInProgress;
Expand All @@ -981,10 +975,6 @@ function completeUnitOfWork(workInProgress: Fiber): Fiber | null {
nextRenderExpirationTime,
);
}
if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
// We're out of completion phase so replaying is fine now.
mayReplayFailedUnitOfWork = true;
}
stopWorkTimer(workInProgress);
resetChildExpirationTime(workInProgress, nextRenderExpirationTime);
if (__DEV__) {
Expand Down Expand Up @@ -1127,7 +1117,7 @@ function completeUnitOfWork(workInProgress: Fiber): Fiber | null {
return null;
}

function performUnitOfWork(workInProgress: Fiber): Fiber | null {
function performUnitOfWork(workInProgress: Fiber, isYieldy: boolean): Fiber | null {
// The current, flushed, state of this fiber is the alternate.
// Ideally nothing should rely on this, but relying on it here
// means that we don't need an additional field on the work in
Expand All @@ -1153,15 +1143,23 @@ function performUnitOfWork(workInProgress: Fiber): Fiber | null {
startProfilerTimer(workInProgress);
}

next = beginWork(current, workInProgress, nextRenderExpirationTime);
if (__DEV__) {
next = beginWorkInDEV(current, workInProgress, nextRenderExpirationTime, isYieldy);
} else {
next = beginWork(current, workInProgress, nextRenderExpirationTime);
}
workInProgress.memoizedProps = workInProgress.pendingProps;

if (workInProgress.mode & ProfileMode) {
// Record the render duration assuming we didn't bailout (or error).
stopProfilerTimerIfRunningAndRecordDelta(workInProgress, true);
}
} else {
next = beginWork(current, workInProgress, nextRenderExpirationTime);
if (__DEV__) {
next = beginWorkInDEV(current, workInProgress, nextRenderExpirationTime, isYieldy);
} else {
next = beginWork(current, workInProgress, nextRenderExpirationTime);
}
workInProgress.memoizedProps = workInProgress.pendingProps;
}

Expand Down Expand Up @@ -1193,13 +1191,40 @@ function workLoop(isYieldy) {
if (!isYieldy) {
// Flush work without yielding
while (nextUnitOfWork !== null) {
nextUnitOfWork = performUnitOfWork(nextUnitOfWork);
nextUnitOfWork = performUnitOfWork(nextUnitOfWork, isYieldy);
}
} else {
// Flush asynchronous work until there's a higher priority event
while (nextUnitOfWork !== null && !shouldYieldToRenderer()) {
nextUnitOfWork = performUnitOfWork(nextUnitOfWork);
nextUnitOfWork = performUnitOfWork(nextUnitOfWork, isYieldy);
}
}
}

function beginWorkInDEV(
current: Fiber | null,
workInProgress: Fiber,
renderExpirationTime: ExpirationTime,
isYieldy: boolean,
): Fiber | null {
try {
return beginWork(current, workInProgress, renderExpirationTime);
} catch (thrownValue) {
resetContextDependences();
resetHooks();
if (nextUnitOfWork === null) {
Copy link
Collaborator

@acdlite acdlite Nov 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's any way to hit this condition. nextUnitOfWork should always be workInProgress.

throw thrownValue;
}
if (__DEV__) {
// Reset global debug state
// We assume this is defined in DEV
(resetCurrentlyProcessingQueue: any)();
}
if (replayFailedUnitOfWorkWithInvokeGuardedCallback) {
const failedUnitOfWork: Fiber = nextUnitOfWork;
replayUnitOfWork(failedUnitOfWork, thrownValue, current, workInProgress, renderExpirationTime);
}
throw thrownValue;
}
}

Expand Down Expand Up @@ -1302,42 +1327,11 @@ function renderRoot(root: FiberRoot, isYieldy: boolean): void {
resetContextDependences();
resetHooks();

// Reset in case completion throws.
// This is only used in DEV and when replaying is on.
let mayReplay;
if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
mayReplay = mayReplayFailedUnitOfWork;
mayReplayFailedUnitOfWork = true;
}

if (nextUnitOfWork === null) {
// This is a fatal error.
didFatal = true;
onUncaughtError(thrownValue);
} else {
if (__DEV__) {
// Reset global debug state
// We assume this is defined in DEV
(resetCurrentlyProcessingQueue: any)();
}

if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
if (mayReplay) {
const failedUnitOfWork: Fiber = nextUnitOfWork;
replayUnitOfWork(failedUnitOfWork, thrownValue, isYieldy);
}
}

// TODO: we already know this isn't true in some cases.
// At least this shows a nicer error message until we figure out the cause.
// https://github.com/facebook/react/issues/12449#issuecomment-386727431
invariant(
nextUnitOfWork !== null,
'Failed to replay rendering after an error. This ' +
'is likely caused by a bug in React. Please file an issue ' +
'with a reproducing case to help us find it.',
);

const sourceFiber: Fiber = nextUnitOfWork;
let returnFiber = sourceFiber.return;
if (returnFiber === null) {
Expand Down
1 change: 0 additions & 1 deletion packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ describe('ReactIncrementalErrorLogging', () => {
'render: 0',
__DEV__ && 'render: 0', // replay
'render: 1',
__DEV__ && 'render: 1', // replay
'componentWillUnmount: 0',
].filter(Boolean),
);
Expand Down