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

Add referrerPolicy option to ReactDOM.preload() #27096

Merged
merged 2 commits into from
Jul 12, 2023

Conversation

styfle
Copy link
Contributor

@styfle styfle commented Jul 11, 2023

Summary

We started using ReactDOM.preload() in Next.js but found the referrerPolicy option was missing.

How did you test this change?

yarn test packages/react-dom/src/__tests__/ReactDOMFloat-test.js

@react-sizebot
Copy link

react-sizebot commented Jul 11, 2023

Comparing: 0a36064...24e2703

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 +0.02% 164.35 kB 164.38 kB = 51.76 kB 51.77 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.02% 171.76 kB 171.79 kB = 53.98 kB 53.99 kB
facebook-www/ReactDOM-prod.classic.js = 567.22 kB 567.27 kB +0.01% 100.04 kB 100.05 kB
facebook-www/ReactDOM-prod.modern.js = 551.02 kB 551.07 kB +0.01% 97.21 kB 97.22 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 24e2703

Comment on lines 3588 to 3594
ReactDOM.preload('rp', {
as: 'image',
imageSrcSet: 'rpsrcset',
imageSizes: 'rpsizes',
referrerPolicy: 'no-referrer',
});

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we just make a new test. The preload calls in this test are designed to demonstrate the keying behavior and throwing the ancillary rp preload here is going to maybe be confusing in the future. There is probably already a more generic test acutally that you can just add referrer policy to and just modify the assertions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a new test in 24e2703

@styfle styfle requested a review from gnoff July 12, 2023 16:30
@gnoff gnoff merged commit 9377e10 into facebook:main Jul 12, 2023
@styfle styfle deleted the add-referrerpolicy-on-reactdom-preload branch July 12, 2023 21:35
github-actions bot pushed a commit that referenced this pull request Jul 12, 2023
kodiakhq bot pushed a commit to vercel/next.js that referenced this pull request Jul 14, 2023
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants