Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't recreate the same fallback on the client if hydrating suspends #24236

Merged
merged 5 commits into from
Apr 1, 2022
Merged

Don't recreate the same fallback on the client if hydrating suspends #24236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
38b3038
Delay showing fallback if hydrating suspends
gaearon Mar 31, 2022
a47c615
Fix up
gaearon Mar 31, 2022
6f96d0a
Include all non-urgent lanes
gaearon Mar 31, 2022
e6734cd
Moar tests
gaearon Apr 1, 2022
6cb9f18
Add test for transitions
gaearon Apr 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
308 changes: 294 additions & 14 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,16 +869,16 @@ describe('ReactDOMFizzServer', () => {
});

// We still can't render it on the client.
expect(Scheduler).toFlushAndYield([
'The server could not finish this Suspense boundary, likely due to an ' +
'error during server rendering. Switched to client rendering.',
]);
expect(Scheduler).toFlushAndYield([]);
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

// We now resolve it on the client.
resolveText('Hello');

Scheduler.unstable_flushAll();
expect(Scheduler).toFlushAndYield([
'The server could not finish this Suspense boundary, likely due to an ' +
'error during server rendering. Switched to client rendering.',
]);

// The client rendered HTML is now in place.
expect(getVisibleChildren(container)).toEqual(
Expand Down Expand Up @@ -2220,6 +2220,286 @@ describe('ReactDOMFizzServer', () => {
},
);

// @gate experimental
it('does not recreate the fallback if server errors and hydration suspends', async () => {
let isClient = false;

function Child() {
if (isClient) {
readText('Yay!');
} else {
throw Error('Oops.');
}
Scheduler.unstable_yieldValue('Yay!');
return 'Yay!';
}

const fallbackRef = React.createRef();
function App() {
return (
<div>
<Suspense fallback={<p ref={fallbackRef}>Loading...</p>}>
<span>
<Child />
</span>
</Suspense>
</div>
);
}
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />, {
onError(error) {
Scheduler.unstable_yieldValue('[s!] ' + error.message);
},
});
pipe(writable);
});
expect(Scheduler).toHaveYielded(['[s!] Oops.']);

// The server could not complete this boundary, so we'll retry on the client.
const serverFallback = container.getElementsByTagName('p')[0];
expect(serverFallback.innerHTML).toBe('Loading...');

// Hydrate the tree. This will suspend.
isClient = true;
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue('[c!] ' + error.message);
},
});
// This should not report any errors yet.
expect(Scheduler).toFlushAndYield([]);
expect(getVisibleChildren(container)).toEqual(
<div>
<p>Loading...</p>
</div>,
);

// Normally, hydrating after server error would force a clean client render.
// However, it suspended so at best we'd only get the same fallback anyway.
// We don't want to recreate the same fallback in the DOM again because
// that's extra work and would restart animations etc. Check we don't do that.
const clientFallback = container.getElementsByTagName('p')[0];
expect(serverFallback).toBe(clientFallback);

// When we're able to fully hydrate, we expect a clean client render.
await act(async () => {
resolveText('Yay!');
});
expect(Scheduler).toFlushAndYield([
'Yay!',
'[c!] The server could not finish this Suspense boundary, ' +
'likely due to an error during server rendering. ' +
'Switched to client rendering.',
]);
expect(getVisibleChildren(container)).toEqual(
<div>
<span>Yay!</span>
</div>,
);
});

// @gate experimental
it(
'does not recreate the fallback if server errors and hydration suspends ' +
'and root receives a transition',
async () => {
let isClient = false;

function Child({color}) {
if (isClient) {
readText('Yay!');
} else {
throw Error('Oops.');
}
Scheduler.unstable_yieldValue('Yay! (' + color + ')');
return 'Yay! (' + color + ')';
}

const fallbackRef = React.createRef();
function App({color}) {
return (
<div>
<Suspense fallback={<p ref={fallbackRef}>Loading...</p>}>
<span>
<Child color={color} />
</span>
</Suspense>
</div>
);
}
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<App color="red" />,
{
onError(error) {
Scheduler.unstable_yieldValue('[s!] ' + error.message);
},
},
);
pipe(writable);
});
expect(Scheduler).toHaveYielded(['[s!] Oops.']);

// The server could not complete this boundary, so we'll retry on the client.
const serverFallback = container.getElementsByTagName('p')[0];
expect(serverFallback.innerHTML).toBe('Loading...');

// Hydrate the tree. This will suspend.
isClient = true;
const root = ReactDOMClient.hydrateRoot(container, <App color="red" />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue('[c!] ' + error.message);
},
});
// This should not report any errors yet.
expect(Scheduler).toFlushAndYield([]);
expect(getVisibleChildren(container)).toEqual(
<div>
<p>Loading...</p>
</div>,
);

