From 43f8cc4265b202977fb71d66fa2b60b1ef891179 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Wed, 27 Jan 2021 12:08:37 -0700 Subject: [PATCH] Use callback priority to determine cancellation --- .../src/ReactFiberWorkLoop.new.js | 29 ++++++++++++++----- .../src/ReactFiberWorkLoop.old.js | 29 ++++++++++++++----- scripts/error-codes/codes.json | 3 +- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 0281eec1ce991..584075cffee42 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -716,22 +716,34 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { // Special case: There's nothing to work on. if (existingCallbackNode !== null) { cancelCallback(existingCallbackNode); - root.callbackNode = null; - root.callbackPriority = NoLanePriority; } + root.callbackNode = null; + root.callbackPriority = NoLanePriority; return; } // Check if there's an existing task. We may be able to reuse it. - if (existingCallbackNode !== null) { - const existingCallbackPriority = root.callbackPriority; + const existingCallbackPriority = root.callbackPriority; + if ( + existingCallbackPriority !== NoLanePriority && + existingCallbackPriority !== InputDiscreteLanePriority + ) { if (existingCallbackPriority === newCallbackPriority) { // The priority hasn't changed. We can reuse the existing task. Exit. return; } - // The priority changed. Cancel the existing callback. We'll schedule a new - // one below. - cancelCallback(existingCallbackNode); + + if (existingCallbackNode != null) { + // The priority changed. Cancel the existing callback. + // We'll schedule a new one below. + cancelCallback(existingCallbackNode); + } else { + // TODO: Temporary. This shouldn't happen, but remove after confirmed. + // Using console['error'] to evade Babel and ESLint + console['error']( + 'Expected scheduled callback to exist. This error is likely caused by a bug in React. Please file an issue.', + ); + } } // Schedule a new callback. @@ -739,6 +751,8 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { if (newCallbackPriority === SyncLanePriority) { // Special case: Sync React callbacks are scheduled on a special // internal queue + + // TODO: After enableDiscreteEventMicroTasks lands, we can remove the fake node. newCallbackNode = scheduleSyncCallback( performSyncWorkOnRoot.bind(null, root), ); @@ -1879,6 +1893,7 @@ function commitRootImpl(root, renderPriorityLevel) { // commitRoot never returns a continuation; it always finishes synchronously. // So we can clear these now to allow a new callback to be scheduled. root.callbackNode = null; + root.callbackPriority = NoLanePriority; // Update the first and last pending times on this root. The new first // pending time is whatever is left on the root fiber. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index c4f49c3e8dd2b..8597c3825325f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -698,22 +698,34 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { // Special case: There's nothing to work on. if (existingCallbackNode !== null) { cancelCallback(existingCallbackNode); - root.callbackNode = null; - root.callbackPriority = NoLanePriority; } + root.callbackNode = null; + root.callbackPriority = NoLanePriority; return; } // Check if there's an existing task. We may be able to reuse it. - if (existingCallbackNode !== null) { - const existingCallbackPriority = root.callbackPriority; + const existingCallbackPriority = root.callbackPriority; + if ( + existingCallbackPriority !== NoLanePriority && + existingCallbackPriority !== InputDiscreteLanePriority + ) { if (existingCallbackPriority === newCallbackPriority) { // The priority hasn't changed. We can reuse the existing task. Exit. return; } - // The priority changed. Cancel the existing callback. We'll schedule a new - // one below. - cancelCallback(existingCallbackNode); + + if (existingCallbackNode != null) { + // The priority changed. Cancel the existing callback. + // We'll schedule a new one below. + cancelCallback(existingCallbackNode); + } else { + // TODO: Temporary. This shouldn't happen, but remove after confirmed. + // Using console['error'] to evade Babel and ESLint + console['error']( + 'Expected scheduled callback to exist. This error is likely caused by a bug in React. Please file an issue.', + ); + } } // Schedule a new callback. @@ -721,6 +733,8 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { if (newCallbackPriority === SyncLanePriority) { // Special case: Sync React callbacks are scheduled on a special // internal queue + + // TODO: After enableDiscreteEventMicroTasks lands, we can remove the fake node. newCallbackNode = scheduleSyncCallback( performSyncWorkOnRoot.bind(null, root), ); @@ -1859,6 +1873,7 @@ function commitRootImpl(root, renderPriorityLevel) { // commitRoot never returns a continuation; it always finishes synchronously. // So we can clear these now to allow a new callback to be scheduled. root.callbackNode = null; + root.callbackPriority = NoLanePriority; // Update the first and last pending times on this root. The new first // pending time is whatever is left on the root fiber. diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 12a8db733b0ce..ac30edd08d14d 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -372,5 +372,6 @@ "381": "This feature is not supported by ReactSuspenseTestUtils.", "382": "This query has received more parameters than the last time the same query was used. Always pass the exact number of parameters that the query needs.", "383": "This query has received fewer parameters than the last time the same query was used. Always pass the exact number of parameters that the query needs.", - "384": "Refreshing the cache is not supported in Server Components." + "384": "Refreshing the cache is not supported in Server Components.", + "385": "Expected scheduled callback to exist. This error is likely caused by a bug in React. Please file an issue." }