From c7656b3f37df0274e36524a43ad0716a10decfc5 Mon Sep 17 00:00:00 2001 From: Ricky Date: Mon, 19 Jul 2021 15:07:38 -0400 Subject: [PATCH] Update test comments with explanations (#21857) --- .../ReactHooksWithNoopRenderer-test.js | 37 +++++++-------- .../ReactIncrementalScheduling-test.js | 47 ++++++------------- .../__tests__/ReactIncrementalUpdates-test.js | 9 ++-- 3 files changed, 38 insertions(+), 55 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 7de5232934486..8fe9b19815eb8 100644 --- 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(); - }); - expect(Scheduler).toHaveYielded(['Total: 0']); + // Interrupt with a high priority update + ReactNoop.flushSync(() => { + ReactNoop.render(); + }); + 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 index 9c94900851475..a16df14067dea 100644 --- 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(); - // 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(); - } 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(); - - // Now flush the cDU update. - expect(Scheduler).toFlushAndYield(['render: 3', 'componentDidUpdate: 3']); - expect(ReactNoop).toMatchRenderedOutput(); - } + expect(Scheduler).toFlushUntilNextPaint([ + 'render: 2', + 'componentDidUpdate: 2', + 'componentDidUpdate (before setState): 2', + 'componentDidUpdate (after setState): 2', + ]); + expect(ReactNoop).toMatchRenderedOutput(); + + // Now flush the cDU update. + expect(Scheduler).toFlushAndYield(['render: 3', 'componentDidUpdate: 3']); + expect(ReactNoop).toMatchRenderedOutput(); }); 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 index 200d6453010bd..b507076e9e910 100644 --- 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,8 +292,6 @@ 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')]); @@ -299,9 +299,12 @@ describe('ReactIncrementalUpdates', () => { // 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',