Skip to content

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Jun 10, 2024

Cherrypick #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.

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 10, 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 10, 2024 6:46pm

@gnoff gnoff merged commit 6230622 into facebook:sync-nextjs Jun 10, 2024
@gnoff gnoff deleted the sync-nextjs branch June 10, 2024 18:41
@facebook-github-bot facebook-github-bot added the React Core Team Opened by a member of the React Core Team label Jun 10, 2024
@react-sizebot
Copy link

Comparing: 1df34bd...a81096d

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 +0.11% 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js +0.04% 497.26 kB 497.46 kB +0.06% 89.11 kB 89.17 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB +0.11% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js +0.04% 502.08 kB 502.28 kB +0.06% 89.80 kB 89.85 kB
facebook-www/ReactDOM-prod.classic.js +0.03% 594.56 kB 594.75 kB +0.06% 104.72 kB 104.78 kB
facebook-www/ReactDOM-prod.modern.js +0.03% 570.95 kB 571.14 kB +0.06% 101.13 kB 101.19 kB
test_utils/ReactAllWarnings.js Deleted 63.89 kB 0.00 kB Deleted 15.97 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 63.89 kB 0.00 kB Deleted 15.97 kB 0.00 kB

Generated by 🚫 dangerJS against a81096d

ztanner added a commit to vercel/next.js that referenced this pull request Jun 11, 2024
<details>
<summary>React upstream changes</summary>

- facebook/react#29835

</details>
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