Repository: facebook/react
Pull request: [schedule] Support multiple callbacks in scheduler

NOTE: We still need a way to test the timing out of callbacks, probably with some type of injection we we could create a 'noopSchedule' similar to 'noopRenderer'.

I'm going to go back and implement that as a 2/n PR, but wanted review of this because it's the direction I would ideally take after that.

This PR would be followed by a PR to add error handling.

---

    Support multiple callbacks in `ReactScheduler`
    
    **what is the change?:**
    We keep a queue of callbacks instead of just one at a time, and call
    them in order first by their timeoutTime and then by the order which
    they were scheduled in.
    
    **why make this change?:**
    We plan on using this module to coordinate JS outside of React, so we
    will need to schedule more than one callback at a time.
    
    **test plan:**
    Added a boatload of shiny new tests. :)
    
    Plus ran all the old ones.

Changed files:
filename: packages/react-scheduler/src/ReactScheduler.js
patch: @@ -14,8 +14,8 @@
  * 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
  * - Better docblock
@@ -31,6 +31,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';
@@ -85,12 +90,31 @@ if (!ExecutionEnvironment.canUseDOM) {
     clearTimeout(timeoutID);
   };
 } else {
-  // Always polyfill requestIdleCallback and cancelIdleCallback
+  // We keep callbacks in a queue.
+  // Calling rIC will push in a new callback at the end of the queue.
+  // When we get idle time, callbacks are removed from the front of the queue
+  // and called.
+  const pendingCallbacks: Array<CallbackConfigType> = [];
 
-  let scheduledRICCallback = null;
-  let isIdleScheduled = false;
-  let timeoutTime = -1;
+  let callbackIdCounter = 0;
+  const getCallbackId = function(): number {
+    callbackIdCounter++;
+    return callbackIdCounter;
+  };
+
+  // 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: {[number]: boolean} = {};
 
+  // We track what the next soonest timeoutTime is, to be able to quickly tell
+  // if none of the scheduled callbacks have timed out.
+  let nextSoonestTimeoutTime = -1;
+
+  let isIdleScheduled = false;
   let isAnimationFrameScheduled = false;
 
   let frameDeadline = 0;
@@ -100,14 +124,75 @@ if (!ExecutionEnvironment.canUseDOM) {
   let previousFrameTime = 33;
   let activeFrameTime = 33;
 
-  const frameDeadlineObject = {
+  const frameDeadlineObject: Deadline = {
     didTimeout: false,
     timeRemaining() {
       const remaining = frameDeadline - now();
       return remaining > 0 ? remaining : 0;
     },
   };
 
+  const safelyCallScheduledCallback = function(callback, callbackId) {
+    if (!registeredCallbackIds[callbackId]) {
+      // ignore cancelled callbacks
+      return;
+    }
+    try {
+      callback(frameDeadlineObject);
+      // Avoid using 'catch' to keep errors easy to debug
+    } finally {
+      // always clean up the callbackId, even if the callback throws
+      delete registeredCallbackIds[callbackId];
+    }
+  };
+
+  /**
+   * Checks for timed out callbacks, runs them, and then checks again to see if
+   * any more have timed out.
+   * Keeps doing this until there are none which have currently timed out.
+   */
+  const callTimedOutCallbacks = function() {
+    if (pendingCallbacks.length === 0) {
+      return;
+    }
+
+    const currentTime = now();
+    // TODO: this would be more efficient if deferred callbacks are stored in
+    // min heap.
+    // Or in a linked list with links for both timeoutTime order and insertion
+    // order.
+    // For now an easy compromise is the current approach:
+    // Keep a pointer to the soonest timeoutTime, and check that first.
+    // If it has not expired, we can skip traversing the whole list.
+    // If it has expired, then we step through all the callbacks.
+    if (nextSoonestTimeoutTime === -1 || nextSoonestTimeoutTime > currentTime) {
+      // We know that none of them have timed out yet.
+      return;
+    }
+    nextSoonestTimeoutTime = -1; // we will reset it below
+
+    // keep checking until we don't find any more timed out callbacks
+    frameDeadlineObject.didTimeout = true;
+    for (let i = 0, len = pendingCallbacks.length; i < len; i++) {
+      const currentCallbackConfig = pendingCallbacks[i];
+      const timeoutTime = currentCallbackConfig.timeoutTime;
+      if (timeoutTime !== -1 && timeoutTime <= currentTime) {
+        // it has timed out!
+        // call it
+        const callback = currentCallbackConfig.scheduledCallback;
+        safelyCallScheduledCallback(callback, timeoutTime);
+      } else {
+        if (
+          timeoutTime !== -1 &&
+          (nextSoonestTimeoutTime === -1 ||
+            timeoutTime < nextSoonestTimeoutTime)
+        ) {
+          nextSoonestTimeoutTime = timeoutTime;
+        }
+      }
+    }
+  };
+
   // We use the postMessage trick to defer idle work until after the repaint.
   const messageKey =
     '__reactIdleCallback$' +
@@ -119,36 +204,30 @@ if (!ExecutionEnvironment.canUseDOM) {
       return;
     }
 
+    if (pendingCallbacks.length === 0) {
+      return;
+    }
     isIdleScheduled = false;
 
-    const currentTime = now();
-    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;
-      } else {
-        // No timeout.
-        if (!isAnimationFrameScheduled) {
-          // Schedule another animation callback so we retry later.
-          isAnimationFrameScheduled = true;
-          requestAnimationFrame(animationTick);
-        }
-        // Exit without invoking the callback.
-        return;
-      }
-    } else {
-      // There's still time left in this idle period.
+    // First call anything which has timed out, until we have caught up.
+    callTimedOutCallbacks();
+
+    let currentTime = now();
+    // Next, as long as we have idle time, try calling more callbacks.
+    while (frameDeadline - currentTime > 0 && pendingCallbacks.length > 0) {
+      const latestCallbackConfig = pendingCallbacks.shift();
       frameDeadlineObject.didTimeout = false;
+      const latestCallback = latestCallbackConfig.scheduledCallback;
+      const newCallbackId = latestCallbackConfig.callbackId;
+      safelyCallScheduledCallback(latestCallback, newCallbackId);
+      currentTime = now();
     }
-
-    timeoutTime = -1;
-    const callback = scheduledRICCallback;
-    scheduledRICCallback = null;
-    if (callback !== null) {
-      callback(frameDeadlineObject);
+    if (pendingCallbacks.length > 0) {
+      if (!isAnimationFrameScheduled) {
+        // Schedule another animation callback so we retry later.
+        isAnimationFrameScheduled = true;
+        requestAnimationFrame(animationTick);
+      }
     }
   };
   // Assumes that we have addEventListener in this environment. Might need
@@ -190,12 +269,23 @@ 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;
+    let timeoutTime = -1;
     if (options != null && typeof options.timeout === 'number') {
       timeoutTime = now() + options.timeout;
     }
+    if (timeoutTime > nextSoonestTimeoutTime) {
+      nextSoonestTimeoutTime = timeoutTime;
+    }
+
+    const newCallbackId = getCallbackId();
+    const scheduledCallbackConfig = {
+      scheduledCallback: callback,
+      callbackId: newCallbackId,
+      timeoutTime,
+    };
+    pendingCallbacks.push(scheduledCallbackConfig);
+
+    registeredCallbackIds[newCallbackId] = true;
     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
@@ -204,13 +294,11 @@ if (!ExecutionEnvironment.canUseDOM) {
       isAnimationFrameScheduled = true;
       requestAnimationFrame(animationTick);
     }
-    return 0;
+    return newCallbackId;
   };
 
-  cIC = function() {
-    scheduledRICCallback = null;
-    isIdleScheduled = false;
-    timeoutTime = -1;
+  cIC = function(callbackId: number) {
+    delete registeredCallbackIds[callbackId];
   };
 }
 

filename: packages/react-scheduler/src/__tests__/ReactScheduler-test.js
patch: @@ -41,16 +41,167 @@ describe('ReactScheduler', () => {
     ReactScheduler = require('react-scheduler');
   });
 
-  it('rIC calls the callback within the frame when not blocked', () => {
-    const {rIC} = ReactScheduler;
-    const cb = jest.fn();
-    rIC(cb);
-    jest.runAllTimers();
-    expect(cb.mock.calls.length).toBe(1);
-    // should have ... TODO details on what we expect
-    expect(cb.mock.calls[0][0].didTimeout).toBe(false);
-    expect(typeof cb.mock.calls[0][0].timeRemaining()).toBe('number');
+  describe('rIC', () => {
+    it('calls the callback within the frame when not blocked', () => {
+      const {rIC} = ReactScheduler;
+      const cb = jest.fn();
+      rIC(cb);
+      jest.runAllTimers();
+      expect(cb.mock.calls.length).toBe(1);
+      // should not have timed out and should include a timeRemaining method
+      expect(cb.mock.calls[0][0].didTimeout).toBe(false);
+      expect(typeof cb.mock.calls[0][0].timeRemaining()).toBe('number');
+    });
+
+    describe('with multiple callbacks', () => {
+      it('accepts multiple callbacks and calls within frame when not blocked', () => {
+        const {rIC} = ReactScheduler;
+        const callbackLog = [];
+        const callbackA = jest.fn(() => callbackLog.push('A'));
+        const callbackB = jest.fn(() => callbackLog.push('B'));
+        rIC(callbackA);
+        // initially waits to call the callback
+        expect(callbackLog).toEqual([]);
+        // waits while second callback is passed
+        rIC(callbackB);
+        expect(callbackLog).toEqual([]);
+        // after a delay, calls as many callbacks as it has time for
+        jest.runAllTimers();
+        expect(callbackLog).toEqual(['A', 'B']);
+        // callbackA should not have timed out and should include a timeRemaining method
+        expect(callbackA.mock.calls[0][0].didTimeout).toBe(false);
+        expect(typeof callbackA.mock.calls[0][0].timeRemaining()).toBe(
+          'number',
+        );
+        // callbackA should not have timed out and should include a timeRemaining method
+        expect(callbackB.mock.calls[0][0].didTimeout).toBe(false);
+        expect(typeof callbackB.mock.calls[0][0].timeRemaining()).toBe(
+          'number',
+        );
+      });
+
+      it(
+        'schedules callbacks in correct order and' +
+          'keeps calling them if there is time',
+        () => {
+          const {rIC} = ReactScheduler;
+          const callbackLog = [];
+          const callbackA = jest.fn(() => {
+            callbackLog.push('A');
+            rIC(callbackC);
+          });
+          const callbackB = jest.fn(() => {
+            callbackLog.push('B');
+          });
+          const callbackC = jest.fn(() => {
+            callbackLog.push('C');
+          });
+
+          rIC(callbackA);
+          // initially waits to call the callback
+          expect(callbackLog).toEqual([]);
+          // continues waiting while B is scheduled
+          rIC(callbackB);
+          expect(callbackLog).toEqual([]);
+          // after a delay, calls the scheduled callbacks,
+          // and also calls new callbacks scheduled by current callbacks
+          jest.runAllTimers();
+          expect(callbackLog).toEqual(['A', 'B', 'C']);
+        },
+      );
+
+      it('schedules callbacks in correct order when callbacks have many nested rIC calls', () => {
+        const {rIC} = ReactScheduler;
+        const callbackLog = [];
+        const callbackA = jest.fn(() => {
+          callbackLog.push('A');
+          rIC(callbackC);
+          rIC(callbackD);
+        });
+        const callbackB = jest.fn(() => {
+          callbackLog.push('B');
+          rIC(callbackE);
+          rIC(callbackF);
+        });
+        const callbackC = jest.fn(() => {
+          callbackLog.push('C');
+        });
+        const callbackD = jest.fn(() => {
+          callbackLog.push('D');
+        });
+        const callbackE = jest.fn(() => {
+          callbackLog.push('E');
+        });
+        const callbackF = jest.fn(() => {
+          callbackLog.push('F');
+        });
+
+        rIC(callbackA);
+        rIC(callbackB);
+        // initially waits to call the callback
+        expect(callbackLog).toEqual([]);
+        // while flushing callbacks, calls as many as it has time for
+        jest.runAllTimers();
+        expect(callbackLog).toEqual(['A', 'B', 'C', 'D', 'E', 'F']);
+      });
+
+      it('schedules callbacks in correct order when they use rIC to schedule themselves', () => {
+        const {rIC} = ReactScheduler;
+        const callbackLog = [];
+        let callbackAIterations = 0;
+        const callbackA = jest.fn(() => {
+          if (callbackAIterations < 1) {
+            rIC(callbackA);
+          }
+          callbackLog.push('A' + callbackAIterations);
+          callbackAIterations++;
+        });
+        const callbackB = jest.fn(() => callbackLog.push('B'));
+
+        rIC(callbackA);
+        // initially waits to call the callback
+        expect(callbackLog).toEqual([]);
+        rIC(callbackB);
+        expect(callbackLog).toEqual([]);
+        // after a delay, calls the latest callback passed
+        jest.runAllTimers();
+        expect(callbackLog).toEqual(['A0', 'B', 'A1']);
+      });
+    });
+  });
+
+  describe('cIC', () => {
+    it('cancels the scheduled callback', () => {
+      const {rIC, cIC} = ReactScheduler;
+      const cb = jest.fn();
+      const callbackId = rIC(cb);
+      expect(cb.mock.calls.length).toBe(0);
+      cIC(callbackId);
+      jest.runAllTimers();
+      expect(cb.mock.calls.length).toBe(0);
+    });
+
+    describe('with multiple callbacks', () => {
+      it('when one callback cancels the next one', () => {
+        const {rIC, cIC} = ReactScheduler;
+        const callbackLog = [];
+        let callbackBId;
+        const callbackA = jest.fn(() => {
+          callbackLog.push('A');
+          cIC(callbackBId);
+        });
+        const callbackB = jest.fn(() => callbackLog.push('B'));
+        rIC(callbackA);
+        callbackBId = rIC(callbackB);
+        // Initially doesn't call anything
+        expect(callbackLog).toEqual([]);
+        jest.runAllTimers();
+        // B should not get called because A cancelled B
+        expect(callbackLog).toEqual(['A']);
+        expect(callbackB.mock.calls.length).toBe(0);
+      });
+    });
   });
 
-  // TODO: test cIC and now
+  // TODO: test 'now'
 });

filename: packages/shared/forks/ReactFeatureFlags.www.js
patch: @@ -12,11 +12,11 @@ import typeof * as FeatureFlagsShimType from './ReactFeatureFlags.www';
 
 // Re-export dynamic flags from the www version.
 export const {
-  enableGetDerivedStateFromCatch,
   debugRenderPhaseSideEffects,
   debugRenderPhaseSideEffectsForStrictMode,
-  warnAboutDeprecatedLifecycles,
+  enableGetDerivedStateFromCatch,
   replayFailedUnitOfWorkWithInvokeGuardedCallback,
+  warnAboutDeprecatedLifecycles,
 } = require('ReactFeatureFlags');
 
 // The rest of the flags are static for better dead code elimination.