Yield to main thread if continuation is returned

Instead of using an imperative method `requestYield` to ask Scheduler to
yield to the main thread, we can assume that any time a Scheduler task
returns a continuation callback, it's because it wants to yield to the
main thread. We can assume the task already checked some condition that
caused it to return a continuation, so we don't need to do any
additional checks — we can immediately yield and schedule a new task
for the continuation.

The replaces the `requestYield` API that I added in ca990e9.

Author: acdlite

packages/scheduler/npm/umd/scheduler.development.js

@@ -54,13 +54,6 @@
     );
   }
 
-  function unstable_requestYield() {
-    return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_requestYield.apply(
-      this,
-      arguments
-    );
-  }
-
   function unstable_runWithPriority() {
     return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_runWithPriority.apply(
       this,
@@ -123,7 +116,6 @@
     unstable_cancelCallback: unstable_cancelCallback,
     unstable_shouldYield: unstable_shouldYield,
     unstable_requestPaint: unstable_requestPaint,
-    unstable_requestYield: unstable_requestYield,
     unstable_runWithPriority: unstable_runWithPriority,
     unstable_next: unstable_next,
     unstable_wrapCallback: unstable_wrapCallback,

packages/scheduler/npm/umd/scheduler.production.min.js

@@ -54,13 +54,6 @@
     );
   }
 
-  function unstable_requestYield() {
-    return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_requestYield.apply(
-      this,
-      arguments
-    );
-  }
-
   function unstable_runWithPriority() {
     return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_runWithPriority.apply(
       this,
@@ -117,7 +110,6 @@
     unstable_cancelCallback: unstable_cancelCallback,
     unstable_shouldYield: unstable_shouldYield,
     unstable_requestPaint: unstable_requestPaint,
-    unstable_requestYield: unstable_requestYield,
     unstable_runWithPriority: unstable_runWithPriority,
     unstable_next: unstable_next,
     unstable_wrapCallback: unstable_wrapCallback,

packages/scheduler/npm/umd/scheduler.profiling.min.js

@@ -54,13 +54,6 @@
     );
   }
 
-  function unstable_requestYield() {
-    return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_requestYield.apply(
-      this,
-      arguments
-    );
-  }
-
   function unstable_runWithPriority() {
     return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_runWithPriority.apply(
       this,
@@ -117,7 +110,6 @@
     unstable_cancelCallback: unstable_cancelCallback,
     unstable_shouldYield: unstable_shouldYield,
     unstable_requestPaint: unstable_requestPaint,
-    unstable_requestYield: unstable_requestYield,
     unstable_runWithPriority: unstable_runWithPriority,
     unstable_next: unstable_next,
     unstable_wrapCallback: unstable_wrapCallback,

packages/scheduler/src/__tests__/Scheduler-test.js

@@ -18,7 +18,6 @@ let performance;
 let cancelCallback;
 let scheduleCallback;
 let requestPaint;
-let requestYield;
 let shouldYield;
 let NormalPriority;
 
@@ -44,7 +43,6 @@ describe('SchedulerBrowser', () => {
     scheduleCallback = Scheduler.unstable_scheduleCallback;
     NormalPriority = Scheduler.unstable_NormalPriority;
     requestPaint = Scheduler.unstable_requestPaint;
-    requestYield = Scheduler.unstable_requestYield;
     shouldYield = Scheduler.unstable_shouldYield;
   });
 
@@ -480,19 +478,13 @@ describe('SchedulerBrowser', () => {
     ]);
   });
 
