Skip to content

Commit

Permalink
Don't recreate the same fallback on the client if hydrating suspends (#…
Browse files Browse the repository at this point in the history 
…24236)

* Delay showing fallback if hydrating suspends

* Fix up

* Include all non-urgent lanes

* Moar tests

* Add test for transitions
  • Loading branch information
@gaearon
gaearon authored Apr 1, 2022
1 parent d352fd0 commit ebd7ff6
Show file tree
Hide file tree
Showing 9 changed files with 315 additions and 52 deletions.
308 changes: 294 additions & 14 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
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

0 comments on commit ebd7ff6

Please sign in to comment.