Merge pull request #59 from facebook/main

Update test comments with explanations (facebook#21857)
sthagen · Jul 19, 2021 · 083a00a · 083a00a
2 parents 425a2d3 + 5579f1d
commit 083a00a
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 55 deletions.
diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
@@ -164,34 +164,31 @@ describe('ReactHooksWithNoopRenderer', () => {
     expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
 
     // Schedule some updates
-    if (gate(flags => flags.enableSyncDefaultUpdates)) {
-      React.startTransition(() => {
-        // TODO: Batched updates need to be inside startTransition?
-        ReactNoop.batchedUpdates(() => {
+    act(() => {
+      if (gate(flags => flags.enableSyncDefaultUpdates)) {
+        React.startTransition(() => {
           counter.current.updateCount(1);
           counter.current.updateCount(count => count + 10);
         });
-      });
-    } else {
-      ReactNoop.batchedUpdates(() => {
+      } else {
         counter.current.updateCount(1);
         counter.current.updateCount(count => count + 10);
-      });
-    }
+      }
 
-    // Partially flush without committing
-    expect(Scheduler).toFlushAndYieldThrough(['Count: 11']);
-    expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
+      // Partially flush without committing
+      expect(Scheduler).toFlushAndYieldThrough(['Count: 11']);
+      expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
 
-    // Interrupt with a high priority update
-    ReactNoop.flushSync(() => {
-      ReactNoop.render(<Counter label="Total" />);
-    });
-    expect(Scheduler).toHaveYielded(['Total: 0']);
+      // Interrupt with a high priority update
+      ReactNoop.flushSync(() => {
+        ReactNoop.render(<Counter label="Total" />);
+      });
+      expect(Scheduler).toHaveYielded(['Total: 0']);
 
-    // Resume rendering
-    expect(Scheduler).toFlushAndYield(['Total: 11']);
-    expect(ReactNoop.getChildren()).toEqual([span('Total: 11')]);
+      // Resume rendering
+      expect(Scheduler).toFlushAndYield(['Total: 11']);
+      expect(ReactNoop.getChildren()).toEqual([span('Total: 11')]);
+    });
   });
 
   it('throws inside class components', () => {

diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js
@@ -230,7 +230,7 @@ describe('ReactIncrementalScheduling', () => {
       state = {tick: 0};
 
       componentDidMount() {
-        ReactNoop.deferredUpdates(() => {
+        React.startTransition(() => {
           Scheduler.unstable_yieldValue(
             'componentDidMount (before setState): ' + this.state.tick,
           );
@@ -242,7 +242,7 @@ describe('ReactIncrementalScheduling', () => {
       }
 
       componentDidUpdate() {
-        ReactNoop.deferredUpdates(() => {
+        React.startTransition(() => {
           Scheduler.unstable_yieldValue(
             'componentDidUpdate: ' + this.state.tick,
           );
@@ -280,38 +280,21 @@ describe('ReactIncrementalScheduling', () => {
     expect(Scheduler).toFlushAndYield(['render: 1', 'componentDidUpdate: 1']);
     expect(ReactNoop).toMatchRenderedOutput(<span prop={1} />);
 
-    // Increment the tick to 2. This will trigger an update inside cDU. Flush
-    // the first update without flushing the second one.
-    if (gate(flags => flags.enableSyncDefaultUpdates)) {
-      React.startTransition(() => {
-        instance.setState({tick: 2});
-      });
-
-      // TODO: why does this flush sync?
-      expect(Scheduler).toFlushAndYieldThrough([
-        'render: 2',
-        'componentDidUpdate: 2',
-        'componentDidUpdate (before setState): 2',
-        'componentDidUpdate (after setState): 2',
-        'render: 3',
-        'componentDidUpdate: 3',
-      ]);
-      expect(ReactNoop).toMatchRenderedOutput(<span prop={3} />);
-    } else {
+    React.startTransition(() => {
       instance.setState({tick: 2});
+    });
 
-      expect(Scheduler).toFlushAndYieldThrough([
-        'render: 2',
-        'componentDidUpdate: 2',
-        'componentDidUpdate (before setState): 2',
-        'componentDidUpdate (after setState): 2',
-      ]);
-      expect(ReactNoop).toMatchRenderedOutput(<span prop={2} />);
-
-      // Now flush the cDU update.
-      expect(Scheduler).toFlushAndYield(['render: 3', 'componentDidUpdate: 3']);
-      expect(ReactNoop).toMatchRenderedOutput(<span prop={3} />);
-    }
+    expect(Scheduler).toFlushUntilNextPaint([
+      'render: 2',
+      'componentDidUpdate: 2',
+      'componentDidUpdate (before setState): 2',
+      'componentDidUpdate (after setState): 2',
+    ]);
+    expect(ReactNoop).toMatchRenderedOutput(<span prop={2} />);
+
+    // Now flush the cDU update.
+    expect(Scheduler).toFlushAndYield(['render: 3', 'componentDidUpdate: 3']);
+    expect(ReactNoop).toMatchRenderedOutput(<span prop={3} />);
   });
 
   it('performs Task work even after time runs out', () => {

diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js
@@ -196,11 +196,13 @@ describe('ReactIncrementalUpdates', () => {
       // Now flush the remaining work. Even though e and f were already processed,
       // they should be processed again, to ensure that the terminal state
       // is deterministic.
-      // TODO: should d, e, f be flushed again first?
       expect(Scheduler).toFlushAndYield([
+        // Since 'g' is in a transition, we'll process 'd' separately first.
+        // That causes us to process 'd' with 'e' and 'f' rebased.
         'd',
         'e',
         'f',
+        // Then we'll re-process everything for 'g'.
         'a',
         'b',
         'c',
@@ -290,18 +292,19 @@ describe('ReactIncrementalUpdates', () => {
       });
 
       // The sync updates should have flushed, but not the async ones.
-      // TODO: should 'd' have flushed?
-      // TODO: should 'f' have flushed? I don't know what enqueueReplaceState is.
       expect(Scheduler).toHaveYielded(['e', 'f']);
       expect(ReactNoop.getChildren()).toEqual([span('f')]);
 
       // Now flush the remaining work. Even though e and f were already processed,
       // they should be processed again, to ensure that the terminal state
       // is deterministic.
       expect(Scheduler).toFlushAndYield([
+        // Since 'g' is in a transition, we'll process 'd' separately first.
+        // That causes us to process 'd' with 'e' and 'f' rebased.
         'd',
         'e',
         'f',
+        // Then we'll re-process everything for 'g'.
         'a',
         'b',
         'c',