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

[Float][Fiber] Assume stylesheets in document are already loaded (#29811) #29812

Merged
merged 1 commit into from
Jun 8, 2024

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Jun 8, 2024

Cherry pick of #29811

When we made stylesheets suspend even during high priority updates we exposed a bug in the loading tracking of stylesheets that are loaded as part of the preamble. This allowed these stylesheets to put suspense boundaries into fallback mode more often than expected because cases where a stylesheet was server rendered could now cause a fallback to trigger which was never intended to happen.

This fix updates resource construction to evaluate whether the instance exists in the DOM prior to construction and if so marks the resource as loaded and inserted.

One ambiguity that needed to be solved still is how to tell whether a stylesheet rendered as part of a late Suspense boundary reveal is already loaded. I updated the instruction to clear out the loading promise after successfully loading. This is useful because later if we encounter this same resource again we can avoid the microtask if it is already loaded. It also means that we can concretely understand that if a stylesheet is in the DOM without this marker then it must have loaded (or errored) already.

…ebook#29811)

When we made stylesheets suspend even during high priority updates we
exposed a bug in the loading tracking of stylesheets that are loaded as
part of the preamble. This allowed these stylesheets to put suspense
boundaries into fallback mode more often than expected because cases
where a stylesheet was server rendered could now cause a fallback to
trigger which was never intended to happen.

This fix updates resource construction to evaluate whether the instance
exists in the DOM prior to construction and if so marks the resource as
loaded and inserted.

One ambiguity that needed to be solved still is how to tell whether a
stylesheet rendered as part of a late Suspense boundary reveal is
already loaded. I updated the instruction to clear out the loading
promise after successfully loading. This is useful because later if we
encounter this same resource again we can avoid the microtask if it is
already loaded. It also means that we can concretely understand that if
a stylesheet is in the DOM without this marker then it must have loaded
(or errored) already.
Copy link

vercel bot commented Jun 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 8, 2024 0:09am

@gnoff gnoff merged commit 34d0c5e into facebook:sync-nextjs Jun 8, 2024
3 of 24 checks passed
@gnoff gnoff deleted the sync-nextjs branch June 8, 2024 00:03
@facebook-github-bot facebook-github-bot added the React Core Team Opened by a member of the React Core Team label Jun 8, 2024
@react-sizebot
Copy link

Comparing: f994737...f85afe9

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.66 kB 6.66 kB = 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js +0.04% 495.89 kB 496.12 kB +0.05% 88.83 kB 88.87 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js +0.04% 500.69 kB 500.92 kB +0.05% 89.51 kB 89.55 kB
facebook-www/ReactDOM-prod.classic.js +0.04% 593.05 kB 593.27 kB +0.05% 104.32 kB 104.37 kB
facebook-www/ReactDOM-prod.modern.js +0.04% 569.27 kB 569.49 kB +0.05% 100.72 kB 100.77 kB
test_utils/ReactAllWarnings.js Deleted 64.35 kB 0.00 kB Deleted 16.05 kB 0.00 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 +0.96% 8.60 kB 8.69 kB +1.30% 2.23 kB 2.26 kB
test_utils/ReactAllWarnings.js Deleted 64.35 kB 0.00 kB Deleted 16.05 kB 0.00 kB

Generated by 🚫 dangerJS against f85afe9

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.

3 participants