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

[Fizz][Float] stop automatically preloading scripts that are not script resources #26877

Merged
merged 1 commit into from
Jun 1, 2023

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented May 31, 2023

Currently we preload all scripts that are not hoisted. One of the original reasons for this is we stopped SSR rendering async scripts that had an onLoad/onError because we needed to be able to distinguish between Float scripts and non-Float scripts during hydration. Hydration has been refactored a bit and we can not get around this limitation so we can just emit the async script in place. However, sync and defer scripts are also preloaded. While this is sometimes desirable it is not universally so and there are issues with conveying priority properly (see fetchpriority) so with this change we remove the automatic preloading of non-Float scripts altogether.

For this change to make sense we also need to emit async scripts with loading handlers during SSR. we previously only preloaded them during SSR because it was necessary to keep async scripts as unambiguously resources when hydrating. One ancillary benefit was that load handlers would always fire b/c there was no chance the script would run before hydration. With this change we go back to having the ability to have load handlers fired before hydration. This is already a problem with images and we don't have a generalized solution for it however our likely approach to this sort of thing where you need to wait for a script to load is to use something akin to importScripts() rather than rendering a script with onLoad.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels May 31, 2023
@gnoff
Copy link
Collaborator Author

gnoff commented May 31, 2023

This diff is much easier to review when hiding whitespace

@react-sizebot
Copy link

react-sizebot commented May 31, 2023

