OSSForks/react
1
0
Fork 0
mirror of https://github.com/zebrajr/react.git synced 2025-12-06 12:20:20 +01:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity

[Fizz] Render preamble eagerly (#33730)

Browse Source 
We unnecessarily render the preamble in a task. This updates the
implementation to perform this render inline.

Testing this is tricky because one of the only ways you could assert
this was even happening is based on how things error if you abort while
rendering the root.

While adding a test for this I discovered that not all abortable tasks
report errors when aborted during a normal render. I've asserted the
current behavior and will address the other issue at another time and
updated the assertion later as necessary
This commit is contained in:
Josh Story 2025-07-08 11:20:12 -04:00 committed by GitHub
parent bbea677b77
commit befc1246b0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 114 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

96
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js vendored
View File

@ -9544,6 +9544,102 @@ describe('ReactDOMFizzServer', () => {
); );
}); });
it('will attempt to render the preamble inline to allow rendering before a later abort in the same task', async () => {
const promise = new Promise(() => {});
function Pending() {
React.use(promise);
}
const controller = new AbortController();
function Abort() {
controller.abort();
return <Comp />;
}
function Comp() {
return null;
}
function App() {
return (
<html>
<head>
<meta content="here" />
</head>
<body>
<main>hello</main>
<Suspense>
<Pending />
</Suspense>
<Abort />
</body>
</html>
);
}
const signal = controller.signal;
let thrownError = null;
const errors = [];
try {
await act(() => {
const {pipe, abort} = renderToPipeableStream(<App />, {
onError(e, ei) {
errors.push({
error: e,
componentStack: normalizeCodeLocInfo(ei.componentStack),
});
},
});
signal.addEventListener('abort', () => abort('boom'));
pipe(writable);
});
} catch (e) {
thrownError = e;
}
expect(thrownError).toBe('boom');
// TODO there should actually be three errors. One for the pending Suspense, one for the fallback task, and one for the task
// that does the abort itself. At the moment abort will flush queues and if there is no pending tasks will close the request before
// the task which initiated the abort can even be processed. This is a bug but not one that I am fixing with the current change
// so I am asserting the current behavior
expect(errors).toEqual([
{
error: 'boom',
componentStack: componentStack([
'Pending',
'Suspense',
'body',
'html',
'App',
]),
},
{
error: 'boom',
componentStack: componentStack([
'Suspense Fallback',
'body',
'html',
'App',
]),
// }, {
// error: 'boom',
// componentStack: componentStack(['Abort', 'body', 'html', 'App'])
},
]);
// We expect the render to throw before streaming anything so the default
// document is still loaded
expect(getVisibleChildren(document)).toEqual(
<html>
<head />
<body>
<div id="container" />
</body>
</html>,
);
});
it('Will wait to flush Document chunks until all boundaries which might contain a preamble are errored or resolved', async () => { it('Will wait to flush Document chunks until all boundaries which might contain a preamble are errored or resolved', async () => {
let rejectFirst; let rejectFirst;
const firstPromise = new Promise((_, reject) => { const firstPromise = new Promise((_, reject) => {

42
packages/react-server/src/ReactFizzServer.js vendored
View File

@ -2206,7 +2206,7 @@ function renderSuspenseList(
function renderPreamble( function renderPreamble(
request: Request, request: Request,
task: Task, task: RenderTask,
blockedSegment: Segment, blockedSegment: Segment,
node: ReactNodeList, node: ReactNodeList,
): void { ): void {
@ -2219,28 +2219,21 @@ function renderPreamble(
false, false,
); );
blockedSegment.preambleChildren.push(preambleSegment); blockedSegment.preambleChildren.push(preambleSegment);
// @TODO we can just attempt to render in the current task rather than spawning a new one task.blockedSegment = preambleSegment;
const preambleTask = createRenderTask( try {
request, preambleSegment.status = RENDERING;
null, renderNode(request, task, node, -1);
node, pushSegmentFinale(
-1, preambleSegment.chunks,
task.blockedBoundary, request.renderState,
preambleSegment, preambleSegment.lastPushedText,
task.blockedPreamble, preambleSegment.textEmbedded,
task.hoistableState, );
request.abortableTasks, preambleSegment.status = COMPLETED;
task.keyPath, finishedSegment(request, task.blockedBoundary, preambleSegment);
task.formatContext, } finally {
task.context, task.blockedSegment = blockedSegment;
task.treeContext, }
task.row,
task.componentStack,
!disableLegacyContext ? task.legacyContext : emptyContextObject,
__DEV__ ? task.debugTask : null,
);
pushComponentStack(preambleTask);
request.pingedTasks.push(preambleTask);
} }
function renderHostElement( function renderHostElement(
@ -2292,7 +2285,8 @@ function renderHostElement(
props, props,
)); ));
if (isPreambleContext(newContext)) { if (isPreambleContext(newContext)) {
renderPreamble(request, task, segment, children); // $FlowFixMe: Refined
renderPreamble(request, (task: RenderTask), segment, children);
} else { } else {
// We use the non-destructive form because if something suspends, we still // We use the non-destructive form because if something suspends, we still
// need to pop back up and finish this subtree of HTML. // need to pop back up and finish this subtree of HTML.