Skip to content

[Fizz] Wrap revealCompletedBoundaries in a ViewTransitions aware version #33293

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

Merged
merged 4 commits into from
May 17, 2025

Conversation

sebmarkbage
Copy link
Collaborator

When needed.

For the external runtime we always include this wrapper.

For others, we only include it if we have an ViewTransitions affecting. If we discover the ViewTransitions late, then we can upgrade an already emitted instruction.

This doesn't yet do anything useful with it, that's coming in a follow up. This is just the mechanism for how it gets installed.

@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label May 16, 2025
@react-sizebot
Copy link

react-sizebot commented May 16, 2025

Comparing: c250b7d...e8e9fc4

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 529.74 kB 529.74 kB = 93.49 kB 93.49 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 651.48 kB 651.48 kB = 114.76 kB 114.75 kB
facebook-www/ReactDOM-prod.classic.js = 675.72 kB 675.72 kB = 118.85 kB 118.84 kB
facebook-www/ReactDOM-prod.modern.js = 666.00 kB 666.00 kB = 117.23 kB 117.23 kB
oss-experimental/react-dom/unstable_server-external-runtime.js +5.86% 9.60 kB 10.16 kB +6.47% 2.54 kB 2.70 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-dom/unstable_server-external-runtime.js +5.86% 9.60 kB 10.16 kB +6.47% 2.54 kB 2.70 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.production.js +0.92% 249.98 kB 252.27 kB +0.75% 44.67 kB 45.00 kB
facebook-www/ReactDOMServerStreaming-prod.modern.js +0.87% 234.28 kB 236.32 kB +0.60% 42.97 kB 43.23 kB
facebook-www/ReactDOMServer-prod.modern.js +0.81% 228.18 kB 230.02 kB +0.54% 40.96 kB 41.18 kB
facebook-www/ReactDOMServer-prod.classic.js +0.80% 231.09 kB 232.93 kB +0.52% 41.29 kB 41.51 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.production.js +0.76% 238.81 kB 240.62 kB +0.49% 42.49 kB 42.70 kB
oss-experimental/react-markup/cjs/react-markup.production.js +0.76% 227.33 kB 229.05 kB +0.48% 41.65 kB 41.85 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.production.js +0.74% 243.89 kB 245.70 kB +0.48% 44.38 kB 44.59 kB
facebook-www/ReactDOMServerStreaming-dev.modern.js +0.67% 378.64 kB 381.18 kB +0.46% 67.95 kB 68.26 kB
oss-experimental/react-markup/cjs/react-markup.react-server.production.js +0.52% 328.44 kB 330.16 kB +0.33% 61.03 kB 61.24 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.development.js +0.52% 358.93 kB 360.79 kB +0.60% 68.18 kB 68.59 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.production.js +0.52% 272.06 kB 273.46 kB +0.68% 47.02 kB 47.34 kB
oss-experimental/react-dom/cjs/react-dom-server.node.production.js +0.51% 273.98 kB 275.38 kB +0.62% 48.22 kB 48.52 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.production.js +0.51% 278.01 kB 279.41 kB +0.65% 49.18 kB 49.50 kB
oss-experimental/react-dom/cjs/react-dom-server.node.development.js +0.49% 423.45 kB 425.54 kB +0.50% 73.70 kB 74.07 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.development.js +0.49% 427.49 kB 429.58 kB +0.52% 74.34 kB 74.73 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.development.js +0.49% 428.50 kB 430.59 kB +0.52% 74.57 kB 74.96 kB
oss-experimental/react-markup/cjs/react-markup.development.js +0.49% 367.37 kB 369.16 kB +0.28% 65.98 kB 66.17 kB
oss-stable-semver/react-server/cjs/react-server.production.js +0.44% 118.36 kB 118.89 kB +0.14% 21.04 kB 21.07 kB
oss-stable/react-server/cjs/react-server.production.js +0.44% 118.36 kB 118.89 kB +0.14% 21.04 kB 21.07 kB
oss-experimental/react-server/cjs/react-server.production.js +0.42% 133.70 kB 134.26 kB +0.14% 23.14 kB 23.17 kB
facebook-www/ReactDOMServer-dev.modern.js +0.36% 385.19 kB 386.58 kB +0.41% 68.81 kB 69.09 kB
facebook-www/ReactDOMServer-dev.classic.js +0.36% 388.64 kB 390.04 kB +0.40% 69.35 kB 69.62 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.development.js +0.34% 399.00 kB 400.35 kB +0.36% 70.55 kB 70.80 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.development.js +0.34% 399.00 kB 400.36 kB +0.36% 70.55 kB 70.80 kB
oss-experimental/react-markup/cjs/react-markup.react-server.development.js +0.33% 549.33 kB 551.12 kB +0.21% 98.30 kB 98.51 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.production.js +0.29% 216.36 kB 216.97 kB +0.11% 39.39 kB 39.44 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.production.js +0.29% 216.38 kB 217.00 kB +0.11% 39.41 kB 39.46 kB
oss-stable-semver/react-server/cjs/react-server.development.js +0.28% 174.89 kB 175.38 kB +0.13% 31.33 kB 31.37 kB
oss-stable/react-server/cjs/react-server.development.js +0.28% 174.89 kB 175.38 kB +0.13% 31.33 kB 31.37 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.production.js +0.28% 220.87 kB 221.49 kB +0.16% 41.15 kB 41.22 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.production.js +0.28% 220.90 kB 221.51 kB +0.16% 41.18 kB 41.24 kB
oss-stable-semver/react-dom/cjs/react-dom-server.bun.production.js +0.28% 221.44 kB 222.05 kB +0.13% 40.72 kB 40.77 kB
oss-stable/react-dom/cjs/react-dom-server.bun.production.js +0.28% 221.51 kB 222.13 kB +0.13% 40.74 kB 40.80 kB
oss-experimental/react-server/cjs/react-server.development.js +0.28% 193.11 kB 193.64 kB +0.08% 33.63 kB 33.65 kB