Comparing: 5fb2c15...1ecd803

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.min.js = 164.37 kB 164.23 kB = 51.79 kB 51.73 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 171.81 kB 171.67 kB = 54.03 kB 53.97 kB
facebook-www/ReactDOM-prod.classic.js = 570.67 kB 570.12 kB = 100.68 kB 100.57 kB
facebook-www/ReactDOM-prod.modern.js = 554.41 kB 553.90 kB = 97.85 kB 97.75 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
facebook-www/ReactDOMServer-dev.classic.js = 380.64 kB 378.37 kB = 82.59 kB 82.22 kB
facebook-www/ReactDOMServer-dev.modern.js = 373.21 kB 370.94 kB = 80.95 kB 80.56 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.development.js = 373.57 kB 371.29 kB = 82.50 kB 81.79 kB
oss-experimental/react-dom/cjs/react-dom-server.node.development.js = 372.40 kB 370.13 kB = 82.15 kB 81.45 kB
oss-experimental/react-dom/cjs/react-dom-static.node.development.js = 372.36 kB 370.09 kB = 82.24 kB 81.54 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.development.js = 371.75 kB 369.47 kB = 82.04 kB 81.33 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.development.js = 371.31 kB 369.04 kB = 82.21 kB 81.51 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.development.js = 370.90 kB 368.63 kB = 82.10 kB 81.40 kB
oss-experimental/react-dom/cjs/react-dom-static.edge.development.js = 370.62 kB 368.34 kB = 82.03 kB 81.33 kB
oss-experimental/react-dom/cjs/react-dom-static.browser.development.js = 370.21 kB 367.93 kB = 81.91 kB 81.22 kB
oss-experimental/react-dom/umd/react-dom-server-legacy.browser.development.js = 389.35 kB 386.95 kB = 82.92 kB 82.49 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.development.js = 368.73 kB 366.45 kB = 81.43 kB 80.72 kB
oss-experimental/react-dom/umd/react-dom-server.browser.development.js = 388.48 kB 386.08 kB = 83.02 kB 82.58 kB
facebook-www/ReactDOMServerStreaming-dev.modern.js = 368.04 kB 365.77 kB = 79.69 kB 79.30 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.development.js = 363.81 kB 361.53 kB = 80.09 kB 79.61 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.development.js = 363.78 kB 361.51 kB = 80.06 kB 79.59 kB
oss-stable/react-dom/cjs/react-dom-server.node.development.js = 363.22 kB 360.95 kB = 79.98 kB 79.50 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.development.js = 363.20 kB 360.92 kB = 79.95 kB 79.47 kB
oss-stable/react-dom/cjs/react-dom-server.edge.development.js = 362.14 kB 359.86 kB = 80.04 kB 79.57 kB
oss-stable-semver/react-dom/cjs/react-dom-server.edge.development.js = 362.11 kB 359.83 kB = 80.02 kB 79.54 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.development.js = 361.99 kB 359.72 kB = 79.63 kB 79.15 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.development.js = 361.97 kB 359.69 kB = 79.60 kB 79.13 kB
oss-stable/react-dom/cjs/react-dom-server.browser.development.js = 361.73 kB 359.45 kB = 79.93 kB 79.45 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.development.js = 361.70 kB 359.42 kB = 79.90 kB 79.42 kB
oss-stable/react-dom/umd/react-dom-server-legacy.browser.development.js = 379.17 kB 376.78 kB = 80.78 kB 80.09 kB
oss-stable-semver/react-dom/umd/react-dom-server-legacy.browser.development.js = 379.15 kB 376.75 kB = 80.75 kB 80.06 kB
oss-stable/react-dom/umd/react-dom-server.browser.development.js = 378.89 kB 376.49 kB = 81.10 kB 80.41 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.development.js = 378.86 kB 376.47 kB = 81.08 kB 80.39 kB
oss-stable/react-dom/cjs/react-dom-server.bun.development.js = 358.97 kB 356.69 kB = 79.04 kB 78.56 kB
oss-stable-semver/react-dom/cjs/react-dom-server.bun.development.js = 358.94 kB 356.67 kB = 79.01 kB 78.54 kB
oss-experimental/react-dom/cjs/react-dom-static.node.production.min.js = 66.79 kB 66.33 kB = 20.93 kB 20.85 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.production.min.js = 66.69 kB 66.23 kB = 20.89 kB 20.79 kB
oss-experimental/react-dom/cjs/react-dom-server.node.production.min.js = 66.81 kB 66.35 kB = 20.93 kB 20.83 kB
oss-stable/react-dom/cjs/react-dom-server.node.production.min.js = 64.74 kB 64.30 kB = 20.01 kB 19.92 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.production.min.js = 64.72 kB 64.27 kB = 19.99 kB 19.89 kB
oss-stable/react-dom/cjs/react-dom-server.edge.production.min.js = 64.62 kB 64.17 kB = 19.98 kB 19.89 kB
oss-stable-semver/react-dom/cjs/react-dom-server.edge.production.min.js = 64.59 kB 64.14 kB = 19.95 kB 19.87 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.production.min.js = 66.02 kB 65.56 kB = 20.05 kB 19.97 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.production.min.js = 63.42 kB 62.98 kB = 18.89 kB 18.82 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.production.min.js = 63.39 kB 62.95 kB = 18.87 kB 18.79 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.production.min.js = 65.39 kB 64.93 kB = 20.12 kB 20.04 kB
oss-stable/react-dom/cjs/react-dom-server.bun.production.min.js = 62.78 kB 62.34 kB = 18.98 kB 18.89 kB
oss-stable-semver/react-dom/cjs/react-dom-server.bun.production.min.js = 62.76 kB 62.31 kB = 18.96 kB 18.87 kB
oss-experimental/react-dom/umd/react-dom-server.browser.production.min.js = 62.50 kB 62.04 kB = 19.66 kB 19.58 kB
oss-experimental/react-dom/cjs/react-dom-static.edge.production.min.js = 62.55 kB 62.09 kB = 19.47 kB 19.39 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.production.min.js = 62.34 kB 61.88 kB = 19.42 kB 19.32 kB
oss-experimental/react-dom/cjs/react-dom-static.browser.production.min.js = 62.22 kB 61.76 kB = 19.37 kB 19.28 kB
oss-stable/react-dom/umd/react-dom-server.browser.production.min.js = 60.53 kB 60.09 kB = 18.81 kB 18.72 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.production.min.js = 60.51 kB 60.06 kB = 18.78 kB 18.69 kB
oss-stable/react-dom/cjs/react-dom-server.browser.production.min.js = 60.37 kB 59.92 kB = 18.55 kB 18.48 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.production.min.js = 60.34 kB 59.90 kB = 18.53 kB 18.46 kB
oss-experimental/react-dom/umd/react-dom-server-legacy.browser.production.min.js = 61.30 kB 60.85 kB = 18.76 kB 18.66 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.production.min.js = 61.13 kB 60.68 kB = 18.45 kB 18.35 kB
oss-stable/react-dom/umd/react-dom-server-legacy.browser.production.min.js = 58.81 kB 58.37 kB = 17.61 kB 17.50 kB
oss-stable-semver/react-dom/umd/react-dom-server-legacy.browser.production.min.js = 58.78 kB 58.34 kB = 17.59 kB 17.48 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.production.min.js = 58.64 kB 58.20 kB = 17.34 kB 17.25 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.production.min.js = 58.61 kB 58.17 kB = 17.31 kB 17.23 kB
facebook-www/ReactDOMServerStreaming-prod.modern.js = 143.15 kB 141.74 kB = 26.95 kB 26.78 kB
facebook-www/ReactDOMServer-prod.classic.js = 140.78 kB 139.37 kB = 26.07 kB 25.91 kB
facebook-www/ReactDOMServer-prod.modern.js = 137.26 kB 135.85 kB = 25.40 kB 25.24 kB

