From 9abc2785cb070148d64fae81e523246b90b92016 Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Thu, 13 Aug 2020 11:49:24 -0500
Subject: [PATCH] Remove wasteful checks from `shouldYield`

`shouldYield` will currently return `true` if there's a higher priority
task in the Scheduler queue.

Since we yield every 5ms anyway, this doesn't really have any practical
benefit. On the contrary, the extra checks on every `shouldYield` call
are wasteful.
---
 .../__tests__/ReactProfiler-test.internal.js    | 10 +++-------
 packages/scheduler/src/Scheduler.js             | 17 +----------------
 .../scheduler/src/__tests__/Scheduler-test.js   | 13 ++++++-------
 3 files changed, 10 insertions(+), 30 deletions(-)

diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js
index 78672895584fd..c3d1dbb3cea71 100644
--- a/packages/react/src/__tests__/ReactProfiler-test.internal.js
+++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js
@@ -2481,15 +2481,11 @@ describe('Profiler', () => {
         // Errors that happen inside of a subscriber should throw,
         throwInOnWorkStarted = true;
         expect(Scheduler).toFlushAndThrow('Expected error onWorkStarted');
-        // Rendering was interrupted by the error that was thrown
-        expect(Scheduler).toHaveYielded([]);
-        // Rendering continues in the next task
-        expect(Scheduler).toFlushAndYield(['Component:text']);
         throwInOnWorkStarted = false;
+        // Rendering was interrupted by the error that was thrown, then
+        // continued and finished in the next task.
+        expect(Scheduler).toHaveYielded(['Component:text']);
         expect(onWorkStarted).toHaveBeenCalled();
-
-        // But the React work should have still been processed.
-        expect(Scheduler).toFlushAndYield([]);
         const tree = renderer.toTree();
         expect(tree.type).toBe(Component);
         expect(tree.props.children).toBe('text');
diff --git a/packages/scheduler/src/Scheduler.js b/packages/scheduler/src/Scheduler.js
index ff9e06c7a345d..9162da0a0875c 100644
--- a/packages/scheduler/src/Scheduler.js
+++ b/packages/scheduler/src/Scheduler.js
@@ -393,21 +393,6 @@ function unstable_getCurrentPriorityLevel() {
   return currentPriorityLevel;
 }
 
-function unstable_shouldYield() {
-  const currentTime = getCurrentTime();
-  advanceTimers(currentTime);
-  const firstTask = peek(taskQueue);
-  return (
-    (firstTask !== currentTask &&
-      currentTask !== null &&
-      firstTask !== null &&
-      firstTask.callback !== null &&
-      firstTask.startTime <= currentTime &&
-      firstTask.expirationTime < currentTask.expirationTime) ||
-    shouldYieldToHost()
-  );
-}
-
 const unstable_requestPaint = requestPaint;
 
 export {
@@ -422,7 +407,7 @@ export {
   unstable_cancelCallback,
   unstable_wrapCallback,
   unstable_getCurrentPriorityLevel,
-  unstable_shouldYield,
+  shouldYieldToHost as unstable_shouldYield,
   unstable_requestPaint,
   unstable_continueExecution,
   unstable_pauseExecution,
diff --git a/packages/scheduler/src/__tests__/Scheduler-test.js b/packages/scheduler/src/__tests__/Scheduler-test.js
index 202501512694c..e8da8ee53d69a 100644
--- a/packages/scheduler/src/__tests__/Scheduler-test.js
+++ b/packages/scheduler/src/__tests__/Scheduler-test.js
@@ -249,7 +249,7 @@ describe('Scheduler', () => {
   });
 
   it(
-    'continuations are interrupted by higher priority work scheduled ' +
+    'continuations do not block higher priority work scheduled ' +
       'inside an executing callback',
     () => {
       const tasks = [
@@ -272,8 +272,8 @@ describe('Scheduler', () => {
               Scheduler.unstable_yieldValue('High pri');
             });
           }
-          if (tasks.length > 0 && shouldYield()) {
-            Scheduler.unstable_yieldValue('Yield!');
+          if (tasks.length > 0) {
+            // Return a continuation
             return work;
           }
         }
@@ -283,9 +283,8 @@ describe('Scheduler', () => {
         'A',
         'B',
         'Schedule high pri',
-        // Even though there's time left in the frame, the low pri callback
-        // should yield to the high pri callback
-        'Yield!',
+        // The high pri callback should fire before the continuation of the
+        // lower pri work
         'High pri',
         // Continue low pri work
         'C',
@@ -662,7 +661,7 @@ describe('Scheduler', () => {
           const [label, ms] = task;
           Scheduler.unstable_advanceTime(ms);
           Scheduler.unstable_yieldValue(label);
-          if (tasks.length > 0 && shouldYield()) {
+          if (tasks.length > 0) {
             return work;
           }
         }