Skip to content

Commit

Permalink
Add back accidentally deleted test comments (#26294)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite authored Mar 3, 2023
1 parent b073104 commit 5c633a4
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,11 @@ describe('ReactBlockingMode', () => {
assertLog(['A1', 'B1']);
expect(root).toMatchRenderedOutput('A1B1');
} else {
// Only the second update should have flushed synchronously
assertLog(['B1']);
expect(root).toMatchRenderedOutput('A0B1');

// Now flush the first update
await waitForAll(['A1']);
expect(root).toMatchRenderedOutput('A1B1');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
</>,
);
});
// Inner contents finish in separate commit from outer
assertLog(['Inner']);
expect(root).toMatchRenderedOutput(
<>
Expand Down Expand Up @@ -178,6 +179,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
await act(async () => {
root.render(<App />);
});
// Inner contents finish in separate commit from outer
assertLog(['Outer', 'Loading...', 'Inner [0]']);
expect(root).toMatchRenderedOutput(
<>
Expand All @@ -190,6 +192,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
await act(async () => {
setCount(1);
});
// Entire update finishes in a single commit
assertLog(['Outer', 'Inner [1]']);
expect(root).toMatchRenderedOutput(
<>
Expand Down Expand Up @@ -227,6 +230,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
</>,
);
});
// Inner contents suspended, so we continue showing a fallback.
assertLog(['Suspend! [Inner]']);
expect(root).toMatchRenderedOutput(
<>
Expand Down Expand Up @@ -276,6 +280,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
await act(async () => {
root.render(<App />);
});
// Each level commits separately
assertLog(['A', 'Loading B...', 'B', 'Loading C...', 'C']);
expect(root).toMatchRenderedOutput(
<>
Expand Down
31 changes: 31 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactCache-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ describe('ReactCache', () => {
await act(async () => {
root.render('Bye');
});
// no cleanup: cache is still retained at the root
assertLog([]);
expect(root).toMatchRenderedOutput('Bye');
});
Expand All @@ -245,6 +246,7 @@ describe('ReactCache', () => {
await act(async () => {
root.render('Bye');
});
// no cleanup: cache is still retained at the root
assertLog([]);
expect(root).toMatchRenderedOutput('Bye');
});
Expand Down Expand Up @@ -273,6 +275,8 @@ describe('ReactCache', () => {
root.render(<App showMore={false} />);
});

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

Expand All @@ -285,6 +289,7 @@ describe('ReactCache', () => {
await act(async () => {
root.render('Bye');
});
// no cleanup: cache is still retained at the root
assertLog([]);
expect(root).toMatchRenderedOutput('Bye');
});
Expand Down Expand Up @@ -320,6 +325,8 @@ describe('ReactCache', () => {
await act(async () => {
root.render(<App showMore={true} />);
});
// Even though there are two new <Cache /> trees, they should share the same
// data cache. So there should be only a single cache miss for A.
assertLog(['Cache miss! [A]', 'Loading...', 'Loading...']);
expect(root).toMatchRenderedOutput('Loading...Loading...');

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

Expand All @@ -368,6 +380,7 @@ describe('ReactCache', () => {
await act(async () => {
root.render('Bye');
});
// no cleanup: cache is still retained at the root
assertLog([]);
expect(root).toMatchRenderedOutput('Bye');
},
Expand Down Expand Up @@ -412,6 +425,7 @@ describe('ReactCache', () => {
await act(async () => {
root.render('Bye');
});
// no cleanup: cache is still retained at the root
assertLog([]);
expect(root).toMatchRenderedOutput('Bye');
});
Expand Down Expand Up @@ -468,6 +482,8 @@ describe('ReactCache', () => {
await act(async () => {
root.render('Bye!');
});
// Cleanup occurs for the *second* cache instance: the first is still
// referenced by the root
assertLog(['Cache cleanup: A [v2]']);
expect(root).toMatchRenderedOutput('Bye!');
});
Expand Down Expand Up @@ -549,6 +565,7 @@ describe('ReactCache', () => {
await act(async () => {
root.render('Bye');
});
// no cleanup: cache is still retained at the root
assertLog([]);
expect(root).toMatchRenderedOutput('Bye');
});
Expand Down Expand Up @@ -728,12 +745,14 @@ describe('ReactCache', () => {
await act(async () => {
resolveMostRecentTextCache('A');
});
// Note that the version has updated, and the previous cache is cleared
assertLog(['A [v2]', 'Cache cleanup: A [v1]']);
expect(root).toMatchRenderedOutput('A [v2]');

await act(async () => {
root.render('Bye');
});
// the original root cache already cleaned up when the refresh completed
assertLog([]);
expect(root).toMatchRenderedOutput('Bye');
});
Expand Down Expand Up @@ -781,12 +800,14 @@ describe('ReactCache', () => {
await act(async () => {
resolveMostRecentTextCache('A');
});
// Note that the version has updated, and the previous cache is cleared
assertLog(['A [v2]']);
expect(root).toMatchRenderedOutput('A [v2]');

await act(async () => {
root.render('Bye');
});
// the original root cache already cleaned up when the refresh completed
assertLog([]);
expect(root).toMatchRenderedOutput('Bye');
});
Expand Down Expand Up @@ -841,12 +862,15 @@ describe('ReactCache', () => {
}),
);
});
// The root should re-render without a cache miss.
// The cache is not cleared up yet, since it's still reference by the root
assertLog(['A [v2]']);
expect(root).toMatchRenderedOutput('A [v2]');

await act(async () => {
root.render('Bye');
});
// the refreshed cache boundary is unmounted and cleans up
assertLog(['Cache cleanup: A [v2]']);
expect(root).toMatchRenderedOutput('Bye');
});
Expand Down Expand Up @@ -920,6 +944,8 @@ describe('ReactCache', () => {
await act(async () => {
root.render('Bye!');
});
// Unmounting children releases the refreshed cache instance only; the root
// still retains the original cache instance used for the first render
assertLog(['Cache cleanup: A [v3]']);
expect(root).toMatchRenderedOutput('Bye!');
});
Expand Down Expand Up @@ -967,6 +993,8 @@ describe('ReactCache', () => {
root.render(<App showMore={true} />);
});

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

