From 99781d605b6a6fd332bba654ac0c328e41df888c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 22 May 2025 10:20:13 -0400 Subject: [PATCH 1/4] [Fizz] Track boundaries in future rows as postponed (#33329) Follow up to #33321. We can mark boundaries that were blocked in the prerender as postponed but without anything to replayed inside them. That way they're not emitted in the prerender but is unblocked when replayed. Technically this does some unnecessary replaying of the path to the otherwise already completed boundary but it simplifies our model by just marking the boundary as needing replaying. --- .../ReactDOMFizzStaticBrowser-test.js | 6 +- packages/react-server/src/ReactFizzServer.js | 129 +++++++++++------- 2 files changed, 87 insertions(+), 48 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index 6fc68f84ab7..fc01dcb1566 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -2262,6 +2262,7 @@ describe('ReactDOMFizzStaticBrowser', () => { C + D ); @@ -2282,6 +2283,7 @@ describe('ReactDOMFizzStaticBrowser', () => { }); const prerendered = await pendingResult; + const postponedState = JSON.stringify(prerendered.postponed); await readIntoContainer(prerendered.prelude); @@ -2289,7 +2291,8 @@ describe('ReactDOMFizzStaticBrowser', () => {
{'Loading A'} {'Loading B'} - {'C' /* TODO: This should not be resolved. */} + {'Loading C'} + {'Loading D'}
, ); @@ -2309,6 +2312,7 @@ describe('ReactDOMFizzStaticBrowser', () => { {'A'} {'B'} {'C'} + {'D'} , ); }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index f3c70b66c06..35c7909396e 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -1725,6 +1725,26 @@ function unblockSuspenseListRow( } } +function trackPostponedSuspenseListRow( + request: Request, + trackedPostpones: PostponedHoles, + postponedRow: null | SuspenseListRow, +): void { + // TODO: Because we unconditionally call this, it will be called by finishedTask + // and so ends up recursive which can lead to stack overflow for very long lists. + if (postponedRow !== null) { + const postponedBoundaries = postponedRow.boundaries; + if (postponedBoundaries !== null) { + postponedRow.boundaries = null; + for (let i = 0; i < postponedBoundaries.length; i++) { + const postponedBoundary = postponedBoundaries[i]; + trackPostponedBoundary(request, trackedPostpones, postponedBoundary); + finishedTask(request, postponedBoundary, null, null); + } + } + } +} + function tryToResolveTogetherRow( request: Request, togetherRow: SuspenseListRow, @@ -3774,6 +3794,49 @@ function renderChildrenArray( } } +function trackPostponedBoundary( + request: Request, + trackedPostpones: PostponedHoles, + boundary: SuspenseBoundary, +): ReplaySuspenseBoundary { + boundary.status = POSTPONED; + // We need to eagerly assign it an ID because we'll need to refer to + // it before flushing and we know that we can't inline it. + boundary.rootSegmentID = request.nextSegmentId++; + + const boundaryKeyPath = boundary.trackedContentKeyPath; + if (boundaryKeyPath === null) { + throw new Error( + 'It should not be possible to postpone at the root. This is a bug in React.', + ); + } + + const fallbackReplayNode = boundary.trackedFallbackNode; + + const children: Array = []; + const boundaryNode: void | ReplayNode = + trackedPostpones.workingMap.get(boundaryKeyPath); + if (boundaryNode === undefined) { + const suspenseBoundary: ReplaySuspenseBoundary = [ + boundaryKeyPath[1], + boundaryKeyPath[2], + children, + null, + fallbackReplayNode, + boundary.rootSegmentID, + ]; + trackedPostpones.workingMap.set(boundaryKeyPath, suspenseBoundary); + addToReplayParent(suspenseBoundary, boundaryKeyPath[0], trackedPostpones); + return suspenseBoundary; + } else { + // Upgrade to ReplaySuspenseBoundary. + const suspenseBoundary: ReplaySuspenseBoundary = (boundaryNode: any); + suspenseBoundary[4] = fallbackReplayNode; + suspenseBoundary[5] = boundary.rootSegmentID; + return suspenseBoundary; + } +} + function trackPostpone( request: Request, trackedPostpones: PostponedHoles, @@ -3796,22 +3859,12 @@ function trackPostpone( } if (boundary !== null && boundary.status === PENDING) { - boundary.status = POSTPONED; - // We need to eagerly assign it an ID because we'll need to refer to - // it before flushing and we know that we can't inline it. - boundary.rootSegmentID = request.nextSegmentId++; - - const boundaryKeyPath = boundary.trackedContentKeyPath; - if (boundaryKeyPath === null) { - throw new Error( - 'It should not be possible to postpone at the root. This is a bug in React.', - ); - } - - const fallbackReplayNode = boundary.trackedFallbackNode; - - const children: Array = []; - if (boundaryKeyPath === keyPath && task.childIndex === -1) { + const boundaryNode = trackPostponedBoundary( + request, + trackedPostpones, + boundary, + ); + if (boundary.trackedContentKeyPath === keyPath && task.childIndex === -1) { // Assign ID if (segment.id === -1) { if (segment.parentFlushed) { @@ -3823,39 +3876,10 @@ function trackPostpone( } } // We postponed directly inside the Suspense boundary so we mark this for resuming. - const boundaryNode: ReplaySuspenseBoundary = [ - boundaryKeyPath[1], - boundaryKeyPath[2], - children, - segment.id, - fallbackReplayNode, - boundary.rootSegmentID, - ]; - trackedPostpones.workingMap.set(boundaryKeyPath, boundaryNode); - addToReplayParent(boundaryNode, boundaryKeyPath[0], trackedPostpones); + boundaryNode[3] = segment.id; return; - } else { - let boundaryNode: void | ReplayNode = - trackedPostpones.workingMap.get(boundaryKeyPath); - if (boundaryNode === undefined) { - boundaryNode = [ - boundaryKeyPath[1], - boundaryKeyPath[2], - children, - null, - fallbackReplayNode, - boundary.rootSegmentID, - ]; - trackedPostpones.workingMap.set(boundaryKeyPath, boundaryNode); - addToReplayParent(boundaryNode, boundaryKeyPath[0], trackedPostpones); - } else { - // Upgrade to ReplaySuspenseBoundary. - const suspenseBoundary: ReplaySuspenseBoundary = (boundaryNode: any); - suspenseBoundary[4] = fallbackReplayNode; - suspenseBoundary[5] = boundary.rootSegmentID; - } - // Fall through to add the child node. } + // Otherwise, fall through to add the child node. } // We know that this will leave a hole so we might as well assign an ID now. @@ -4941,7 +4965,18 @@ function finishedTask( } else if (boundary.status === POSTPONED) { const boundaryRow = boundary.row; if (boundaryRow !== null) { + if (request.trackedPostpones !== null) { + // If this boundary is postponed, then we need to also postpone any blocked boundaries + // in the next row. + trackPostponedSuspenseListRow( + request, + request.trackedPostpones, + boundaryRow.next, + ); + } if (--boundaryRow.pendingTasks === 0) { + // This is really unnecessary since we've already postponed the boundaries but + // for pairity with other track+finish paths. We might end up using the hoisting. finishSuspenseListRow(request, boundaryRow); } } From 08064ea6713ff55730c5d4f50b733891ebe0e875 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 22 May 2025 10:21:28 -0400 Subject: [PATCH 2/4] [Fizz] Make ViewTransition enter/exit/share null the same as none (#33331) I believe that these mean the same thing. We don't have to emit the attribute if it's `none` for these cases because if there is no matching scenario we won't apply the animation in this case. The only case where we have to emit `none` in the attribute is for `vt-update` because those can block updates from propagating upwards. --- .../src/server/ReactFizzConfigDOM.js | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index ef69bd4582a..9418f6035b2 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -759,10 +759,9 @@ const SUBTREE_SCOPE = ~(ENTER_SCOPE | EXIT_SCOPE); type ViewTransitionContext = { update: 'none' | 'auto' | string, - // null here means that this case can never trigger. Not "auto" like it does in props. - enter: null | 'none' | 'auto' | string, - exit: null | 'none' | 'auto' | string, - share: null | 'none' | 'auto' | string, + enter: 'none' | 'auto' | string, + exit: 'none' | 'auto' | string, + share: 'none' | 'auto' | string, name: 'auto' | string, autoName: string, // a name that can be used if an explicit one is not defined. nameIdx: number, // keeps track of how many duplicates of this name we've emitted. @@ -917,8 +916,8 @@ function getSuspenseViewTransition( // we would've used (the parent ViewTransition name or auto-assign one). const viewTransition: ViewTransitionContext = { update: parentViewTransition.update, // For deep updates. - enter: null, - exit: null, + enter: 'none', + exit: 'none', share: parentViewTransition.update, // For exit or enter of reveals. name: parentViewTransition.autoName, autoName: parentViewTransition.autoName, @@ -989,13 +988,8 @@ export function getViewTransitionFormatContext( share = parentViewTransition.share; } else { name = 'auto'; - share = null; // share is only relevant if there's an explicit name + share = 'none'; // share is only relevant if there's an explicit name } - } else if (share === 'none') { - // I believe if share is disabled, it means the same thing as if it doesn't - // exit because enter/exit will take precedence and if it's deeply nested - // it just animates along whatever the parent does when disabled. - share = null; } else { if (share == null) { share = 'auto'; @@ -1008,12 +1002,12 @@ export function getViewTransitionFormatContext( } } if (!(parentContext.tagScope & EXIT_SCOPE)) { - exit = null; // exit is only relevant for the first ViewTransition inside fallback + exit = 'none'; // exit is only relevant for the first ViewTransition inside fallback } else { resumableState.instructions |= NeedUpgradeToViewTransitions; } if (!(parentContext.tagScope & ENTER_SCOPE)) { - enter = null; // enter is only relevant for the first ViewTransition inside content + enter = 'none'; // enter is only relevant for the first ViewTransition inside content } else { resumableState.instructions |= NeedUpgradeToViewTransitions; } @@ -1125,13 +1119,13 @@ function pushViewTransitionAttributes( viewTransition.nameIdx++; } pushStringAttribute(target, 'vt-update', viewTransition.update); - if (viewTransition.enter !== null) { + if (viewTransition.enter !== 'none') { pushStringAttribute(target, 'vt-enter', viewTransition.enter); } - if (viewTransition.exit !== null) { + if (viewTransition.exit !== 'none') { pushStringAttribute(target, 'vt-exit', viewTransition.exit); } - if (viewTransition.share !== null) { + if (viewTransition.share !== 'none') { pushStringAttribute(target, 'vt-share', viewTransition.share); } } From 91ac1fea1aacf80c9eb2815956e230921b55808c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 22 May 2025 10:25:13 -0400 Subject: [PATCH 3/4] [Fizz] Pass batch as argument to revealCompletedBoundaries (#33330) Follow up to #33293. This solves a race condition when boundaries are added to the batch after the `startViewTransition` call. This doesn't matter yet but it will once we start assigning names before the `startViewTransition` call. A possible alternative solution might be to ensure the names are added synchronously in the event that adds to the batch. It's possible to keep adding to a batch until the snapshot has happened. --- ...tDOMFizzInstructionSetInlineCodeStrings.js | 4 ++-- .../ReactDOMFizzInstructionSetShared.js | 24 ++++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js b/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js index aa7209fadac..429876692eb 100644 --- a/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js +++ b/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js @@ -6,9 +6,9 @@ export const markShellTime = 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 = - '$RB=[];$RV=function(){$RT=performance.now();var d=$RB;$RB=[];for(var a=0;a { + // TODO + }); transition.finished.finally(() => { if (document['__reactViewTransition'] === transition) { document['__reactViewTransition'] = null; } }); + // Queue any future completions into its own batch since they won't have been + // snapshotted by this one. + window['$RB'] = []; return; } // Fall through to reveal. @@ -109,7 +117,7 @@ export function revealCompletedBoundariesWithViewTransitions(revealBoundaries) { // Fall through to reveal. } // ViewTransitions v2 not supported or no ViewTransitions found. Reveal immediately. - revealBoundaries(); + revealBoundaries(batch); } export function clientRenderBoundary( @@ -182,7 +190,7 @@ export function completeBoundary(suspenseBoundaryID, contentID) { // We always schedule the flush in a timer even if it's very low or negative to allow // for multiple completeBoundary calls that are already queued to have a chance to // make the batch. - setTimeout(window['$RV'], msUntilTimeout); + setTimeout(window['$RV'].bind(null, window['$RB']), msUntilTimeout); } } From 8ce15b0f56a066ece465963ca1370e46113bb868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 22 May 2025 10:40:28 -0400 Subject: [PATCH 4/4] [Fizz] Apply View Transition Name and Class to SSR:ed View Transitions (#33332) Stacked on #33330. This walks the element tree to activate the various classes under different scenarios. There are some edge case things that are a little different since we can't express every scenario without virtual nodes. The main thing that's still missing though is avoiding animating updates if it can be contained to a layout or enter/exit/share if they're out of the viewport. I.e. layout stuff. --- fixtures/view-transition/server/render.js | 2 - .../view-transition/src/components/Page.js | 48 +++-- ...tDOMFizzInstructionSetInlineCodeStrings.js | 2 +- .../ReactDOMFizzInstructionSetShared.js | 171 +++++++++++++++++- 4 files changed, 204 insertions(+), 19 deletions(-) diff --git a/fixtures/view-transition/server/render.js b/fixtures/view-transition/server/render.js index 716290212e5..11d352eabdd 100644 --- a/fixtures/view-transition/server/render.js +++ b/fixtures/view-transition/server/render.js @@ -23,8 +23,6 @@ export default function render(url, res) { const {pipe, abort} = renderToPipeableStream( , { - // TODO: Temporary hack. Detect from attributes instead. - bootstrapScriptContent: 'window._useVT = true;', bootstrapScripts: [assets['main.js']], onShellReady() { // If something errored before we started streaming, we set the error code appropriately. diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index 061b1edb2cb..39d0803af77 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -200,21 +200,41 @@ export default function Page({url, navigate}) {
!!
- + +
+ +

█████

+ +

████

+

███████

+

████

+

██

+

██████

+

███

+

████

+
+ + }> -

these

-

rows

-

exist

-

to

-

test

-

scrolling

-

content

-

out

-

of

- {portal} -

the

-

viewport

- +
+

these

+

rows

+ +

exist

+ +

to

+

test

+

scrolling

+

content

+

out

+

of

+ {portal} +

the

+

viewport

+ +
{show ? : null} diff --git a/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js b/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js index 429876692eb..363e11f988d 100644 --- a/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js +++ b/packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js @@ -8,7 +8,7 @@ export const clientRenderBoundary = export const completeBoundary = '$RB=[];$RV=function(c){$RT=performance.now();for(var a=0;a