Skip to content
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

Add support for multiple callbacks in ReactScheduler #12682

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
155 changes: 133 additions & 22 deletions packages/react-scheduler/src/ReactScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@
* control than requestAnimationFrame and requestIdleCallback.
* Current TODO items:
* X- Pull out the rIC polyfill built into React
* - Initial test coverage
* - Support for multiple callbacks
* X- Initial test coverage
* X- Support for multiple callbacks
* - Support for two priorities; serial and deferred
* - Better test coverage
* - Mock out the react-scheduler module, not the browser APIs, in renderer
* tests
* - Add fixture test of react-scheduler
* - Better docblock
* - Polish documentation, API
*/
Expand All @@ -31,6 +34,11 @@
// The frame rate is dynamically adjusted.

import type {Deadline} from 'react-reconciler';
type CallbackConfigType = {|
scheduledCallback: Deadline => void,
timeoutTime: number,
callbackId: number, // used for cancelling
|};

import ExecutionEnvironment from 'fbjs/lib/ExecutionEnvironment';
import warning from 'fbjs/lib/warning';
Expand Down Expand Up @@ -87,9 +95,19 @@ if (!ExecutionEnvironment.canUseDOM) {
} else {
// Always polyfill requestIdleCallback and cancelIdleCallback

let scheduledRICCallback = null;
// Number.MAX_SAFE_INTEGER is not supported in IE
const MAX_SAFE_INTEGER = Number.MAX_SAFE_INTEGER || 9007199254740991;
let callbackIdCounter = 1;
let scheduledCallbackConfig: CallbackConfigType | null = null;
const getCallbackId = function(): number {
callbackIdCounter =
callbackIdCounter >= MAX_SAFE_INTEGER ? 1 : callbackIdCounter + 1;
return callbackIdCounter;
};
let isIdleScheduled = false;
let timeoutTime = -1;
let isCurrentlyRunningCallback = false;
// We keep a queue of pending callbacks
let pendingCallbacks: Array<CallbackConfigType> = [];

let isAnimationFrameScheduled = false;

Expand All @@ -100,14 +118,42 @@ if (!ExecutionEnvironment.canUseDOM) {
let previousFrameTime = 33;
let activeFrameTime = 33;

const frameDeadlineObject = {
// When a callback is scheduled, we register it by adding it's id to this
// object.
// If the user calls 'cIC' with the id of that callback, it will be
// unregistered by removing the id from this object.
// Then we skip calling any callback which is not registered.
// This means cancelling is an O(1) time complexity instead of O(n).
const registeredCallbackIds: Map<number, boolean> = new Map();

const frameDeadlineObject: Deadline = {
didTimeout: false,
timeRemaining() {
const remaining = frameDeadline - now();
return remaining > 0 ? remaining : 0;
},
};

const safelyCallScheduledCallback = function(callback, callbackId) {
if (!registeredCallbackIds.get(callbackId)) {
// ignore cancelled callbacks
return;
}
isCurrentlyRunningCallback = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not necessary. Just reset the list before you start working.

try {
callback(frameDeadlineObject);
registeredCallbackIds.delete(callbackId);
isCurrentlyRunningCallback = false;
} catch (e) {
registeredCallbackIds.delete(callbackId);
isCurrentlyRunningCallback = false;
// Still throw it, but not in this frame.
setTimeout(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sebmarkbage would like your input here, I think this is the last thing to decide before landing this PR. My previous comment here:


We discussed this in person and considered using postMessage to either throw the error sooner or even to create a postMessage event for each callback, so that errors could be thrown without interrupting the queue of callbacks getting called.

@sebmarkbage we would like you to make the call on whether the setTimeout is good enough for now, or should we explore using postMessage or some other approach?

I also considered saving the errors and just throwing them once we finish whatever is in the queue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not in this frame?

This will mess with debugging since it'll be a caught exception so you can't break point on it. Suspense will destroy the use of "pause on caught exception".

Another technique is to call this recursively with a try/finally and you don't need the catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We talked a bit more and decided the 'try/finally' trick won't work, because a 2nd error thrown in the queue will get swallowed.

Going to look at alternatives, based on what we currently do in React to handle errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

screen shot 2018-05-03 at 10 04 07 am
screen shot 2018-05-03 at 10 03 59 am

throw e;
});
}
};

// We use the postMessage trick to defer idle work until after the repaint.
const messageKey =
'__reactIdleCallback$' +
Expand All @@ -119,16 +165,22 @@ if (!ExecutionEnvironment.canUseDOM) {
return;
}

if (scheduledCallbackConfig === null) {
return;
}

isIdleScheduled = false;

const currentTime = now();
const timeoutTime = scheduledCallbackConfig.timeoutTime;
let didTimeout = false;
if (frameDeadline - currentTime <= 0) {
// There's no time left in this idle period. Check if the callback has
// a timeout and whether it's been exceeded.
if (timeoutTime !== -1 && timeoutTime <= currentTime) {
// Exceeded the timeout. Invoke the callback even though there's no
// time left.
frameDeadlineObject.didTimeout = true;
didTimeout = true;
} else {
// No timeout.
if (!isAnimationFrameScheduled) {
Expand All @@ -141,14 +193,15 @@ if (!ExecutionEnvironment.canUseDOM) {
}
} else {
// There's still time left in this idle period.
frameDeadlineObject.didTimeout = false;
didTimeout = false;
}

timeoutTime = -1;
const callback = scheduledRICCallback;
scheduledRICCallback = null;
if (callback !== null) {
callback(frameDeadlineObject);
const scheduledCallback = scheduledCallbackConfig.scheduledCallback;
const scheduledCallbackId = scheduledCallbackConfig.callbackId;
scheduledCallbackConfig = null;
if (scheduledCallback !== null && typeof scheduledCallbackId === 'number') {
frameDeadlineObject.didTimeout = didTimeout;
safelyCallScheduledCallback(scheduledCallback, scheduledCallbackId);
}
};
// Assumes that we have addEventListener in this environment. Might need
Expand Down Expand Up @@ -190,12 +243,72 @@ if (!ExecutionEnvironment.canUseDOM) {
callback: (deadline: Deadline) => void,
options?: {timeout: number},
): number {
// This assumes that we only schedule one callback at a time because that's
// how Fiber uses it.
scheduledRICCallback = callback;
if (options != null && typeof options.timeout === 'number') {
timeoutTime = now() + options.timeout;
// Handling multiple callbacks:
// For now we implement the behavior expected when the callbacks are
// serial updates, such that each update relies on the previous ones
// having been called before it runs.
// So we call anything in the queue before the latest callback

const latestCallbackId = getCallbackId();
if (scheduledCallbackConfig === null) {
// Set up the next callback config
let timeoutTime = -1;
if (options != null && typeof options.timeout === 'number') {
timeoutTime = now() + options.timeout;
}
scheduledCallbackConfig = {
scheduledCallback: callback,
timeoutTime,
callbackId: latestCallbackId,
};
registeredCallbackIds.set(latestCallbackId, true);
} else {
// If we have a previous callback config, we call that and then schedule
// the latest callback.
const previouslyScheduledCallbackConfig = scheduledCallbackConfig;

// Then set up the next callback config
let timeoutTime = -1;
if (options != null && typeof options.timeout === 'number') {
timeoutTime = now() + options.timeout;
}
scheduledCallbackConfig = {
scheduledCallback: callback,
timeoutTime,
callbackId: latestCallbackId,
};
registeredCallbackIds.set(latestCallbackId, true);

// If we have previousCallback, call it. This may trigger recursion.
const previousCallbackTimeout: number =
previouslyScheduledCallbackConfig.timeoutTime;
const previousCallback =
previouslyScheduledCallbackConfig.scheduledCallback;
if (isCurrentlyRunningCallback) {
// we are inside a recursive call to rIC
// add this callback to a pending queue and run after we exit
pendingCallbacks.push(previouslyScheduledCallbackConfig);
} else {
const prevCallbackId = previouslyScheduledCallbackConfig.callbackId;
frameDeadlineObject.didTimeout =
previousCallbackTimeout !== -1 && previousCallbackTimeout <= now();
safelyCallScheduledCallback(previousCallback, prevCallbackId);
while (pendingCallbacks.length) {
// the callback recursively called rIC and new callbacks are pending
const callbackConfig = pendingCallbacks.shift();
const pendingCallback = callbackConfig.scheduledCallback;
const pendingCallbackTimeout = callbackConfig.timeoutTime;
frameDeadlineObject.didTimeout =
pendingCallbackTimeout !== -1 && pendingCallbackTimeout <= now();
safelyCallScheduledCallback(
pendingCallback,
callbackConfig.callbackId,
);
}
}
}

// finally, after clearing previous callbacks, schedule the latest one
if (!isAnimationFrameScheduled) {
// If rAF didn't already schedule one, we need to schedule a frame.
// TODO: If this rAF doesn't materialize because the browser throttles, we
Expand All @@ -204,13 +317,11 @@ if (!ExecutionEnvironment.canUseDOM) {
isAnimationFrameScheduled = true;
requestAnimationFrame(animationTick);
}
return 0;
return latestCallbackId;
};

cIC = function() {
scheduledRICCallback = null;
isIdleScheduled = false;
timeoutTime = -1;
cIC = function(callbackId: number) {
registeredCallbackIds.delete(callbackId);
};
}

Expand Down
Loading