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.

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

[Fizz] Fix Client Render after Postpone #27905

Merged
merged 2 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
135 changes: 135 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7178,6 +7178,141 @@ describe('ReactDOMFizzServer', () => {
);
});

// @gate enablePostpone
it('can client render a boundary after having already postponed', async () => {
let prerendering = true;
let ssr = true;

function Postpone() {
if (prerendering) {
React.unstable_postpone();
}
return 'Hello';
}

function ServerError() {
if (ssr) {
throw new Error('server error');
}
return 'World';
}

function App() {
return (
<div>
<Suspense fallback="Loading1">
<Postpone />
<ServerError />
</Suspense>
<Suspense fallback="Loading2">
<Postpone />
</Suspense>
</div>
);
}

const prerenderErrors = [];
const prerendered = await ReactDOMFizzStatic.prerenderToNodeStream(
<App />,
{
onError(x) {
prerenderErrors.push(x.message);
},
},
);
expect(prerendered.postponed).not.toBe(null);

prerendering = false;

const ssrErrors = [];

const resumed = ReactDOMFizzServer.resumeToPipeableStream(
<App />,
JSON.parse(JSON.stringify(prerendered.postponed)),
{
onError(x) {
ssrErrors.push(x.message);
},
},
);

const windowErrors = [];
function globalError(e) {
windowErrors.push(e.message);
}
window.addEventListener('error', globalError);

// Create a separate stream so it doesn't close the writable. I.e. simple concat.
const preludeWritable = new Stream.PassThrough();
preludeWritable.setEncoding('utf8');
preludeWritable.on('data', chunk => {
writable.write(chunk);
});

await act(() => {
prerendered.prelude.pipe(preludeWritable);
});

expect(windowErrors).toEqual([]);

expect(getVisibleChildren(container)).toEqual(
<div>
{'Loading1'}
{'Loading2'}
</div>,
);

await act(() => {
resumed.pipe(writable);
});

expect(prerenderErrors).toEqual(['server error']);

// Since this errored, we shouldn't have to replay it.
expect(ssrErrors).toEqual([]);

expect(windowErrors).toEqual([]);

// Still loading...
expect(getVisibleChildren(container)).toEqual(
<div>
{'Loading1'}
{'Hello'}
</div>,
);

const recoverableErrors = [];

ssr = false;

await clientAct(() => {
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(x) {
recoverableErrors.push(x.message);
},
});
});

expect(recoverableErrors).toEqual(
__DEV__
? ['server error']
: [
'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>
{'Hello'}
{'World'}
{'Hello'}
</div>,
);

expect(windowErrors).toEqual([]);

window.removeEventListener('error', globalError);
});

// @gate enablePostpone
it('can postpone in fallback', async () => {
let prerendering = true;
Expand Down
33 changes: 33 additions & 0 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,8 @@ function renderSuspenseBoundary(
}
encodeErrorForBoundary(newBoundary, errorDigest, error, thrownInfo);

untrackBoundary(request, newBoundary);

// We don't need to decrement any task numbers because we didn't spawn any new task.
// We don't need to schedule any task because we know the parent has written yet.
// We do need to fallthrough to create the fallback though.
Expand Down Expand Up @@ -2672,6 +2674,34 @@ function trackPostpone(
}
}

// In case a boundary errors, we need to stop tracking it because we won't
// resume it.
function untrackBoundary(request: Request, boundary: SuspenseBoundary) {
const trackedPostpones = request.trackedPostpones;
if (trackedPostpones === null) {
return;
}
const boundaryKeyPath = boundary.trackedContentKeyPath;
if (boundaryKeyPath === null) {
return;
}
const boundaryNode: void | ReplayNode =
trackedPostpones.workingMap.get(boundaryKeyPath);
if (boundaryNode === undefined) {
return;
}

// Downgrade to plain ReplayNode since we won't replay through it.
// $FlowFixMe[cannot-write]: We intentionally downgrade this to the other tuple.
boundaryNode.length = 4;
// Remove any resumable slots.
boundaryNode[2] = [];
boundaryNode[3] = null;

// TODO: We should really just remove the boundary from all parent paths too so
// we don't replay the path to it.
}

function injectPostponedHole(
request: Request,
task: RenderTask,
Expand Down Expand Up @@ -3007,6 +3037,7 @@ function erroredTask(
if (boundary.status !== CLIENT_RENDERED) {
boundary.status = CLIENT_RENDERED;
encodeErrorForBoundary(boundary, errorDigest, error, errorInfo);
untrackBoundary(request, boundary);

// Regardless of what happens next, this boundary won't be displayed,
// so we can flush it, if the parent already flushed.
Expand Down Expand Up @@ -3192,6 +3223,8 @@ function abortTask(task: Task, request: Request, error: mixed): void {
}
encodeErrorForBoundary(boundary, errorDigest, errorMessage, errorInfo);

untrackBoundary(request, boundary);

if (boundary.parentFlushed) {
request.clientRenderedBoundaries.push(boundary);
}
Expand Down