Use generator-based API for ReactNoop.yield

Author: acdlite

src/renderers/noop/ReactNoop.js

@@ -213,25 +213,36 @@ var roots = new Map();
 var DEFAULT_ROOT_ID = '<default>';
 
 let yieldBeforeNextUnitOfWork = false;
+let yieldValue = null;
 
-function flushUnitsOfWork(n) {
-  yieldBeforeNextUnitOfWork = false;
-  const cb = scheduledDeferredCallback;
-  if (cb !== null) {
+function* flushUnitsOfWork(n: number): Generator<mixed, void, void> {
+  var didStop = false;
+  while (!didStop && scheduledDeferredCallback !== null) {
+    var cb = scheduledDeferredCallback;
     scheduledDeferredCallback = null;
-    let unitsRemaining = n;
+    yieldBeforeNextUnitOfWork = false;
+    yieldValue = null;
+    var unitsRemaining = n;
+    var didYield = false;
     cb({
       timeRemaining() {
         if (yieldBeforeNextUnitOfWork) {
-          yieldBeforeNextUnitOfWork = false;
+          didYield = true;
           return 0;
         }
         if (unitsRemaining-- > 0) {
           return 999;
         }
+        didStop = true;
         return 0;
       },
     });
+
+    if (didYield) {
+      const valueToYield = yieldValue;
+      yieldValue = null;
+      yield valueToYield;
+    }
   }
 }
 
@@ -302,23 +313,44 @@ var ReactNoop = {
   flushDeferredPri(timeout: number = Infinity) {
     // The legacy version of this function decremented the timeout before
     // returning the new time.
-    // TODO: Convert tests to use flushUnitsOfWork instead.
+    // TODO: Convert tests to use flushUnitsOfWork or flushAndYield instead.
     const n = timeout / 5 - 1;
-    flushUnitsOfWork(n);
+    const iterator = flushUnitsOfWork(n);
+    let value = iterator.next();
+    while (!value.done) {
+      value = iterator.next();
+    }
+    // Don't flush animation priority in this legacy function. Some tests may
+    // still rely on this behavior.
   },
 
   flush() {
     ReactNoop.flushAnimationPri();
     ReactNoop.flushDeferredPri();
   },
 
+  *flushAndYield(unitsOfWork: number = Infinity): Generator<mixed, void, void> {
+    for (const value of flushUnitsOfWork(unitsOfWork)) {
+      yield value;
+    }
+    ReactNoop.flushAnimationPri();
+  },
+
   flushUnitsOfWork(n: number) {
-    flushUnitsOfWork(n);
+    const iterator = flushUnitsOfWork(n);
+    let value = iterator.next();
+    while (!value.done) {
+      value = iterator.next();
+    }
+    // TODO: We should always flush animation priority after flushing normal/low
+    // priority. Move this to flushUnitsOfWork generator once tests
+    // are converted.
     ReactNoop.flushAnimationPri();
   },
 
-  yieldBeforeNextUnitOfWork() {
+  yield(value: mixed) {
     yieldBeforeNextUnitOfWork = true;
+    yieldValue = value;
   },
 
   performAnimationWork(fn: Function) {

src/renderers/shared/fiber/__tests__/ReactIncrementalTriangle-test.js

@@ -15,7 +15,7 @@ var React;
 var ReactNoop;
 var ReactFeatureFlags;
 
-describe('ReactConcurrency', () => {
+describe('ReactIncrementalTriangle', () => {
   beforeEach(() => {
     jest.resetModules();
     React = require('react');
@@ -33,7 +33,6 @@ describe('ReactConcurrency', () => {
     let triangles = [];
     let leafTriangles = [];
     let yieldAfterEachRender = false;
-    let lastRenderedTriangle = null;
     class Triangle extends React.Component {
       constructor(props) {
         super();
@@ -64,9 +63,8 @@ describe('ReactConcurrency', () => {
         );
       }
       render() {
-        lastRenderedTriangle = this;
         if (yieldAfterEachRender) {
-          ReactNoop.yieldBeforeNextUnitOfWork();
+          ReactNoop.yield(this);
         }
         const {counter, depth} = this.props;
         if (depth === 0) {
@@ -83,7 +81,7 @@ describe('ReactConcurrency', () => {
       }
     }
 
-    let app;
+    let appInstance;
     class App extends React.Component {
       state = {counter: 0};
       interrupt() {
@@ -98,16 +96,32 @@ describe('ReactConcurrency', () => {
         return currentCounter;
       }
       render() {
-        app = this;
+        appInstance = this;
         return <Triangle counter={this.state.counter} depth={3} />;
       }
     }
 
     const depth = 3;
-    ReactNoop.render(<App depth={depth} />);
-    ReactNoop.flush();
-    // Check initial mount
-    treeIsConsistent(0);
+
+    function reset(nextStep = 0) {
+      triangles = [];
+      leafTriangles = [];
+
+      const previousYieldAfterEachRender = yieldAfterEachRender;
+      yieldAfterEachRender = false;
+      ReactNoop.render([]);
+      ReactNoop.flush();
+      ReactNoop.render(<App depth={depth} />);
+      ReactNoop.flush();
+      // Check initial mount
+      treeIsConsistent(nextStep);
+      yieldAfterEachRender = previousYieldAfterEachRender;
+      return appInstance;
+    }
+
+    reset();
+    const totalChildren = leafTriangles.length;
+    const totalTriangles = triangles.length;
 
     function treeIsConsistent(counter, activeTriangles = new Set()) {
       let activeIndices = [];
@@ -136,15 +150,9 @@ describe('ReactConcurrency', () => {
       }
     }
 
-    function renderNextTriangle() {
-      yieldAfterEachRender = true;
-      lastRenderedTriangle = null;
-      ReactNoop.flush();
-      yieldAfterEachRender = false;
-      return lastRenderedTriangle;
-    }
-
     function step(nextCounter, ...configs) {
+      const app = reset();
+
       const currentCounter = app.setCounter(nextCounter);
 
       const onKeyframes = new Map();
@@ -156,25 +164,28 @@ describe('ReactConcurrency', () => {
           throw new Error('targetIndex should be the index of a leaf triangle');
         }
 
-        const onTriangles = onKeyframes.get(targetIndex) || new Set();
-        onTriangles.add(targetTriangle);
-        onKeyframes.set(onKeyframe, onTriangles);
+        if (onKeyframe >= 0 && onKeyframe < totalTriangles) {
+          const onTriangles = onKeyframes.get(targetIndex) || new Set();
+          onTriangles.add(targetTriangle);
+          onKeyframes.set(onKeyframe, onTriangles);
+        }
 
-        const offTriangles = offKeyframes.get(targetIndex) || new Set();
-        offTriangles.add(targetTriangle);
-        offKeyframes.set(offKeyframe, offTriangles);
+        if (offKeyframe >= 0 && offKeyframe < totalTriangles) {
+          const offTriangles = offKeyframes.get(targetIndex) || new Set();
+          offTriangles.add(targetTriangle);
+          offKeyframes.set(offKeyframe, offTriangles);
+        }
       }
 
       const activeTriangles = new Set();
 
+      yieldAfterEachRender = true;
       let i = 0;
-      let renderedTriangle = renderNextTriangle();
-      while (renderedTriangle !== null) {
+      for (var renderedTriangle of ReactNoop.flushAndYield()) {
         if (i++ > 999) {
           throw new Error('Infinite loop');
         }
         treeIsConsistent(currentCounter, activeTriangles);
-
         var onTriangles = onKeyframes.get(renderedTriangle.index);
         if (onTriangles) {
           onTriangles.forEach(targetTriangle => {
@@ -184,6 +195,7 @@ describe('ReactConcurrency', () => {
             activeTriangles.add(targetTriangle);
             onTriangles.delete(targetTriangle);
           });
+          onKeyframes.delete(renderedTriangle.index);
         }
 
         var offTriangles = offKeyframes.get(renderedTriangle.index);
@@ -195,19 +207,24 @@ describe('ReactConcurrency', () => {
             activeTriangles.delete(targetTriangle);
             offTriangles.delete(targetTriangle);
           });
+          offKeyframes.delete(renderedTriangle.index);
         }
 
         app.interrupt();
         ReactNoop.flushAnimationPri();
         treeIsConsistent(currentCounter, activeTriangles);
+      }
+      yieldAfterEachRender = false;
 
-        renderedTriangle = renderNextTriangle();
+      if (onKeyframes.size !== 0 || offKeyframes.size !== 0) {
+        onKeyframes.forEach(k => console.log(k.size));
+        throw new Error('Some keyframes were not fired.');
       }
 
       treeIsConsistent(nextCounter, activeTriangles);
     }
 
-    return {step};
+    return {step, totalChildren, totalTriangles};
   }
 
   it('works', () => {
@@ -224,12 +241,55 @@ describe('ReactConcurrency', () => {
     // hi-pri update to "deactivate" that same node.
     step(2, [5, 5, 10]);
     step(3, [22, 20, 22]);
-    step(4, [13, 10, 30]);
-    step(5, [7, 35, 38]);
-    step(6, [17, 8, 14]);
 
     // Simulate multiple hover effects in the same step.
-    step(7, [3, 4, 21], [17, 8, 14], [19, 7, 12]);
-    step(8, [3, 4, 39], [17, 8, 14], [19, 7, 12]);
+    step(4, [3, 4, 21], [17, 8, 14], [19, 7, 12]);
+    step(5, [3, 4, 39], [17, 8, 14], [19, 7, 12]);
+
+    // The following tests are test cases that at one point or another failed.
+    // Don't touch them (unless there's a good reason).
+    step(6, [23, 11, 6]);
+    step(7, [7, 4, 3], [4, 14, 6]);
+  });
+
+  it('fuzz tester', () => {
+    // This test is not deterministic because the inputs are randomized. It runs
+    // a limited number of tests on every run. If it fails, it will output the
+    // inputs that led to the failure. Add the failing case to the test above
+    // to prevent future regressions.
+    const limit = 100;
+
+    const {step, totalChildren, totalTriangles} = TriangleTester();
+
+    function randomInteger(min, max) {
+      min = Math.ceil(min);
+      max = Math.floor(max);
+      return Math.floor(Math.random() * (max - min)) + min;
+    }
+
+    function randomConfig() {
+      const targetIndex = randomInteger(0, totalChildren);
+      const onKeyframe = randomInteger(0, totalTriangles);
+      // Allow for end keyframe to happen after entire tree has flushed
+      const offKeyframe = randomInteger(0, totalTriangles * 2);
+      return [targetIndex, onKeyframe, offKeyframe];
+    }
+
+    for (let nextStep = 1; nextStep <= limit; nextStep++) {
+      const configs = [randomConfig(), randomConfig(), randomConfig()];
+      try {
+        step(nextStep, ...configs);
+      } catch (error) {
+        console.error(error);
+        const configStr = configs
+          .map(config => `[${config.join(', ')}]`)
+          .join(', ');
+        throw new Error(
+          `Triangle fuzz tester failure. Add this the ReactIncrementalTriangle-test suite and fix it:
+  step(n, ${configStr})
+`,
+        );
+      }
+    }
   });
 });