From bff5fe5a8555c40ee20f320f9ca3108410e9ee10 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Mon, 12 Jul 2021 18:06:27 -0400 Subject: [PATCH] Update test comments with explanations --- .../src/__tests__/ReactHooksWithNoopRenderer-test.js | 1 - .../src/__tests__/ReactIncrementalScheduling-test.js | 2 +- .../src/__tests__/ReactIncrementalUpdates-test.js | 9 ++++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 7de5232934486..f04d35f329372 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -166,7 +166,6 @@ describe('ReactHooksWithNoopRenderer', () => { // Schedule some updates if (gate(flags => flags.enableSyncDefaultUpdates)) { React.startTransition(() => { - // TODO: Batched updates need to be inside startTransition? ReactNoop.batchedUpdates(() => { counter.current.updateCount(1); counter.current.updateCount(count => count + 10); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js index 9c94900851475..4230faa511f32 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js @@ -287,12 +287,12 @@ describe('ReactIncrementalScheduling', () => { 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', + // This renders because the scheduled update is default, which is sync. 'render: 3', 'componentDidUpdate: 3', ]); 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',