// Normally, hydrating after server error would force a clean client render.
// However, it suspended so at best we'd only get the same fallback anyway.
// We don't want to recreate the same fallback in the DOM again because
// that's extra work and would restart animations etc. Check we don't do that.
const clientFallback = container.getElementsByTagName('p')[0];
expect(serverFallback).toBe(clientFallback);

// Transition updates shouldn't recreate the fallback either.
React.startTransition(() => {
root.render(<App color="blue" />);
});
Scheduler.unstable_flushAll();
jest.runAllTimers();
const clientFallback2 = container.getElementsByTagName('p')[0];
expect(clientFallback2).toBe(serverFallback);

// When we're able to fully hydrate, we expect a clean client render.
await act(async () => {
resolveText('Yay!');
});
expect(Scheduler).toFlushAndYield([
'Yay! (red)',
'[c!] The server could not finish this Suspense boundary, ' +
'likely due to an error during server rendering. ' +
'Switched to client rendering.',
'Yay! (blue)',
]);
expect(getVisibleChildren(container)).toEqual(
<div>
<span>Yay! (blue)</span>
</div>,
);
},
);

// @gate experimental
it(
'recreates the fallback if server errors and hydration suspends but ' +
'client receives new props',
async () => {
let isClient = false;

function Child() {
const value = 'Yay!';
if (isClient) {
readText(value);
} else {
throw Error('Oops.');
}
Scheduler.unstable_yieldValue(value);
return value;
}

const fallbackRef = React.createRef();
function App({fallbackText}) {
return (
<div>
<Suspense fallback={<p ref={fallbackRef}>{fallbackText}</p>}>
<span>
<Child />
</span>
</Suspense>
</div>
);
}

await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<App fallbackText="Loading..." />,
{
onError(error) {
Scheduler.unstable_yieldValue('[s!] ' + error.message);
},
},
);
pipe(writable);
});
expect(Scheduler).toHaveYielded(['[s!] Oops.']);

const serverFallback = container.getElementsByTagName('p')[0];
expect(serverFallback.innerHTML).toBe('Loading...');

// Hydrate the tree. This will suspend.
isClient = true;
const root = ReactDOMClient.hydrateRoot(
container,
<App fallbackText="Loading..." />,
{
onRecoverableError(error) {
Scheduler.unstable_yieldValue('[c!] ' + error.message);
},
},
);
// This should not report any errors yet.
expect(Scheduler).toFlushAndYield([]);
expect(getVisibleChildren(container)).toEqual(
<div>
<p>Loading...</p>
</div>,
);

// Normally, hydration after server error would force a clean client render.
// However, that suspended so at best we'd only get a fallback anyway.
// We don't want to replace a fallback with the same fallback because
// that's extra work and would restart animations etc. Verify we don't do that.
const clientFallback1 = container.getElementsByTagName('p')[0];
expect(serverFallback).toBe(clientFallback1);

// However, an update may have changed the fallback props. In that case we have to
// actually force it to re-render on the client and throw away the server one.
root.render(<App fallbackText="More loading..." />);
Scheduler.unstable_flushAll();
jest.runAllTimers();
expect(Scheduler).toHaveYielded([
'[c!] The server could not finish this Suspense boundary, ' +
'likely due to an error during server rendering. ' +
'Switched to client rendering.',
]);
expect(getVisibleChildren(container)).toEqual(
<div>
<p>More loading...</p>
</div>,
);
// This should be a clean render without reusing DOM.
const clientFallback2 = container.getElementsByTagName('p')[0];
expect(clientFallback2).not.toBe(clientFallback1);

// Verify we can still do a clean content render after.
await act(async () => {
resolveText('Yay!');
});
expect(Scheduler).toFlushAndYield(['Yay!']);
expect(getVisibleChildren(container)).toEqual(
<div>
<span>Yay!</span>
</div>,
);
},
);

// @gate experimental
it(
'errors during hydration force a client render at the nearest Suspense ' +
Expand Down Expand Up @@ -2293,25 +2573,25 @@ describe('ReactDOMFizzServer', () => {
},
});

// An error logged but instead of surfacing it to the UI, we switched
// to client rendering.
expect(Scheduler).toFlushAndYield([
'Hydration error',
'There was an error while hydrating this Suspense boundary. Switched ' +
'to client rendering.',
]);
// An error happened but instead of surfacing it to the UI, we suspended.
expect(Scheduler).toFlushAndYield([]);
expect(getVisibleChildren(container)).toEqual(
<div>
<span />
Loading...
<span>Yay!</span>
<span />
</div>,
);

await act(async () => {
resolveText('Yay!');
});
expect(Scheduler).toFlushAndYield(['Yay!']);
expect(Scheduler).toFlushAndYield([
'Yay!',
'Hydration error',
'There was an error while hydrating this Suspense boundary. Switched ' +
'to client rendering.',
]);
expect(getVisibleChildren(container)).toEqual(
<div>
<span />
Expand Down
Loading