Skip to content

Commit 5c633a4

Browse files
authored
Add back accidentally deleted test comments (#26294)
The codemod I used in #26288 accidentally caused some comments to be deleted. Because not all affected lines included comments, I didn't notice until after landing. This adds the comments back.
1 parent b073104 commit 5c633a4

9 files changed

+124
-0
lines changed

packages/react-reconciler/src/__tests__/ReactBatching-test.internal.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,11 @@ describe('ReactBlockingMode', () => {
166166
assertLog(['A1', 'B1']);
167167
expect(root).toMatchRenderedOutput('A1B1');
168168
} else {
169+
// Only the second update should have flushed synchronously
169170
assertLog(['B1']);
170171
expect(root).toMatchRenderedOutput('A0B1');
171172

173+
// Now flush the first update
172174
await waitForAll(['A1']);
173175
expect(root).toMatchRenderedOutput('A1B1');
174176
}

packages/react-reconciler/src/__tests__/ReactCPUSuspense-test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
143143
</>,
144144
);
145145
});
146+
// Inner contents finish in separate commit from outer
146147
assertLog(['Inner']);
147148
expect(root).toMatchRenderedOutput(
148149
<>
@@ -178,6 +179,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
178179
await act(async () => {
179180
root.render(<App />);
180181
});
182+
// Inner contents finish in separate commit from outer
181183
assertLog(['Outer', 'Loading...', 'Inner [0]']);
182184
expect(root).toMatchRenderedOutput(
183185
<>
@@ -190,6 +192,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
190192
await act(async () => {
191193
setCount(1);
192194
});
195+
// Entire update finishes in a single commit
193196
assertLog(['Outer', 'Inner [1]']);
194197
expect(root).toMatchRenderedOutput(
195198
<>
@@ -227,6 +230,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
227230
</>,
228231
);
229232
});
233+
// Inner contents suspended, so we continue showing a fallback.
230234
assertLog(['Suspend! [Inner]']);
231235
expect(root).toMatchRenderedOutput(
232236
<>
@@ -276,6 +280,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
276280
await act(async () => {
277281
root.render(<App />);
278282
});
283+
// Each level commits separately
279284
assertLog(['A', 'Loading B...', 'B', 'Loading C...', 'C']);
280285
expect(root).toMatchRenderedOutput(
281286
<>

packages/react-reconciler/src/__tests__/ReactCache-test.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ describe('ReactCache', () => {
219219
await act(async () => {
220220
root.render('Bye');
221221
});
222+
// no cleanup: cache is still retained at the root
222223
assertLog([]);
223224
expect(root).toMatchRenderedOutput('Bye');
224225
});
@@ -245,6 +246,7 @@ describe('ReactCache', () => {
245246
await act(async () => {
246247
root.render('Bye');
247248
});
249+
// no cleanup: cache is still retained at the root
248250
assertLog([]);
249251
expect(root).toMatchRenderedOutput('Bye');
250252
});
@@ -273,6 +275,8 @@ describe('ReactCache', () => {
273275
root.render(<App showMore={false} />);
274276
});
275277

278+
// Even though there are two new <Cache /> trees, they should share the same
279+
// data cache. So there should be only a single cache miss for A.
276280
assertLog(['Cache miss! [A]', 'Loading...', 'Loading...']);
277281
expect(root).toMatchRenderedOutput('Loading...Loading...');
278282

@@ -285,6 +289,7 @@ describe('ReactCache', () => {
285289
await act(async () => {
286290
root.render('Bye');
287291
});
292+
// no cleanup: cache is still retained at the root
288293
assertLog([]);
289294
expect(root).toMatchRenderedOutput('Bye');
290295
});
@@ -320,6 +325,8 @@ describe('ReactCache', () => {
320325
await act(async () => {
321326
root.render(<App showMore={true} />);
322327
});
328+
// Even though there are two new <Cache /> trees, they should share the same
329+
// data cache. So there should be only a single cache miss for A.
323330
assertLog(['Cache miss! [A]', 'Loading...', 'Loading...']);
324331
expect(root).toMatchRenderedOutput('Loading...Loading...');
325332

@@ -332,6 +339,9 @@ describe('ReactCache', () => {
332339
await act(async () => {
333340
root.render('Bye');
334341
});
342+
// cleanup occurs for the cache shared by the inner cache boundaries (which
343+
// are not shared w the root because they were added in an update)
344+
// note that no cache is created for the root since the cache is never accessed
335345
assertLog(['Cache cleanup: A [v1]']);
336346
expect(root).toMatchRenderedOutput('Bye');
337347
});
@@ -356,6 +366,8 @@ describe('ReactCache', () => {
356366
await act(async () => {
357367
root.render(<App />);
358368
});
369+
// Even though there is a nested <Cache /> boundary, it should share the same
370+
// data cache as the root. So there should be only a single cache miss for A.
359371
assertLog(['Cache miss! [A]', 'Loading...']);
360372
expect(root).toMatchRenderedOutput('Loading...');
361373

@@ -368,6 +380,7 @@ describe('ReactCache', () => {
368380
await act(async () => {
369381
root.render('Bye');
370382
});
383+
// no cleanup: cache is still retained at the root
371384
assertLog([]);
372385
expect(root).toMatchRenderedOutput('Bye');
373386
},
@@ -412,6 +425,7 @@ describe('ReactCache', () => {
412425
await act(async () => {
413426
root.render('Bye');
414427
});
428+
// no cleanup: cache is still retained at the root
415429
assertLog([]);
416430
expect(root).toMatchRenderedOutput('Bye');
417431
});
@@ -468,6 +482,8 @@ describe('ReactCache', () => {
468482
await act(async () => {
469483
root.render('Bye!');
470484
});
485+
// Cleanup occurs for the *second* cache instance: the first is still
486+
// referenced by the root
471487
assertLog(['Cache cleanup: A [v2]']);
472488
expect(root).toMatchRenderedOutput('Bye!');
473489
});
@@ -549,6 +565,7 @@ describe('ReactCache', () => {
549565
await act(async () => {
550566
root.render('Bye');
551567
});
568+
// no cleanup: cache is still retained at the root
552569
assertLog([]);
553570
expect(root).toMatchRenderedOutput('Bye');
554571
});
@@ -728,12 +745,14 @@ describe('ReactCache', () => {
728745
await act(async () => {
729746
resolveMostRecentTextCache('A');
730747
});
748+
// Note that the version has updated, and the previous cache is cleared
731749
assertLog(['A [v2]', 'Cache cleanup: A [v1]']);
732750
expect(root).toMatchRenderedOutput('A [v2]');
733751

734752
await act(async () => {
735753
root.render('Bye');
736754
});
755+
// the original root cache already cleaned up when the refresh completed
737756
assertLog([]);
738757
expect(root).toMatchRenderedOutput('Bye');
739758
});
@@ -781,12 +800,14 @@ describe('ReactCache', () => {
781800
await act(async () => {
782801
resolveMostRecentTextCache('A');
783802
});
803+
// Note that the version has updated, and the previous cache is cleared
784804
assertLog(['A [v2]']);
785805
expect(root).toMatchRenderedOutput('A [v2]');
786806

787807
await act(async () => {
788808
root.render('Bye');
789809
});
810+
// the original root cache already cleaned up when the refresh completed
790811
assertLog([]);
791812
expect(root).toMatchRenderedOutput('Bye');
792813
});
@@ -841,12 +862,15 @@ describe('ReactCache', () => {
841862
}),
842863
);
843864
});
865+
// The root should re-render without a cache miss.
866+
// The cache is not cleared up yet, since it's still reference by the root
844867
assertLog(['A [v2]']);
845868
expect(root).toMatchRenderedOutput('A [v2]');
846869

847870
await act(async () => {
848871
root.render('Bye');
849872
});
873+
// the refreshed cache boundary is unmounted and cleans up
850874
assertLog(['Cache cleanup: A [v2]']);
851875
expect(root).toMatchRenderedOutput('Bye');
852876
});
@@ -920,6 +944,8 @@ describe('ReactCache', () => {
920944
await act(async () => {
921945
root.render('Bye!');
922946
});
947+
// Unmounting children releases the refreshed cache instance only; the root
948+
// still retains the original cache instance used for the first render
923949
assertLog(['Cache cleanup: A [v3]']);
924950
expect(root).toMatchRenderedOutput('Bye!');
925951
});
@@ -967,6 +993,8 @@ describe('ReactCache', () => {
967993
root.render(<App showMore={true} />);
968994
});
969995

996+
// Even though there are two new <Cache /> trees, they should share the same
997+
// data cache. So there should be only a single cache miss for A.
970998
assertLog(['Cache miss! [A]', 'Loading...', 'Loading...']);
971999
expect(root).toMatchRenderedOutput('Loading...Loading...');
9721000

@@ -1064,6 +1092,9 @@ describe('ReactCache', () => {
10641092
await act(async () => {
10651093
root.render('Bye!');
10661094
});
1095+
// Unmounting children releases both cache boundaries, but the original
1096+
// cache instance (used by second boundary) is still referenced by the root.
1097+
// only the second cache instance is freed.
10671098
assertLog(['Cache cleanup: A [v2]']);
10681099
expect(root).toMatchRenderedOutput('Bye!');
10691100
},

packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,11 @@ describe('ReactLazyContextPropagation', () => {
339339
setOtherValue(1);
340340
setOtherValue(0);
341341
});
342+
// NOTE: If this didn't yield anything, that indicates that we never visited
343+
// the consumer during the render phase, which probably means the eager
344+
// bailout mechanism kicked in. Because we're testing the _lazy_ bailout
345+
// mechanism, update this test to foil the _eager_ bailout, somehow. Perhaps
346+
// by switching to useReducer.
342347
assertLog(['Consumer']);
343348
expect(root).toMatchRenderedOutput('0');
344349
});

packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ describe('ReactDeferredValue', () => {
7878
root.render(<App value={2} />);
7979

8080
await waitForPaint(['Original: 2']);
81+
// The deferred value updates in a separate render
8182
await waitForPaint(['Deferred: 2']);
8283
});
8384
expect(root).toMatchRenderedOutput(
@@ -92,6 +93,7 @@ describe('ReactDeferredValue', () => {
9293
startTransition(() => {
9394
root.render(<App value={3} />);
9495
});
96+
// The deferred value updates in the same render as the original
9597
await waitForPaint(['Original: 3', 'Deferred: 3']);
9698
});
9799
expect(root).toMatchRenderedOutput(
@@ -137,6 +139,7 @@ describe('ReactDeferredValue', () => {
137139
root.render(<App value={2} />);
138140

139141
await waitForPaint(['Original: 2']);
142+
// The deferred value updates in a separate render
140143
await waitForPaint(['Deferred: 2']);
141144
});
142145
expect(root).toMatchRenderedOutput(
@@ -151,6 +154,7 @@ describe('ReactDeferredValue', () => {
151154
startTransition(() => {
152155
root.render(<App value={3} />);
153156
});
157+
// The deferred value updates in the same render as the original
154158
await waitForPaint(['Original: 3', 'Deferred: 3']);
155159
});
156160
expect(root).toMatchRenderedOutput(
@@ -201,6 +205,7 @@ describe('ReactDeferredValue', () => {
201205
root.render(<App value={2} />);
202206

203207
await waitForPaint(['Original: 2']);
208+
// The deferred value updates in a separate render
204209
await waitForPaint(['Deferred: 2']);
205210
});
206211
expect(root).toMatchRenderedOutput(
@@ -215,6 +220,7 @@ describe('ReactDeferredValue', () => {
215220
startTransition(() => {
216221
root.render(<App value={3} />);
217222
});
223+
// The deferred value updates in the same render as the original
218224
await waitForPaint(['Original: 3', 'Deferred: 3']);
219225
});
220226
expect(root).toMatchRenderedOutput(
@@ -270,6 +276,9 @@ describe('ReactDeferredValue', () => {
270276
startTransition(() => {
271277
root.render(<App value={2} />);
272278
});
279+
// In the regression, the memoized value was not updated during non-urgent
280+
// updates, so this would flip the deferred value back to the initial
281+
// value (1) instead of reusing the current one (2).
273282
await waitForPaint(['Original: 2', 'Deferred: 2']);
274283
expect(root).toMatchRenderedOutput(
275284
<div>

packages/react-reconciler/src/__tests__/ReactExpiration-test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,14 +171,17 @@ describe('ReactExpiration', () => {
171171
});
172172
// Advance the timer.
173173
Scheduler.unstable_advanceTime(2000);
174+
// Partially flush the first update, then interrupt it.
174175
await waitFor(['A [render]']);
175176
interrupt();
176177

178+
// Don't advance time by enough to expire the first update.
177179
assertLog([]);
178180
expect(ReactNoop).toMatchRenderedOutput(null);
179181

180182
// Schedule another update.
181183
ReactNoop.render(<TextClass text="B" />);
184+
// Both updates are batched
182185
await waitForAll(['B [render]', 'B [commit]']);
183186
expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);
184187

@@ -190,6 +193,8 @@ describe('ReactExpiration', () => {
190193
expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);
191194
// Schedule another update.
192195
ReactNoop.render(<TextClass text="B" />);
196+
// The updates should flush in the same batch, since as far as the scheduler
197+
// knows, they may have occurred inside the same event.
193198
await waitForAll(['B [render]', 'B [commit]']);
194199
});
195200

@@ -223,14 +228,17 @@ describe('ReactExpiration', () => {
223228
});
224229
// Advance the timer.
225230
Scheduler.unstable_advanceTime(2000);
231+
// Partially flush the first update, then interrupt it.
226232
await waitFor(['A [render]']);
227233
interrupt();
228234

235+
// Don't advance time by enough to expire the first update.
229236
assertLog([]);
230237
expect(ReactNoop).toMatchRenderedOutput(null);
231238

232239
// Schedule another update.
233240
ReactNoop.render(<TextClass text="B" />);
241+
// Both updates are batched
234242
await waitForAll(['B [render]', 'B [commit]']);
235243
expect(ReactNoop).toMatchRenderedOutput(<span prop="B" />);
236244

@@ -247,6 +255,8 @@ describe('ReactExpiration', () => {
247255

248256
// Schedule another update.
249257
ReactNoop.render(<TextClass text="B" />);
258+
// The updates should flush in the same batch, since as far as the scheduler
259+
// knows, they may have occurred inside the same event.
250260
await waitForAll(['B [render]', 'B [commit]']);
251261
},
252262
);
@@ -471,7 +481,9 @@ describe('ReactExpiration', () => {
471481
// In other words, we can flush just the first child without flushing
472482
// the rest.
473483
Scheduler.unstable_flushNumberOfYields(1);
484+
// Yield right after first child.
474485
assertLog(['Sync pri: 1']);
486+
// Now do the rest.
475487
await waitForAll(['Normal pri: 1']);
476488
});
477489
expect(root).toMatchRenderedOutput('Sync pri: 1, Normal pri: 1');
@@ -533,6 +545,7 @@ describe('ReactExpiration', () => {
533545
await waitFor(['Sync pri: 0']);
534546
updateSyncPri();
535547
});
548+
// Same thing should happen as last time
536549
assertLog([
537550
// Interrupt idle update to render sync update
538551
'Sync pri: 1',
@@ -733,6 +746,7 @@ describe('ReactExpiration', () => {
733746
Scheduler.unstable_flushNumberOfYields(1);
734747
assertLog(['A1', 'B1', 'C1']);
735748
});
749+
// The effect flushes after paint.
736750
assertLog(['Effect: 1']);
737751
});
738752
});

0 commit comments

Comments
 (0)