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.

Sign up for GitHub

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

Update preload links to support nonce and fetchpriority #26826

Merged
merged 1 commit into from
May 23, 2023
Merged

Update preload links to support nonce and fetchpriority #26826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5510,6 +5510,7 @@ function preloadAsStylePropsFromProps(href: string, props: any): PreloadProps {
as: 'style',
href: href,
crossOrigin: props.crossOrigin,
fetchPriority: props.fetchPriority,
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 didn't make these changes to preloadPropsFromPreloadOptions. It seems like that's used for ReactDOM.preload - please let me know if I should make similar changes there as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

@liuyenwei we should support this for preload and preinit functions if you want to open a separate PR for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good, i can make that change as well. thanks @gnoff !

integrity: props.integrity,
media: props.media,
hrefLang: props.hrefLang,
Expand All @@ -5523,7 +5524,9 @@ function preloadAsScriptPropsFromProps(href: string, props: any): PreloadProps {
as: 'script',
href,
crossOrigin: props.crossOrigin,
fetchPriority: props.fetchPriority,
integrity: props.integrity,
nonce: props.nonce,
referrerPolicy: props.referrerPolicy,
};
}
Expand Down
76 changes: 76 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFloat-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4531,6 +4531,43 @@ body {
);
});

// @gate enableFloat
it('respects attributes defined on the stylesheet element when preloading stylesheets during server rendering', async () => {
await act(() => {
const {pipe} = renderToPipeableStream(
<html>
<head>
<link rel="stylesheet" fetchPriority="high" href="foo" />
</head>
<body>
<link rel="stylesheet" fetchPriority="low" href="notaresource" />
<div>hello world</div>
</body>
</html>,
);
pipe(writable);
});

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
<link rel="preload" as="style" fetchpriority="high" href="foo" />
<link
rel="preload"
as="style"
fetchpriority="low"
href="notaresource"
/>
<link rel="stylesheet" fetchpriority="high" href="foo" />
</head>
<body>
<link rel="stylesheet" fetchpriority="low" href="notaresource" />
<div>hello world</div>
</body>
</html>,
);
});

// @gate enableFloat
it('hoists stylesheet resources to the correct precedence', async () => {
await act(() => {
Expand Down Expand Up @@ -5913,6 +5950,45 @@ background-color: green;
);
});

// @gate enableFloat
it('respects attributes defined on the script element when preloading scripts during server rendering', async () => {
await act(() => {
const {pipe} = renderToPipeableStream(
<html>
<head />
<body>
<script src="foo" fetchPriority="high" nonce="1234" />
<script src="bar" fetchPriority="low" nonce="1234" />
hello world
</body>
</html>,
);
pipe(writable);
});

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
<link
rel="preload"
href="foo"
fetchpriority="high"
nonce="1234"
as="script"
/>
<link
rel="preload"
href="bar"
fetchpriority="low"
nonce="1234"
as="script"
/>
</head>
<body>hello world</body>
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 noticed the <script> elements were not appearing here - this seems weird to me, but looking through the other tests I'm assuming this is expected behavior?

Copy link
Collaborator

Choose a reason for hiding this comment

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

getMeaningfulChildren filters out some scripts like instruction scripts. Sometimes it can't tell whether a script is meaningful so you can render it with a specail attribute data-meaningful={true} and it will then be included if you really want to assert that the script is included. In this case though you are really just making sure the nonce and fetcpriority show up on the preload so I would leave it as is personally

</html>,
);
});

// @gate enableFloat
it('does not create script resources when inside an <svg> context', async () => {
await act(() => {
Expand Down