-  it('requestYield forces a yield immediately', () => {
+  it('yielding continues in a new task regardless of how much time is remaining', () => {
     scheduleCallback(NormalPriority, () => {
       runtime.log('Original Task');
       runtime.log('shouldYield: ' + shouldYield());
-      runtime.log('requestYield');
-      requestYield();
-      runtime.log('shouldYield: ' + shouldYield());
+      runtime.log('Return a continuation');
       return () => {
         runtime.log('Continuation Task');
-        runtime.log('shouldYield: ' + shouldYield());
-        runtime.log('Advance time past frame deadline');
-        runtime.advanceTime(10000);
-        runtime.log('shouldYield: ' + shouldYield());
       };
     });
     runtime.assertLog(['Post Message']);
@@ -501,27 +493,20 @@ describe('SchedulerBrowser', () => {
     runtime.assertLog([
       'Message Event',
       'Original Task',
+      // Immediately before returning a continuation, `shouldYield` returns
+      // false, which means there must be time remaining in the frame.
       'shouldYield: false',
-      'requestYield',
-      // Immediately after calling requestYield, shouldYield starts
-      // returning true, even though no time has elapsed in the frame
-      'shouldYield: true',
+      'Return a continuation',
 
-      // The continuation should be scheduled in a separate macrotask.
+      // The continuation should be scheduled in a separate macrotask even
+      // though there's time remaining.
       'Post Message',
     ]);
 
     // No time has elapsed
     expect(performance.now()).toBe(0);
 
-    // Subsequent tasks work as normal
     runtime.fireMessageEvent();
-    runtime.assertLog([
-      'Message Event',
-      'Continuation Task',
-      'shouldYield: false',
-      'Advance time past frame deadline',
-      'shouldYield: true',
-    ]);
+    runtime.assertLog(['Message Event', 'Continuation Task']);
   });
 });

packages/scheduler/src/__tests__/SchedulerMock-test.js

@@ -726,37 +726,53 @@ describe('Scheduler', () => {
       expect(Scheduler).toFlushWithoutYielding();
     });
 
-    it('requestYield forces a yield immediately', () => {
+    it('toFlushUntilNextPaint stops if a continuation is returned', () => {
       scheduleCallback(NormalPriority, () => {
         Scheduler.unstable_yieldValue('Original Task');
-        Scheduler.unstable_yieldValue(
-          'shouldYield: ' + Scheduler.unstable_shouldYield(),
-        );
-        Scheduler.unstable_yieldValue('requestYield');
-        Scheduler.unstable_requestYield();
-        Scheduler.unstable_yieldValue(
-          'shouldYield: ' + Scheduler.unstable_shouldYield(),
-        );
+        Scheduler.unstable_yieldValue('shouldYield: ' + shouldYield());
+        Scheduler.unstable_yieldValue('Return a continuation');
         return () => {
           Scheduler.unstable_yieldValue('Continuation Task');
-          Scheduler.unstable_yieldValue(
-            'shouldYield: ' + Scheduler.unstable_shouldYield(),
-          );
-          Scheduler.unstable_yieldValue('Advance time past frame deadline');
-          Scheduler.unstable_yieldValue(
-            'shouldYield: ' + Scheduler.unstable_shouldYield(),
-          );
         };
       });
 
-      // The continuation should be scheduled in a separate macrotask.
       expect(Scheduler).toFlushUntilNextPaint([
         'Original Task',
+        // Immediately before returning a continuation, `shouldYield` returns
+        // false, which means there must be time remaining in the frame.
         'shouldYield: false',
-        'requestYield',
-        // Immediately after calling requestYield, shouldYield starts
-        // returning true
-        'shouldYield: true',
+        'Return a continuation',
+
+        // The continuation should not flush yet.
+      ]);
+
+      // No time has elapsed
+      expect(Scheduler.unstable_now()).toBe(0);
+
+      // Continue the task
+      expect(Scheduler).toFlushAndYield(['Continuation Task']);
+    });
+
+    it("toFlushAndYield keeps flushing even if there's a continuation", () => {
+      scheduleCallback(NormalPriority, () => {
+        Scheduler.unstable_yieldValue('Original Task');
+        Scheduler.unstable_yieldValue('shouldYield: ' + shouldYield());
+        Scheduler.unstable_yieldValue('Return a continuation');
+        return () => {
+          Scheduler.unstable_yieldValue('Continuation Task');
+        };
+      });
+
+      expect(Scheduler).toFlushAndYield([
+        'Original Task',
+        // Immediately before returning a continuation, `shouldYield` returns
+        // false, which means there must be time remaining in the frame.
+        'shouldYield: false',
+        'Return a continuation',
+
+        // The continuation should flush immediately, even though the task
+        // yielded a continuation.
+        'Continuation Task',
       ]);
     });
   });

packages/scheduler/src/__tests__/SchedulerPostTask-test.js

@@ -23,7 +23,6 @@ let UserBlockingPriority;
 let LowPriority;
 let IdlePriority;
 let shouldYield;
-let requestYield;
 
 // The Scheduler postTask implementation uses a new postTask browser API to
 // schedule work on the main thread. This test suite mocks all browser methods
@@ -47,7 +46,6 @@ describe('SchedulerPostTask', () => {
     LowPriority = Scheduler.unstable_LowPriority;
     IdlePriority = Scheduler.unstable_IdlePriority;
     shouldYield = Scheduler.unstable_shouldYield;
-    requestYield = Scheduler.unstable_requestYield;
   });
 
   afterEach(() => {
@@ -301,19 +299,13 @@ describe('SchedulerPostTask', () => {
     ]);
   });
 
-  it('requestYield forces a yield immediately', () => {
+  it('yielding continues in a new task regardless of how much time is remaining', () => {
     scheduleCallback(NormalPriority, () => {
       runtime.log('Original Task');
       runtime.log('shouldYield: ' + shouldYield());
-      runtime.log('requestYield');
-      requestYield();
-      runtime.log('shouldYield: ' + shouldYield());
+      runtime.log('Return a continuation');
       return () => {
         runtime.log('Continuation Task');
-        runtime.log('shouldYield: ' + shouldYield());
-        runtime.log('Advance time past frame deadline');
-        runtime.advanceTime(10000);
-        runtime.log('shouldYield: ' + shouldYield());
       };
     });
     runtime.assertLog(['Post Task 0 [user-visible]']);
@@ -322,27 +314,20 @@ describe('SchedulerPostTask', () => {
     runtime.assertLog([
       'Task 0 Fired',
       'Original Task',
+      // Immediately before returning a continuation, `shouldYield` returns
+      // false, which means there must be time remaining in the frame.
       'shouldYield: false',
-      'requestYield',
-      // Immediately after calling requestYield, shouldYield starts
-      // returning true, even though no time has elapsed in the frame
-      'shouldYield: true',
+      'Return a continuation',
 
-      // The continuation should be scheduled in a separate macrotask.
+      // The continuation should be scheduled in a separate macrotask even
+      // though there's time remaining.
       'Post Task 1 [user-visible]',
     ]);
 
     // No time has elapsed
     expect(performance.now()).toBe(0);
 
-    // Subsequent tasks work as normal
     runtime.flushTasks();
-    runtime.assertLog([
-      'Task 1 Fired',
-      'Continuation Task',
-      'shouldYield: false',
-      'Advance time past frame deadline',
-      'shouldYield: true',
-    ]);
+    runtime.assertLog(['Task 1 Fired', 'Continuation Task']);
   });
 });

packages/scheduler/src/forks/Scheduler.js

@@ -212,10 +212,14 @@ function workLoop(hasTimeRemaining, initialTime) {
       const continuationCallback = callback(didUserCallbackTimeout);
       currentTime = getCurrentTime();
       if (typeof continuationCallback === 'function') {
+        // If a continuation is returned, immediately yield to the main thread
+        // regardless of how much time is left in the current time slice.
         currentTask.callback = continuationCallback;
         if (enableProfiling) {
           markTaskYield(currentTask, currentTime);
         }
+        advanceTimers(currentTime);
+        return true;
       } else {
         if (enableProfiling) {
           markTaskCompleted(currentTask, currentTime);
@@ -224,8 +228,8 @@ function workLoop(hasTimeRemaining, initialTime) {
         if (currentTask === peek(taskQueue)) {
           pop(taskQueue);
         }
+        advanceTimers(currentTime);
       }
-      advanceTimers(currentTime);
     } else {
       pop(taskQueue);
     }
@@ -495,11 +499,6 @@ function requestPaint() {
   // Since we yield every frame regardless, `requestPaint` has no effect.
 }
 
-function requestYield() {
-  // Force a yield at the next opportunity.
-  startTime = -99999;
-}
-
 function forceFrameRate(fps) {
   if (fps < 0 || fps > 125) {
     // Using console['error'] to evade Babel and ESLint
@@ -617,7 +616,6 @@ export {
   unstable_getCurrentPriorityLevel,
   shouldYieldToHost as unstable_shouldYield,
   requestPaint as unstable_requestPaint,
-  requestYield as unstable_requestYield,
   unstable_continueExecution,
   unstable_pauseExecution,
   unstable_getFirstCallbackNode,

packages/scheduler/src/forks/SchedulerMock.js

@@ -195,10 +195,22 @@ function workLoop(hasTimeRemaining, initialTime) {
       const continuationCallback = callback(didUserCallbackTimeout);
       currentTime = getCurrentTime();
       if (typeof continuationCallback === 'function') {
+        // If a continuation is returned, immediately yield to the main thread
+        // regardless of how much time is left in the current time slice.
         currentTask.callback = continuationCallback;
         if (enableProfiling) {
           markTaskYield(currentTask, currentTime);
         }
+        advanceTimers(currentTime);
+
+        if (shouldYieldForPaint) {
+          needsPaint = true;
+          return true;
+        } else {
+          // If `shouldYieldForPaint` is false, we keep flushing synchronously
+          // without yielding to the main thread. This is the behavior of the
+          // `toFlushAndYield` and `toFlushAndYieldThrough` testing helpers .
+        }
       } else {
         if (enableProfiling) {
           markTaskCompleted(currentTask, currentTime);
@@ -207,8 +219,8 @@ function workLoop(hasTimeRemaining, initialTime) {
         if (currentTask === peek(taskQueue)) {
           pop(taskQueue);
         }
+        advanceTimers(currentTime);
       }
-      advanceTimers(currentTime);
     } else {
       pop(taskQueue);
     }
@@ -608,11 +620,6 @@ function requestPaint() {
   needsPaint = true;
 }
 
-function requestYield() {
-  // Force a yield at the next opportunity.
-  shouldYieldForPaint = needsPaint = true;
-}
-
 export {
   ImmediatePriority as unstable_ImmediatePriority,
   UserBlockingPriority as unstable_UserBlockingPriority,
@@ -627,7 +634,6 @@ export {
   unstable_getCurrentPriorityLevel,
   shouldYieldToHost as unstable_shouldYield,
   requestPaint as unstable_requestPaint,
-  requestYield as unstable_requestYield,
   unstable_continueExecution,
   unstable_pauseExecution,
   unstable_getFirstCallbackNode,

packages/scheduler/src/forks/SchedulerPostTask.js

@@ -67,11 +67,6 @@ export function unstable_requestPaint() {
   // Since we yield every frame regardless, `requestPaint` has no effect.
 }
 
-export function unstable_requestYield() {
-  // Force a yield at the next opportunity.
-  deadline = -99999;
-}
-
 type SchedulerCallback<T> = (
   didTimeout_DEPRECATED: boolean,
 ) =>