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] emit viewport meta before preloads #27201

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Aug 7, 2023

Fixes: #27200

preloads for images that appear before the viewport meta may be loaded twice because the proper device image information is not used with the preload but is with the image itself. The viewport meta should be emitted earlier than all preloads to avoid this.

this change moves the queue for the viewport meta to preconnects which already has the right priority for this tag

… the proper resolution loaded in some cases. This change updates our queueing strategy to utilize our preconnect queue for the viewport meta tag
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 7, 2023
@gnoff gnoff requested a review from sebmarkbage August 7, 2023 19:53
@react-sizebot
Copy link

Comparing: 997f52f...bc15396

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.38 kB 164.38 kB = 51.77 kB 51.77 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 171.79 kB 171.79 kB = 53.99 kB 53.99 kB
facebook-www/ReactDOM-prod.classic.js = 567.40 kB 567.40 kB = 100.10 kB 100.10 kB
facebook-www/ReactDOM-prod.modern.js = 551.20 kB 551.20 kB = 97.26 kB 97.26 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against bc15396

Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

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

Should the queue be renamed something more generic?

@gnoff
Copy link
Collaborator Author

gnoff commented Aug 7, 2023

Yes probably. I considered it for this PR but I think I want to rename them all. I don't want to just end up with super high, high, medium, low etc... so using what they are for is nice.

In reality I could have created a separate queue and just put it after (or before) preconnects but the order isn't important between these sets and it is slightly more effecient to keep them together. I'll think about a broader rename as I land the other PRs that affect our queues

@gnoff gnoff merged commit ea17cc1 into facebook:main Aug 7, 2023
2 checks passed
@gnoff gnoff deleted the float-viewport branch August 7, 2023 22:22
github-actions bot pushed a commit that referenced this pull request Aug 7, 2023
Fixes: #27200

preloads for images that appear before the viewport meta may be loaded
twice because the proper device image information is not used with the
preload but is with the image itself. The viewport meta should be
emitted earlier than all preloads to avoid this.

this change moves the queue for the viewport meta to preconnects which
already has the right priority for this tag

DiffTrain build for [ea17cc1](ea17cc1)
kodiakhq bot pushed a commit to vercel/next.js that referenced this pull request Aug 8, 2023
…#53700)

The root cause is `ReactDOM.preload()` inserts `<link rel="preload">` above the `<meta name="viewport">`. 

This PR adds a test to prove that upgrading React fixes the issue (see commits).

- Depends on facebook/react#27201
- Depends on #53742
- Fixes #53574
- Related #52995
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Fixes: facebook#27200 

preloads for images that appear before the viewport meta may be loaded
twice because the proper device image information is not used with the
preload but is with the image itself. The viewport meta should be
emitted earlier than all preloads to avoid this.

this change moves the queue for the viewport meta to preconnects which
already has the right priority for this tag
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Fixes: #27200

preloads for images that appear before the viewport meta may be loaded
twice because the proper device image information is not used with the
preload but is with the image itself. The viewport meta should be
emitted earlier than all preloads to avoid this.

this change moves the queue for the viewport meta to preconnects which
already has the right priority for this tag

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

Bug: ReactDOM.preload()
4 participants