Generated by 🚫 dangerJS against e8e9fc4

…n needed

For the external runtime we always include this wrapper.

For others, we only include it if we have an ViewTransitions affecting.
If we discover the ViewTransitions late, then we can upgrade an already
emitted instruction.
…io that might need it

We don't know if we'll actually outline a boundary to need this but just in case.
@sebmarkbage sebmarkbage force-pushed the fizzvtruntime branch 2 times, most recently from ffe59f1 to a38be02 Compare May 16, 2025 20:08
window['$RV'] = revealCompletedBoundariesWithViewTransitions.bind(
null,
revealCompletedBoundaries,
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally we’d want to make sure static functions like this gets inlined but since this function will be pass as first call into startViewTransition and reused in multiple places it needs to be a separate function anyway.

null,
// eslint-disable-next-line dot-notation
window['$RV'],
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could optimize this by special casing when it goes together with the complete boundary instruction but it’s not really much difference atm so not worth it.

@@ -896,6 +900,7 @@ export function getChildFormatContext(
}

function getSuspenseViewTransition(
resumableState: ResumableState,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you add this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally thought I'd do it in both branches but then realized that I could do it only if we ever render the fallback.

@sebmarkbage sebmarkbage merged commit 6060367 into facebook:main May 17, 2025
177 of 179 checks passed
github-actions bot pushed a commit that referenced this pull request May 17, 2025
…ion (#33293)

When needed.

For the external runtime we always include this wrapper.

For others, we only include it if we have an ViewTransitions affecting.
If we discover the ViewTransitions late, then we can upgrade an already
emitted instruction.

This doesn't yet do anything useful with it, that's coming in a follow
up. This is just the mechanism for how it gets installed.

DiffTrain build for [6060367](6060367)
sebmarkbage added a commit that referenced this pull request May 22, 2025
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.
github-actions bot pushed a commit that referenced this pull request May 22, 2025
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.

DiffTrain build for [91ac1fe](91ac1fe)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants