[Fizz] Don't handle errors in completeBoundary instruction (#33073)

Stacked on #33066 and #33068.

Currently we're passing `errorDigest` to `completeBoundary` if there is
a client side error (only CSS loading atm). This only exists because of
`completeBoundaryWithStyles`. Normally if there's a server-side error
we'd emit the `clientRenderBoundary` instruction instead. This adds
unnecessary code to the common case where all styles are in the head.
This is about to get worse with batching because client render shouldn't
be throttled but complete should be.

The first commit moves the client render logic inline into
`completeBoundaryWithStyles` so we only pay for it when styles are used.

However, the approach I went with in the second commit is to reuse the
`$RX` instruction instead (`clientRenderBoundary`). That way if you have
both it ends up being amortized. However, it does mean we have to emit
the `$RX` (along with the `$RC` helper if any
`completeBoundaryWithStyles` instruction is needed.
This commit is contained in:
Sebastian Markbåge 2025-05-01 15:44:17 -04:00 committed by GitHub
parent bb57fa7351
commit ee077b6ccd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 64 additions and 65 deletions

View File

@ -4482,14 +4482,14 @@ export function writeCompletedSegmentInstruction(
}
}
const completeBoundaryScriptFunctionOnly = stringToPrecomputedChunk(
completeBoundaryFunction,
);
const completeBoundaryScript1Full = stringToPrecomputedChunk(
completeBoundaryFunction + '$RC("',
);
const completeBoundaryScript1Partial = stringToPrecomputedChunk('$RC("');
const completeBoundaryWithStylesScript1FullBoth = stringToPrecomputedChunk(
completeBoundaryFunction + styleInsertionFunction + '$RR("',
);
const completeBoundaryWithStylesScript1FullPartial = stringToPrecomputedChunk(
styleInsertionFunction + '$RR("',
);
@ -4531,19 +4531,27 @@ export function writeCompletedBoundaryInstruction(
writeChunk(destination, renderState.startInlineScript);
writeChunk(destination, endOfStartTag);
if (requiresStyleInsertion) {
if (
(resumableState.instructions & SentClientRenderFunction) ===
NothingSent
) {
// The completeBoundaryWithStyles function depends on the client render function.
resumableState.instructions |= SentClientRenderFunction;
writeChunk(destination, clientRenderScriptFunctionOnly);
}
if (
(resumableState.instructions & SentCompleteBoundaryFunction) ===
NothingSent
) {
resumableState.instructions |=
SentStyleInsertionFunction | SentCompleteBoundaryFunction;
writeChunk(destination, completeBoundaryWithStylesScript1FullBoth);
} else if (
// The completeBoundaryWithStyles function depends on the complete boundary function.
resumableState.instructions |= SentCompleteBoundaryFunction;
writeChunk(destination, completeBoundaryScriptFunctionOnly);
}
if (
(resumableState.instructions & SentStyleInsertionFunction) ===
NothingSent
) {
resumableState.instructions |= SentStyleInsertionFunction;
writeChunk(destination, completeBoundaryWithStylesScript1FullPartial);
} else {
writeChunk(destination, completeBoundaryWithStylesScript1Partial);
@ -4608,6 +4616,9 @@ export function writeCompletedBoundaryInstruction(
return writeBootstrap(destination, renderState) && writeMore;
}
const clientRenderScriptFunctionOnly =
stringToPrecomputedChunk(clientRenderFunction);
const clientRenderScript1Full = stringToPrecomputedChunk(
clientRenderFunction + ';$RX("',
);

View File

@ -4,9 +4,9 @@
export const clientRenderBoundary =
'$RX=function(b,c,d,e,f){var a=document.getElementById(b);a&&(b=a.previousSibling,b.data="$!",a=a.dataset,c&&(a.dgst=c),d&&(a.msg=d),e&&(a.stck=e),f&&(a.cstck=f),b._reactRetry&&b._reactRetry())};';
export const completeBoundary =
'$RC=function(b,d,e){if(d=document.getElementById(d)){d.parentNode.removeChild(d);var a=document.getElementById(b);if(a){b=a.previousSibling;if(e)b.data="$!",a.setAttribute("data-dgst",e);else{e=b.parentNode;a=b.nextSibling;var f=0;do{if(a&&8===a.nodeType){var c=a.data;if("/$"===c||"/&"===c)if(0===f)break;else f--;else"$"!==c&&"$?"!==c&&"$!"!==c&&"&"!==c||f++}c=a.nextSibling;e.removeChild(a);a=c}while(a);for(;d.firstChild;)e.insertBefore(d.firstChild,a);b.data="$"}b._reactRetry&&b._reactRetry()}}};';
'$RC=function(a,d){if(d=document.getElementById(d))if(d.parentNode.removeChild(d),a=document.getElementById(a)){a=a.previousSibling;var f=a.parentNode,b=a.nextSibling,e=0;do{if(b&&8===b.nodeType){var c=b.data;if("/$"===c||"/&"===c)if(0===e)break;else e--;else"$"!==c&&"$?"!==c&&"$!"!==c&&"&"!==c||e++}c=b.nextSibling;f.removeChild(b);b=c}while(b);for(;d.firstChild;)f.insertBefore(d.firstChild,b);a.data="$";a._reactRetry&&a._reactRetry()}};';
export const completeBoundaryWithStyles =
'$RM=new Map;\n$RR=function(r,t,w){function u(n){this._p=null;n()}for(var p=new Map,q=document,g,b,h=q.querySelectorAll("link[data-precedence],style[data-precedence]"),v=[],k=0;b=h[k++];)"not all"===b.getAttribute("media")?v.push(b):("LINK"===b.tagName&&$RM.set(b.getAttribute("href"),b),p.set(b.dataset.precedence,g=b));b=0;h=[];var l,a;for(k=!0;;){if(k){var e=w[b++];if(!e){k=!1;b=0;continue}var c=!1,m=0;var d=e[m++];if(a=$RM.get(d)){var f=a._p;c=!0}else{a=q.createElement("link");a.href=d;a.rel=\n"stylesheet";for(a.dataset.precedence=l=e[m++];f=e[m++];)a.setAttribute(f,e[m++]);f=a._p=new Promise(function(n,x){a.onload=u.bind(a,n);a.onerror=u.bind(a,x)});$RM.set(d,a)}d=a.getAttribute("media");!f||d&&!matchMedia(d).matches||h.push(f);if(c)continue}else{a=v[b++];if(!a)break;l=a.getAttribute("data-precedence");a.removeAttribute("media")}c=p.get(l)||g;c===g&&(g=a);p.set(l,a);c?c.parentNode.insertBefore(a,c.nextSibling):(c=q.head,c.insertBefore(a,c.firstChild))}Promise.all(h).then($RC.bind(null,\nr,t,""),$RC.bind(null,r,t,"Resource failed to load"))};';
'$RM=new Map;\n$RR=function(r,v,w){function t(n){this._p=null;n()}for(var p=new Map,q=document,g,b,h=q.querySelectorAll("link[data-precedence],style[data-precedence]"),u=[],k=0;b=h[k++];)"not all"===b.getAttribute("media")?u.push(b):("LINK"===b.tagName&&$RM.set(b.getAttribute("href"),b),p.set(b.dataset.precedence,g=b));b=0;h=[];var l,a;for(k=!0;;){if(k){var e=w[b++];if(!e){k=!1;b=0;continue}var c=!1,m=0;var d=e[m++];if(a=$RM.get(d)){var f=a._p;c=!0}else{a=q.createElement("link");a.href=d;a.rel=\n"stylesheet";for(a.dataset.precedence=l=e[m++];f=e[m++];)a.setAttribute(f,e[m++]);f=a._p=new Promise(function(n,x){a.onload=t.bind(a,n);a.onerror=t.bind(a,x)});$RM.set(d,a)}d=a.getAttribute("media");!f||d&&!matchMedia(d).matches||h.push(f);if(c)continue}else{a=u[b++];if(!a)break;l=a.getAttribute("data-precedence");a.removeAttribute("media")}c=p.get(l)||g;c===g&&(g=a);p.set(l,a);c?c.parentNode.insertBefore(a,c.nextSibling):(c=q.head,c.insertBefore(a,c.firstChild))}Promise.all(h).then($RC.bind(null,\nr,v),$RX.bind(null,r,"CSS failed to load"))};';
export const completeSegment =
'$RS=function(a,b){a=document.getElementById(a);b=document.getElementById(b);for(a.parentNode.removeChild(a);a.firstChild;)b.parentNode.insertBefore(a.firstChild,b);b.parentNode.removeChild(b)};';
export const formReplaying =

View File

@ -47,7 +47,7 @@ export function clientRenderBoundary(
}
}
export function completeBoundary(suspenseBoundaryID, contentID, errorDigest) {
export function completeBoundary(suspenseBoundaryID, contentID) {
const contentNode = document.getElementById(contentID);
if (!contentNode) {
// If the client has failed hydration we may have already deleted the streaming
@ -70,52 +70,47 @@ export function completeBoundary(suspenseBoundaryID, contentID, errorDigest) {
// Find the boundary around the fallback. This is always the previous node.
const suspenseNode = suspenseIdNode.previousSibling;
if (!errorDigest) {
// Clear all the existing children. This is complicated because
// there can be embedded Suspense boundaries in the fallback.
// This is similar to clearSuspenseBoundary in ReactFiberConfigDOM.
// TODO: We could avoid this if we never emitted suspense boundaries in fallback trees.
// They never hydrate anyway. However, currently we support incrementally loading the fallback.
const parentInstance = suspenseNode.parentNode;
let node = suspenseNode.nextSibling;
let depth = 0;
do {
if (node && node.nodeType === COMMENT_NODE) {
const data = node.data;
if (data === SUSPENSE_END_DATA || data === ACTIVITY_END_DATA) {
if (depth === 0) {
break;
} else {
depth--;
}
} else if (
data === SUSPENSE_START_DATA ||
data === SUSPENSE_PENDING_START_DATA ||
data === SUSPENSE_FALLBACK_START_DATA ||
data === ACTIVITY_START_DATA
) {
depth++;
// Clear all the existing children. This is complicated because
// there can be embedded Suspense boundaries in the fallback.
// This is similar to clearSuspenseBoundary in ReactFiberConfigDOM.
// TODO: We could avoid this if we never emitted suspense boundaries in fallback trees.
// They never hydrate anyway. However, currently we support incrementally loading the fallback.
const parentInstance = suspenseNode.parentNode;
let node = suspenseNode.nextSibling;
let depth = 0;
do {
if (node && node.nodeType === COMMENT_NODE) {
const data = node.data;
if (data === SUSPENSE_END_DATA || data === ACTIVITY_END_DATA) {
if (depth === 0) {
break;
} else {
depth--;
}
} else if (
data === SUSPENSE_START_DATA ||
data === SUSPENSE_PENDING_START_DATA ||
data === SUSPENSE_FALLBACK_START_DATA ||
data === ACTIVITY_START_DATA
) {
depth++;
}
const nextNode = node.nextSibling;
parentInstance.removeChild(node);
node = nextNode;
} while (node);
const endOfBoundary = node;
// Insert all the children from the contentNode between the start and end of suspense boundary.
while (contentNode.firstChild) {
parentInstance.insertBefore(contentNode.firstChild, endOfBoundary);
}
suspenseNode.data = SUSPENSE_START_DATA;
} else {
suspenseNode.data = SUSPENSE_FALLBACK_START_DATA;
suspenseIdNode.setAttribute('data-dgst', errorDigest);
const nextNode = node.nextSibling;
parentInstance.removeChild(node);
node = nextNode;
} while (node);
const endOfBoundary = node;
// Insert all the children from the contentNode between the start and end of suspense boundary.
while (contentNode.firstChild) {
parentInstance.insertBefore(contentNode.firstChild, endOfBoundary);
}
suspenseNode.data = SUSPENSE_START_DATA;
if (suspenseNode['_reactRetry']) {
suspenseNode['_reactRetry']();
}
@ -234,13 +229,8 @@ export function completeBoundaryWithStyles(
}
Promise.all(dependencies).then(
window['$RC'].bind(null, suspenseBoundaryID, contentID, ''),
window['$RC'].bind(
null,
suspenseBoundaryID,
contentID,
'Resource failed to load',
),
window['$RC'].bind(null, suspenseBoundaryID, contentID),
window['$RX'].bind(null, suspenseBoundaryID, 'CSS failed to load'),
);
}

View File

@ -1630,14 +1630,14 @@ describe('ReactDOMFizzStaticBrowser', () => {
// We are mostly just trying to assert that no preload for our stylesheet was emitted
// prior to sending the segment the stylesheet was for. This test is asserting this
// because the boundary complete instruction is sent when we are writing the
const instructionIndex = result.indexOf('$RC');
const instructionIndex = result.indexOf('$RX');
expect(instructionIndex > -1).toBe(true);
const slice = result.slice(0, instructionIndex + '$RC'.length);
const slice = result.slice(0, instructionIndex + '$RX'.length);
expect(slice).toBe(
'<!DOCTYPE html><html><head><link rel="expect" href="#«R»" blocking="render"/></head>' +
'<body>hello<!--$?--><template id="B:1"></template><!--/$--><template id="«R»"></template>' +
'<div hidden id="S:1">world<!-- --></div><script>$RC',
'<div hidden id="S:1">world<!-- --></div><script>$RX',
);
});

View File

@ -1265,9 +1265,7 @@ body {
const suspenseInstance = boundaryTemplateInstance.previousSibling;
expect(suspenseInstance.data).toEqual('$!');
expect(boundaryTemplateInstance.dataset.dgst).toBe(
'Resource failed to load',
);
expect(boundaryTemplateInstance.dataset.dgst).toBe('CSS failed to load');
expect(getMeaningfulChildren(document)).toEqual(
<html>
@ -1313,7 +1311,7 @@ body {
);
expect(errors).toEqual([
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
'Resource failed to load',
'CSS failed to load',
]);
});