Generated by 🚫 dangerJS against 1ecd803

@gnoff gnoff requested a review from sebmarkbage May 31, 2023 16:00
@gnoff gnoff changed the title No script preload [Fizz][Float] stop automatically preloading scripts that are not script resources May 31, 2023
…ginal reasons for this is we stopped SSR rendering async scripts that had an onLoad/onError because we needed to be able to distinguish between Float scripts and non-Float scripts during hydration. Hydration has been refactored a bit and we can not get around this limitation so we can just emit the async script in place. However, sync and defer scripts are also preloaded. While this is sometimes desirable it is not universally so and there are issues with conveying priority properly (see fetchpriority) so with this change we remove the automatic preloading of non-Float scripts altogether.
@gnoff gnoff merged commit e1ad4aa into facebook:main Jun 1, 2023
@gnoff gnoff deleted the no-script-preload branch June 1, 2023 20:34
github-actions bot pushed a commit that referenced this pull request Jun 1, 2023
…pt resources (#26877)

Currently we preload all scripts that are not hoisted. One of the
original reasons for this is we stopped SSR rendering async scripts that
had an onLoad/onError because we needed to be able to distinguish
between Float scripts and non-Float scripts during hydration. Hydration
has been refactored a bit and we can not get around this limitation so
we can just emit the async script in place. However, sync and defer
scripts are also preloaded. While this is sometimes desirable it is not
universally so and there are issues with conveying priority properly
(see fetchpriority) so with this change we remove the automatic
preloading of non-Float scripts altogether.

For this change to make sense we also need to emit async scripts with
loading handlers during SSR. we previously only preloaded them during
SSR because it was necessary to keep async scripts as unambiguously
resources when hydrating. One ancillary benefit was that load handlers
would always fire b/c there was no chance the script would run before
hydration. With this change we go back to having the ability to have
load handlers fired before hydration. This is already a problem with
images and we don't have a generalized solution for it however our
likely approach to this sort of thing where you need to wait for a
script to load is to use something akin to `importScripts()` rather than
rendering a script with onLoad.

DiffTrain build for [e1ad4aa](e1ad4aa)
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…pt resources (facebook#26877)

Currently we preload all scripts that are not hoisted. One of the
original reasons for this is we stopped SSR rendering async scripts that
had an onLoad/onError because we needed to be able to distinguish
between Float scripts and non-Float scripts during hydration. Hydration
has been refactored a bit and we can not get around this limitation so
we can just emit the async script in place. However, sync and defer
scripts are also preloaded. While this is sometimes desirable it is not
universally so and there are issues with conveying priority properly
(see fetchpriority) so with this change we remove the automatic
preloading of non-Float scripts altogether.

For this change to make sense we also need to emit async scripts with
loading handlers during SSR. we previously only preloaded them during
SSR because it was necessary to keep async scripts as unambiguously
resources when hydrating. One ancillary benefit was that load handlers
would always fire b/c there was no chance the script would run before
hydration. With this change we go back to having the ability to have
load handlers fired before hydration. This is already a problem with
images and we don't have a generalized solution for it however our
likely approach to this sort of thing where you need to wait for a
script to load is to use something akin to `importScripts()` rather than
rendering a script with onLoad.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
…pt resources (#26877)

Currently we preload all scripts that are not hoisted. One of the
original reasons for this is we stopped SSR rendering async scripts that
had an onLoad/onError because we needed to be able to distinguish
between Float scripts and non-Float scripts during hydration. Hydration
has been refactored a bit and we can not get around this limitation so
we can just emit the async script in place. However, sync and defer
scripts are also preloaded. While this is sometimes desirable it is not
universally so and there are issues with conveying priority properly
(see fetchpriority) so with this change we remove the automatic
preloading of non-Float scripts altogether.

For this change to make sense we also need to emit async scripts with
loading handlers during SSR. we previously only preloaded them during
SSR because it was necessary to keep async scripts as unambiguously
resources when hydrating. One ancillary benefit was that load handlers
would always fire b/c there was no chance the script would run before
hydration. With this change we go back to having the ability to have
load handlers fired before hydration. This is already a problem with
images and we don't have a generalized solution for it however our
likely approach to this sort of thing where you need to wait for a
script to load is to use something akin to `importScripts()` rather than
rendering a script with onLoad.

DiffTrain build for commit e1ad4aa.
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