Skip to content

Commit 4221565

Browse files
committed
Cancel pending commit before starting on root
Moves the cancelTimeout call to right before creating a new work-in- progress root. Fixes a class of bugs where a pending commit is not cancelled, causing an incomplete tree to accidentally commit. In the interest of fixing downstream bugs quickly, I'm landing this without a test case; I'll add one in a follow up.
1 parent 9ebe176 commit 4221565

File tree

2 files changed

+9
-17
lines changed

2 files changed

+9
-17
lines changed

packages/react-reconciler/src/ReactFiberScheduler.js

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -479,15 +479,6 @@ function scheduleCallbackForRoot(
479479
}
480480
}
481481

482-
const timeoutHandle = root.timeoutHandle;
483-
if (timeoutHandle !== noTimeout) {
484-
// The root previous suspended and scheduled a timeout to commit a fallback
485-
// state. Now that we have additional work, cancel the timeout.
486-
root.timeoutHandle = noTimeout;
487-
// $FlowFixMe Complains noTimeout is not a TimeoutID, despite the check above
488-
cancelTimeout(timeoutHandle);
489-
}
490-
491482
// Add the current set of interactions to the pending set associated with
492483
// this root.
493484
schedulePendingInteraction(root, expirationTime);
@@ -673,6 +664,15 @@ export function flushControlled(fn: () => mixed): void {
673664
function prepareFreshStack(root, expirationTime) {
674665
root.pendingCommitExpirationTime = NoWork;
675666

667+
const timeoutHandle = root.timeoutHandle;
668+
if (timeoutHandle !== noTimeout) {
669+
// The root previous suspended and scheduled a timeout to commit a fallback
670+
// state. Now that we have additional work, cancel the timeout.
671+
root.timeoutHandle = noTimeout;
672+
// $FlowFixMe Complains noTimeout is not a TimeoutID, despite the check above
673+
cancelTimeout(timeoutHandle);
674+
}
675+
676676
if (workInProgress !== null) {
677677
let interruptedWork = workInProgress.return;
678678
while (interruptedWork !== null) {

packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1724,11 +1724,3 @@ describe('ReactSuspenseWithNoopRenderer', () => {
17241724
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
17251725
});
17261726
});
1727-
1728-
// TODO:
1729-
// An update suspends, timeout is scheduled. Update again with different timeout.
1730-
// An update suspends, a higher priority update also suspends, each has different timeouts.
1731-
// Can update siblings of a timed out placeholder without suspending
1732-
// Pinging during the render phase
1733-
// Synchronous thenable
1734-
// Start time is computed using earliest suspended time

0 commit comments

Comments
 (0)