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.
Andrew Clark committed
Apr 15, 2019 at 16:00 UTC
4221565e1552425e9416035b5a4bf0ff567d18be
2 files changed
+9
-17
packages/react-reconciler/src/ReactFiberScheduler.js
+9
-9
@@ -479,15 +479,6 @@ function scheduleCallbackForRoot(
479
}
480
}
481
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
-
482
// Add the current set of interactions to the pending set associated with
483
// this root.
484
schedulePendingInteraction(root, expirationTime);
@@ -673,6 +664,15 @@ export function flushControlled(fn: () => mixed): void {
664
function prepareFreshStack(root, expirationTime) {
665
root.pendingCommitExpirationTime = NoWork;
666
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
+
676
if (workInProgress !== null) {
677
let interruptedWork = workInProgress.return;
678
while (interruptedWork !== null) {
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
-8
@@ -1724,11 +1724,3 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1724
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
1725
});
1726
});
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