Remove didTimeout check from work loop

acdlite · acdlite · commit 3f8115cdd1e6 · 2020-08-14T10:45:24.000-04:00
No longer need this, since we have starvation protection in userspace.

This will also allow us to remove the concept from the Scheduler
package, which is nice because `postTask` doesn't currently support it.
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -754,7 +754,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
 
 // This is the entry point for every concurrent task, i.e. anything that
 // goes through Scheduler.
-function performConcurrentWorkOnRoot(root, didTimeout) {
+function performConcurrentWorkOnRoot(root) {
   // Since we know we're in a React event, we can clear the current
   // event time. The next update will compute a new event time.
   currentEventTime = NoTimestamp;
@@ -794,19 +794,6 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
     return null;
   }
 
-  // TODO: We only check `didTimeout` defensively, to account for a Scheduler
-  // bug where `shouldYield` sometimes returns `true` even if `didTimeout` is
-  // true, which leads to an infinite loop. Once the bug in Scheduler is
-  // fixed, we can remove this, since we track expiration ourselves.
-  if (didTimeout) {
-    // Something expired. Flush synchronously until there's no expired
-    // work left.
-    markRootExpired(root, lanes);
-    // This will schedule a synchronous callback.
-    ensureRootIsScheduled(root, now());
-    return null;
-  }
-
   let exitStatus = renderRootConcurrent(root, lanes);
 
   if (
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -741,7 +741,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
 
 // This is the entry point for every concurrent task, i.e. anything that
 // goes through Scheduler.
-function performConcurrentWorkOnRoot(root, didTimeout) {
+function performConcurrentWorkOnRoot(root) {
   // Since we know we're in a React event, we can clear the current
   // event time. The next update will compute a new event time.
   currentEventTime = NoTimestamp;
@@ -781,19 +781,6 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
     return null;
   }
 
-  // TODO: We only check `didTimeout` defensively, to account for a Scheduler
-  // bug where `shouldYield` sometimes returns `true` even if `didTimeout` is
-  // true, which leads to an infinite loop. Once the bug in Scheduler is
-  // fixed, we can remove this, since we track expiration ourselves.
-  if (didTimeout) {
-    // Something expired. Flush synchronously until there's no expired
-    // work left.
-    markRootExpired(root, lanes);
-    // This will schedule a synchronous callback.
-    ensureRootIsScheduled(root, now());
-    return null;
-  }
-
   let exitStatus = renderRootConcurrent(root, lanes);
 
   if (
diff --git a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js
@@ -446,8 +446,6 @@ describe(
       React = require('react');
       ReactNoop = require('react-noop-renderer');
       Scheduler = require('scheduler');
-
-      React = require('react');
     });
 
     afterEach(() => {
@@ -531,6 +529,14 @@ describe(
 
         // Expire the task
         Scheduler.unstable_advanceTime(10000);
+        // Scheduling a new update is a trick to force the expiration to kick
+        // in. We don't check if a update has been starved at the beginning of
+        // working on it, since there's no point — we're already working on it.
+        // We only check before yielding to the main thread (to avoid starvation
+        // by other main thread work) or when receiving an update (to avoid
+        // starvation by incoming updates).
+        ReactNoop.render(<App />);
+
         // Because the render expired, React should finish the tree without
         // consulting `shouldYield` again
         expect(Scheduler).toFlushExpired(['B', 'C']);
