Skip to content

Commit

Permalink
Revert "Start prerendering Suspense retries immediately (facebook#30934…
Browse files Browse the repository at this point in the history 
…)"

This reverts commit d6cb4e7.
  • Loading branch information
Jack Pope committed Sep 18, 2024
1 parent e72127a commit 89c9014
Show file tree
Hide file tree
Showing 29 changed files with 463 additions and 1,899 deletions.
98 changes: 45 additions & 53 deletions packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ let Suspense;
let TextResource;
let textResourceShouldFail;
let waitForAll;
let waitForPaint;
let assertLog;
let waitForThrow;
let act;
Expand All @@ -38,7 +37,6 @@ describe('ReactCache', () => {
waitForAll = InternalTestUtils.waitForAll;
assertLog = InternalTestUtils.assertLog;
waitForThrow = InternalTestUtils.waitForThrow;
waitForPaint = InternalTestUtils.waitForPaint;
act = InternalTestUtils.act;

TextResource = createResource(
Expand Down Expand Up @@ -121,12 +119,7 @@ describe('ReactCache', () => {
const root = ReactNoop.createRoot();
root.render(<App />);

await waitForAll([
'Suspend! [Hi]',
'Loading...',

...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []),
]);
await waitForAll(['Suspend! [Hi]', 'Loading...']);

jest.advanceTimersByTime(100);
assertLog(['Promise resolved [Hi]']);
Expand All @@ -145,12 +138,7 @@ describe('ReactCache', () => {
const root = ReactNoop.createRoot();
root.render(<App />);

await waitForAll([
'Suspend! [Hi]',
'Loading...',

...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []),
]);
await waitForAll(['Suspend! [Hi]', 'Loading...']);

textResourceShouldFail = true;
let error;
Expand All @@ -160,7 +148,15 @@ describe('ReactCache', () => {
error = e;
}
expect(error.message).toMatch('Failed to load: Hi');
assertLog(['Promise rejected [Hi]', 'Error! [Hi]', 'Error! [Hi]']);
assertLog([
'Promise rejected [Hi]',
'Error! [Hi]',
'Error! [Hi]',

...(gate('enableSiblingPrerendering')
? ['Error! [Hi]', 'Error! [Hi]']
: []),
]);

// Should throw again on a subsequent read
root.render(<App />);
Expand Down Expand Up @@ -191,27 +187,15 @@ describe('ReactCache', () => {

if (__DEV__) {
await expect(async () => {
await waitForAll([
'App',
'Loading...',

...(gate('enableSiblingPrerendering') ? ['App'] : []),
]);
await waitForAll(['App', 'Loading...']);
}).toErrorDev([
'Invalid key type. Expected a string, number, symbol, or ' +
"boolean, but instead received: [ 'Hi', 100 ]\n\n" +
'To use non-primitive values as keys, you must pass a hash ' +
'function as the second argument to createResource().',

...(gate('enableSiblingPrerendering') ? ['Invalid key type'] : []),
]);
} else {
await waitForAll([
'App',
'Loading...',

...(gate('enableSiblingPrerendering') ? ['App'] : []),
]);
await waitForAll(['App', 'Loading...']);
}
});

Expand All @@ -228,17 +212,13 @@ describe('ReactCache', () => {
<AsyncText ms={100} text={3} />
</Suspense>,
);
await waitForPaint(['Suspend! [1]', 'Loading...']);
await waitForAll(['Suspend! [1]', 'Loading...']);
jest.advanceTimersByTime(100);
assertLog(['Promise resolved [1]']);
await waitForAll([1, 'Suspend! [2]']);

jest.advanceTimersByTime(100);
assertLog(['Promise resolved [2]']);
await waitForAll([1, 2, 'Suspend! [3]']);
await waitForAll([1, 'Suspend! [2]', 1, 'Suspend! [2]', 'Suspend! [3]']);

jest.advanceTimersByTime(100);
assertLog(['Promise resolved [3]']);
assertLog(['Promise resolved [2]', 'Promise resolved [3]']);
await waitForAll([1, 2, 3]);

await act(() => jest.advanceTimersByTime(100));
Expand All @@ -253,17 +233,24 @@ describe('ReactCache', () => {
</Suspense>,
);

await waitForAll([
await waitForAll([1, 'Suspend! [4]', 'Loading...']);

await act(() => jest.advanceTimersByTime(100));
assertLog([
'Promise resolved [4]',

1,
'Suspend! [4]',
'Loading...',
4,
'Suspend! [5]',
1,
'Suspend! [4]',
4,
'Suspend! [5]',
]);

await act(() => jest.advanceTimersByTime(100));
assertLog(['Promise resolved [4]', 'Promise resolved [5]', 1, 4, 5]);
'Promise resolved [5]',
1,
4,
5,
]);

expect(root).toMatchRenderedOutput('145');

Expand All @@ -284,14 +271,24 @@ describe('ReactCache', () => {
// 2 and 3 suspend because they were evicted from the cache
'Suspend! [2]',
'Loading...',
]);

await act(() => jest.advanceTimersByTime(100));
assertLog([
'Promise resolved [2]',

1,
'Suspend! [2]',
2,
'Suspend! [3]',
1,
2,
'Suspend! [3]',
]);

await act(() => jest.advanceTimersByTime(100));
assertLog(['Promise resolved [2]', 'Promise resolved [3]', 1, 2, 3]);
'Promise resolved [3]',
1,
2,
3,
]);
expect(root).toMatchRenderedOutput('123');
});

Expand Down Expand Up @@ -366,12 +363,7 @@ describe('ReactCache', () => {
</Suspense>,
);

await waitForAll([
'Suspend! [Hi]',
'Loading...',

...(gate('enableSiblingPrerendering') ? ['Suspend! [Hi]'] : []),
]);
await waitForAll(['Suspend! [Hi]', 'Loading...']);

resolveThenable('Hi');
// This thenable improperly resolves twice. We should not update the
Expand Down
97 changes: 10 additions & 87 deletions packages/react-devtools-shared/src/__tests__/TimelineProfiler-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ let utils;
// TODO: This is how other DevTools tests access the version but we should find
// a better solution for this
const ReactVersionTestingAgainst = process.env.REACT_VERSION || ReactVersion;
// Disabling this while the flag is off in experimental. Leaving the logic so we can
// restore the behavior when we turn the flag back on.
const enableSiblingPrerendering =
false && semver.gte(ReactVersionTestingAgainst, '19.0.0');

// This flag is on experimental which disables timeline profiler.
const enableComponentPerformanceTrack =
Expand Down Expand Up @@ -1672,11 +1668,7 @@ describe('Timeline profiler', () => {
</React.Suspense>,
);

await waitForAll([
'suspended',

...(enableSiblingPrerendering ? ['suspended'] : []),
]);
await waitForAll(['suspended']);

Scheduler.unstable_advanceTime(10);
resolveFn();
Expand All @@ -1687,38 +1679,9 @@ describe('Timeline profiler', () => {
const timelineData = stopProfilingAndGetTimelineData();

// Verify the Suspense event and duration was recorded.
if (enableSiblingPrerendering) {
expect(timelineData.suspenseEvents).toMatchInlineSnapshot(`
[
{
"componentName": "Example",
"depth": 0,
"duration": 10,
"id": "0",
"phase": "mount",
"promiseName": "",
"resolution": "resolved",
"timestamp": 10,
"type": "suspense",
"warning": null,
},
{
"componentName": "Example",
"depth": 0,
"duration": 10,
"id": "0",
"phase": "mount",
"promiseName": "",
"resolution": "resolved",
"timestamp": 10,
"type": "suspense",
"warning": null,
},
]
`);
} else {
const suspenseEvent = timelineData.suspenseEvents[0];
expect(suspenseEvent).toMatchInlineSnapshot(`
expect(timelineData.suspenseEvents).toHaveLength(1);
const suspenseEvent = timelineData.suspenseEvents[0];
expect(suspenseEvent).toMatchInlineSnapshot(`
{
"componentName": "Example",
"depth": 0,
Expand All @@ -1732,13 +1695,10 @@ describe('Timeline profiler', () => {
"warning": null,
}
`);
}

// There should be two batches of renders: Suspeneded and resolved.
expect(timelineData.batchUIDToMeasuresMap.size).toBe(2);
expect(timelineData.componentMeasures).toHaveLength(
enableSiblingPrerendering ? 3 : 2,
);
expect(timelineData.componentMeasures).toHaveLength(2);
});

it('should mark concurrent render with suspense that rejects', async () => {
Expand All @@ -1765,11 +1725,7 @@ describe('Timeline profiler', () => {
</React.Suspense>,
);

await waitForAll([
'suspended',

...(enableSiblingPrerendering ? ['suspended'] : []),
]);
await waitForAll(['suspended']);

Scheduler.unstable_advanceTime(10);
rejectFn();
Expand All @@ -1780,39 +1736,9 @@ describe('Timeline profiler', () => {
const timelineData = stopProfilingAndGetTimelineData();

// Verify the Suspense event and duration was recorded.
if (enableSiblingPrerendering) {
expect(timelineData.suspenseEvents).toMatchInlineSnapshot(`
[
{
"componentName": "Example",
"depth": 0,
"duration": 10,
"id": "0",
"phase": "mount",
"promiseName": "",
"resolution": "rejected",
"timestamp": 10,
"type": "suspense",
"warning": null,
},
{
"componentName": "Example",
"depth": 0,
"duration": 10,
"id": "0",
"phase": "mount",
"promiseName": "",
"resolution": "rejected",
"timestamp": 10,
"type": "suspense",
"warning": null,
},
]
`);
} else {
expect(timelineData.suspenseEvents).toHaveLength(1);
const suspenseEvent = timelineData.suspenseEvents[0];
expect(suspenseEvent).toMatchInlineSnapshot(`
expect(timelineData.suspenseEvents).toHaveLength(1);
const suspenseEvent = timelineData.suspenseEvents[0];
expect(suspenseEvent).toMatchInlineSnapshot(`
{
"componentName": "Example",
"depth": 0,
Expand All @@ -1826,13 +1752,10 @@ describe('Timeline profiler', () => {
"warning": null,
}
`);
}

// There should be two batches of renders: Suspeneded and resolved.
expect(timelineData.batchUIDToMeasuresMap.size).toBe(2);
expect(timelineData.componentMeasures).toHaveLength(
enableSiblingPrerendering ? 3 : 2,
);
expect(timelineData.componentMeasures).toHaveLength(2);
});

it('should mark cascading class component state updates', async () => {
Expand Down
14 changes: 2 additions & 12 deletions packages/react-dom/src/__tests__/ReactDOMForm-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1459,23 +1459,13 @@ describe('ReactDOMForm', () => {
</Suspense>,
),
);
assertLog([
'Suspend! [Count: 0]',
'Loading...',

...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 0]'] : []),
]);
assertLog(['Suspend! [Count: 0]', 'Loading...']);
await act(() => resolveText('Count: 0'));
assertLog(['Count: 0']);

// Dispatch outside of a transition. This will trigger a loading state.
await act(() => dispatch());
assertLog([
'Suspend! [Count: 1]',
'Loading...',

...(gate('enableSiblingPrerendering') ? ['Suspend! [Count: 1]'] : []),
]);
assertLog(['Suspend! [Count: 1]', 'Loading...']);
expect(container.textContent).toBe('Loading...');

await act(() => resolveText('Count: 1'));
Expand Down
8 changes: 1 addition & 7 deletions packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,13 +160,7 @@ describe('ReactDOMSuspensePlaceholder', () => {
});

expect(container.textContent).toEqual('Loading...');
assertLog([
'A',
'Suspend! [B]',
'Loading...',

...(gate('enableSiblingPrerendering') ? ['A', 'Suspend! [B]', 'C'] : []),
]);
assertLog(['A', 'Suspend! [B]', 'Loading...']);
await act(() => {
resolveText('B');
});
Expand Down
Loading

0 comments on commit 89c9014

Please sign in to comment.