Expand Down Expand Up @@ -1064,6 +1092,9 @@ describe('ReactCache', () => {
await act(async () => {
root.render('Bye!');
});
// Unmounting children releases both cache boundaries, but the original
// cache instance (used by second boundary) is still referenced by the root.
// only the second cache instance is freed.
assertLog(['Cache cleanup: A [v2]']);
expect(root).toMatchRenderedOutput('Bye!');
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,11 @@ describe('ReactLazyContextPropagation', () => {
setOtherValue(1);
setOtherValue(0);
});
// NOTE: If this didn't yield anything, that indicates that we never visited
// the consumer during the render phase, which probably means the eager
// bailout mechanism kicked in. Because we're testing the _lazy_ bailout
// mechanism, update this test to foil the _eager_ bailout, somehow. Perhaps
// by switching to useReducer.
assertLog(['Consumer']);
expect(root).toMatchRenderedOutput('0');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ describe('ReactDeferredValue', () => {
root.render(<App value={2} />);

await waitForPaint(['Original: 2']);
// The deferred value updates in a separate render
await waitForPaint(['Deferred: 2']);
});
expect(root).toMatchRenderedOutput(
Expand All @@ -92,6 +93,7 @@ describe('ReactDeferredValue', () => {
startTransition(() => {
root.render(<App value={3} />);
});
// The deferred value updates in the same render as the original
await waitForPaint(['Original: 3', 'Deferred: 3']);
});
expect(root).toMatchRenderedOutput(
Expand Down Expand Up @@ -137,6 +139,7 @@ describe('ReactDeferredValue', () => {
root.render(<App value={2} />);

await waitForPaint(['Original: 2']);
// The deferred value updates in a separate render
await waitForPaint(['Deferred: 2']);
});
expect(root).toMatchRenderedOutput(
Expand All @@ -151,6 +154,7 @@ describe('ReactDeferredValue', () => {
startTransition(() => {
root.render(<App value={3} />);
});
// The deferred value updates in the same render as the original
await waitForPaint(['Original: 3', 'Deferred: 3']);
});
expect(root).toMatchRenderedOutput(
Expand Down Expand Up @@ -201,6 +205,7 @@ describe('ReactDeferredValue', () => {
root.render(<App value={2} />);

await waitForPaint(['Original: 2']);
// The deferred value updates in a separate render
await waitForPaint(['Deferred: 2']);
});
expect(root).toMatchRenderedOutput(
Expand All @@ -215,6 +220,7 @@ describe('ReactDeferredValue', () => {
startTransition(() => {
root.render(<App value={3} />);
});
// The deferred value updates in the same render as the original
await waitForPaint(['Original: 3', 'Deferred: 3']);
});
expect(root).toMatchRenderedOutput(
Expand Down Expand Up @@ -270,6 +276,9 @@ describe('ReactDeferredValue', () => {
startTransition(() => {
root.render(<App value={2} />);
});
// In the regression, the memoized value was not updated during non-urgent
// updates, so this would flip the deferred value back to the initial
// value (1) instead of reusing the current one (2).
await waitForPaint(['Original: 2', 'Deferred: 2']);
expect(root).toMatchRenderedOutput(
<div>
Expand Down
14 changes: 14 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactExpiration-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,17 @@ describe('ReactExpiration', () => {
});
// Advance the timer.
Scheduler.unstable_advanceTime(2000);
// Partially flush the first update, then interrupt it.
await waitFor(['A [render]']);
interrupt();

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

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

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

Expand Down Expand Up @@ -223,14 +228,17 @@ describe('ReactExpiration', () => {
});
// Advance the timer.
Scheduler.unstable_advanceTime(2000);
// Partially flush the first update, then interrupt it.
await waitFor(['A [render]']);
interrupt();

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

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

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

// Schedule another update.
ReactNoop.render(<TextClass text="B" />);
// The updates should flush in the same batch, since as far as the scheduler
// knows, they may have occurred inside the same event.
await waitForAll(['B [render]', 'B [commit]']);
},
);
Expand Down Expand Up @@ -471,7 +481,9 @@ describe('ReactExpiration', () => {
// In other words, we can flush just the first child without flushing
// the rest.
Scheduler.unstable_flushNumberOfYields(1);
// Yield right after first child.
assertLog(['Sync pri: 1']);
// Now do the rest.
await waitForAll(['Normal pri: 1']);
});
expect(root).toMatchRenderedOutput('Sync pri: 1, Normal pri: 1');
Expand Down Expand Up @@ -533,6 +545,7 @@ describe('ReactExpiration', () => {
await waitFor(['Sync pri: 0']);
updateSyncPri();
});
// Same thing should happen as last time
assertLog([
// Interrupt idle update to render sync update
'Sync pri: 1',
Expand Down Expand Up @@ -733,6 +746,7 @@ describe('ReactExpiration', () => {
Scheduler.unstable_flushNumberOfYields(1);
assertLog(['A1', 'B1', 'C1']);
});
// The effect flushes after paint.
assertLog(['Effect: 1']);
});
});
Loading

0 comments on commit 5c633a4

Please sign in to comment.