-
Notifications
You must be signed in to change notification settings - Fork 46.8k
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.
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
Implement pauseExecution, continueExecution, dumpQueue for Scheduler #14053
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,6 +68,27 @@ | |
); | ||
} | ||
|
||
function unstable_getFirstCallbackNode() { | ||
return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_getFirstCallbackNode.apply( | ||
this, | ||
arguments | ||
); | ||
} | ||
|
||
function unstable_pauseExecution() { | ||
return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_pauseExecution.apply( | ||
this, | ||
arguments | ||
); | ||
} | ||
|
||
function unstable_continueExecution() { | ||
return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_continueExecution.apply( | ||
this, | ||
arguments | ||
); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I strongly dislike the fact that the newly proposed API methods only exist on the DEV build. Do other packages do this? I know that sometimes methods are no-ops in production builds, but having them be missing entirely feels weird. I also think the fact that these methods exist only for DEV reinforces the notion that you could just use existing browser tooling (like the browser's built-in pause/resume functionality). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have an opinion on this. no-oping in production seems a good solution in my book. |
||
|
||
return Object.freeze({ | ||
unstable_now: unstable_now, | ||
unstable_scheduleCallback: unstable_scheduleCallback, | ||
|
@@ -76,5 +97,8 @@ | |
unstable_runWithPriority: unstable_runWithPriority, | ||
unstable_wrapCallback: unstable_wrapCallback, | ||
unstable_getCurrentPriorityLevel: unstable_getCurrentPriorityLevel, | ||
unstable_continueExecution: unstable_continueExecution, | ||
unstable_pauseExecution: unstable_pauseExecution, | ||
unstable_getFirstCallbackNode: unstable_getFirstCallbackNode, | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,8 @@ | |
|
||
/* eslint-disable no-var */ | ||
|
||
import {enableSchedulerDebugging} from 'shared/ReactFeatureFlags'; | ||
|
||
// TODO: Use symbols? | ||
var ImmediatePriority = 1; | ||
var UserBlockingPriority = 2; | ||
|
@@ -33,6 +35,9 @@ var IDLE_PRIORITY = maxSigned31BitInt; | |
var firstCallbackNode = null; | ||
|
||
var currentDidTimeout = false; | ||
// Pausing the scheduler is useful for debugging. | ||
var isSchedulerPaused = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @acdlite I think I might want to expose this variable too, since for debugging it's necessary to know if the thing is paused at all in the first place |
||
|
||
var currentPriorityLevel = NormalPriority; | ||
var currentEventStartTime = -1; | ||
var currentExpirationTime = -1; | ||
|
@@ -173,13 +178,23 @@ function flushImmediateWork() { | |
} | ||
|
||
function flushWork(didTimeout) { | ||
// Exit right away if we're currently paused | ||
|
||
if (enableSchedulerDebugging && isSchedulerPaused) { | ||
return; | ||
} | ||
|
||
isExecutingCallback = true; | ||
const previousDidTimeout = currentDidTimeout; | ||
currentDidTimeout = didTimeout; | ||
try { | ||
if (didTimeout) { | ||
// Flush all the expired callbacks without yielding. | ||
while (firstCallbackNode !== null) { | ||
while ( | ||
firstCallbackNode !== null && | ||
!(enableSchedulerDebugging && isSchedulerPaused) | ||
) { | ||
// TODO Wrap i nfeature flag | ||
// Read the current time. Flush all the callbacks that expire at or | ||
// earlier than that time. Then read the current time again and repeat. | ||
// This optimizes for as few performance.now calls as possible. | ||
|
@@ -189,7 +204,8 @@ function flushWork(didTimeout) { | |
flushFirstCallback(); | ||
} while ( | ||
firstCallbackNode !== null && | ||
firstCallbackNode.expirationTime <= currentTime | ||
firstCallbackNode.expirationTime <= currentTime && | ||
!(enableSchedulerDebugging && isSchedulerPaused) | ||
); | ||
continue; | ||
} | ||
|
@@ -199,6 +215,9 @@ function flushWork(didTimeout) { | |
// Keep flushing callbacks until we run out of time in the frame. | ||
if (firstCallbackNode !== null) { | ||
do { | ||
if (enableSchedulerDebugging && isSchedulerPaused) { | ||
break; | ||
} | ||
flushFirstCallback(); | ||
} while (firstCallbackNode !== null && !shouldYieldToHost()); | ||
} | ||
|
@@ -342,6 +361,21 @@ function unstable_scheduleCallback(callback, deprecated_options) { | |
return newNode; | ||
} | ||
|
||
function unstable_pauseExecution() { | ||
isSchedulerPaused = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why isn't the browser's built-in pause/resume functionality sufficient for whatever you're using this for? |
||
} | ||
|
||
function unstable_continueExecution() { | ||
isSchedulerPaused = false; | ||
if (firstCallbackNode !== null) { | ||
ensureHostCallbackIsScheduled(); | ||
} | ||
} | ||
|
||
function unstable_getFirstCallbackNode() { | ||
return firstCallbackNode; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of exposing play, pause, and dump-queue– what if we just exposed the queue itself ( (This also assumes that we just use the browser's built-in play/pause script execution functionality. |
||
|
||
function unstable_cancelCallback(callbackNode) { | ||
var next = callbackNode.next; | ||
if (next === null) { | ||
|
@@ -659,5 +693,8 @@ export { | |
unstable_wrapCallback, | ||
unstable_getCurrentPriorityLevel, | ||
unstable_shouldYield, | ||
unstable_continueExecution, | ||
unstable_pauseExecution, | ||
unstable_getFirstCallbackNode, | ||
getCurrentTime as unstable_now, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding a new integration test!