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

[Float] Support script preloads #25432

Merged
merged 2 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
7 changes: 4 additions & 3 deletions packages/react-dom-bindings/src/client/ReactDOMFloatClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {getCurrentRootHostContainer} from 'react-reconciler/src/ReactFiberHostCo

// The resource types we support. currently they match the form for the as argument.
// In the future this may need to change, especially when modules / scripts are supported
type ResourceType = 'style' | 'font';
type ResourceType = 'style' | 'font' | 'script';

type PreloadProps = {
rel: 'preload',
Expand Down Expand Up @@ -150,7 +150,7 @@ function getDocumentFromRoot(root: FloatRoot): Document {
// ReactDOM.Preload
// --------------------------------------
type PreloadAs = ResourceType;
type PreloadOptions = {as: PreloadAs, crossOrigin?: string};
type PreloadOptions = {as: PreloadAs, crossOrigin?: string, integrity?: string};
function preload(href: string, options: PreloadOptions) {
if (__DEV__) {
validatePreloadArguments(href, options);
Expand Down Expand Up @@ -194,6 +194,7 @@ function preloadPropsFromPreloadOptions(
rel: 'preload',
as,
crossOrigin: as === 'font' ? '' : options.crossOrigin,
integrity: options.integrity,
};
}

Expand Down Expand Up @@ -832,7 +833,7 @@ export function isHostResourceType(type: string, props: Props): boolean {
}

function isResourceAsType(as: mixed): boolean {
return as === 'style' || as === 'font';
return as === 'style' || as === 'font' || as === 'script';
}

// When passing user input into querySelector(All) the embedded string must not alter
Expand Down
6 changes: 4 additions & 2 deletions packages/react-dom-bindings/src/server/ReactDOMFloatServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {

type Props = {[string]: mixed};

type ResourceType = 'style' | 'font';
type ResourceType = 'style' | 'font' | 'script';

type PreloadProps = {
rel: 'preload',
Expand Down Expand Up @@ -123,7 +123,7 @@ export const ReactDOMServerDispatcher = {
};

type PreloadAs = ResourceType;
type PreloadOptions = {as: PreloadAs, crossOrigin?: string};
type PreloadOptions = {as: PreloadAs, crossOrigin?: string, integrity?: string};
function preload(href: string, options: PreloadOptions) {
if (!currentResources) {
// While we expect that preload calls are primarily going to be observed
Expand Down Expand Up @@ -248,6 +248,7 @@ function preloadPropsFromPreloadOptions(
rel: 'preload',
as,
crossOrigin: as === 'font' ? '' : options.crossOrigin,
integrity: options.integrity,
};
}

Expand Down Expand Up @@ -526,6 +527,7 @@ export function resourcesFromLink(props: Props): boolean {
return false;
}
switch (as) {
case 'script':
case 'style':
case 'font': {
if (__DEV__) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,44 +16,42 @@ export function validateUnmatchedLinkResourceProps(
currentProps: ?Props,
) {
if (__DEV__) {
if (pendingProps.rel !== 'font' && pendingProps.rel !== 'style') {
if (currentProps != null) {
const originalResourceName =
typeof currentProps.href === 'string'
? `Resource with href "${currentProps.href}"`
: 'Resource';
const originalRelStatement = getValueDescriptorExpectingEnumForWarning(
currentProps.rel,
);
const pendingRelStatement = getValueDescriptorExpectingEnumForWarning(
pendingProps.rel,
);
const pendingHrefStatement =
typeof pendingProps.href === 'string'
? ` and the updated href is "${pendingProps.href}"`
: '';
console.error(
'A <link> previously rendered as a %s but was updated with a rel type that is not' +
' valid for a Resource type. Generally Resources are not expected to ever have updated' +
' props however in some limited circumstances it can be valid when changing the href.' +
' When React encounters props that invalidate the Resource it is the same as not rendering' +
' a Resource at all. valid rel types for Resources are "font" and "style". The previous' +
' rel for this instance was %s. The updated rel is %s%s.',
originalResourceName,
originalRelStatement,
pendingRelStatement,
pendingHrefStatement,
);
} else {
const pendingRelStatement = getValueDescriptorExpectingEnumForWarning(
pendingProps.rel,
);
console.error(
'A <link> is rendering as a Resource but has an invalid rel property. The rel encountered is %s.' +
' This is a bug in React.',
pendingRelStatement,
);
}
if (currentProps != null) {
const originalResourceName =
typeof currentProps.href === 'string'
? `Resource with href "${currentProps.href}"`
: 'Resource';
const originalRelStatement = getValueDescriptorExpectingEnumForWarning(
currentProps.rel,
);
const pendingRelStatement = getValueDescriptorExpectingEnumForWarning(
pendingProps.rel,
);
const pendingHrefStatement =
typeof pendingProps.href === 'string'
? ` and the updated href is "${pendingProps.href}"`
: '';
console.error(
'A <link> previously rendered as a %s but was updated with a rel type that is not' +
' valid for a Resource type. Generally Resources are not expected to ever have updated' +
' props however in some limited circumstances it can be valid when changing the href.' +
' When React encounters props that invalidate the Resource it is the same as not rendering' +
' a Resource at all. valid rel types for Resources are "stylesheet" and "preload". The previous' +
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I fixed an erroneous wording here but otherwise I simply removed the wrapping if block with non-sensical condition test

' rel for this instance was %s. The updated rel is %s%s.',
originalResourceName,
originalRelStatement,
pendingRelStatement,
pendingHrefStatement,
);
} else {
const pendingRelStatement = getValueDescriptorExpectingEnumForWarning(
pendingProps.rel,
);
console.error(
'A <link> is rendering as a Resource but has an invalid rel property. The rel encountered is %s.' +
' This is a bug in React.',
pendingRelStatement,
);
}
}
}
Expand Down Expand Up @@ -517,6 +515,7 @@ export function validatePreloadArguments(href: mixed, options: mixed) {
}
break;
}
case 'script':
case 'style': {
break;
}
Expand All @@ -529,7 +528,7 @@ export function validatePreloadArguments(href: mixed, options: mixed) {
' Please use one of the following valid values instead: %s. The href for the preload call where this' +
' warning originated is "%s".',
typeOfAs,
'"style" and "font"',
'"style", "font", or "script"',
href,
);
}
Expand Down Expand Up @@ -557,7 +556,6 @@ export function validatePreinitArguments(href: mixed, options: mixed) {
} else {
const as = options.as;
switch (as) {
case 'font':
case 'style': {
break;
}
Expand Down
106 changes: 100 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMFloat-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ describe('ReactDOMFloat', () => {
' valid for a Resource type. Generally Resources are not expected to ever have updated' +
' props however in some limited circumstances it can be valid when changing the href.' +
' When React encounters props that invalidate the Resource it is the same as not rendering' +
' a Resource at all. valid rel types for Resources are "font" and "style". The previous' +
' a Resource at all. valid rel types for Resources are "stylesheet" and "preload". The previous' +
' rel for this instance was "stylesheet". The updated rel is "author" and the updated href is "bar".',
);
expect(getVisibleChildren(document)).toEqual(
Expand Down Expand Up @@ -407,6 +407,96 @@ describe('ReactDOMFloat', () => {
</html>,
);
});

it('supports script preloads', async () => {
function ServerApp() {
ReactDOM.preload('foo', {as: 'script', integrity: 'foo hash'});
ReactDOM.preload('bar', {
as: 'script',
crossOrigin: 'use-credentials',
integrity: 'bar hash',
});
return (
<html>
<link rel="preload" href="baz" as="script" />
<head>
<title>hi</title>
</head>
<body>foo</body>
</html>
);
}
function ClientApp() {
ReactDOM.preload('foo', {as: 'script', integrity: 'foo hash'});
ReactDOM.preload('qux', {as: 'script'});
return (
<html>
<head>
<title>hi</title>
</head>
<body>foo</body>
<link
rel="preload"
href="quux"
as="script"
crossOrigin=""
integrity="quux hash"
/>
</html>
);
}

await actIntoEmptyDocument(() => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<ServerApp />);
pipe(writable);
});
expect(getVisibleChildren(document)).toEqual(
<html>
<head>
<link rel="preload" as="script" href="foo" integrity="foo hash" />
<link
rel="preload"
as="script"
href="bar"
crossorigin="use-credentials"
integrity="bar hash"
/>
<link rel="preload" as="script" href="baz" />
<title>hi</title>
</head>
<body>foo</body>
</html>,
);

ReactDOMClient.hydrateRoot(document, <ClientApp />);
expect(Scheduler).toFlushWithoutYielding();

expect(getVisibleChildren(document)).toEqual(
<html>
<head>
<link rel="preload" as="script" href="foo" integrity="foo hash" />
<link
rel="preload"
as="script"
href="bar"
crossorigin="use-credentials"
integrity="bar hash"
/>
<link rel="preload" as="script" href="baz" />
<title>hi</title>
<link rel="preload" as="script" href="qux" />
<link
rel="preload"
as="script"
href="quux"
crossorigin=""
integrity="quux hash"
/>
</head>
<body>foo</body>
</html>,
);
});
});

describe('ReactDOM.preinit as style', () => {
Expand Down Expand Up @@ -2885,7 +2975,11 @@ describe('ReactDOMFloat', () => {
(mockError, scenarioNumber) => {
if (__DEV__) {
expect(mockError.mock.calls[scenarioNumber]).toEqual(
makeArgs('undefined', '"style" and "font"', 'foo'),
makeArgs(
'undefined',
'"style", "font", or "script"',
'foo',
),
);
} else {
expect(mockError).not.toHaveBeenCalled();
Expand All @@ -2898,7 +2992,7 @@ describe('ReactDOMFloat', () => {
(mockError, scenarioNumber) => {
if (__DEV__) {
expect(mockError.mock.calls[scenarioNumber]).toEqual(
makeArgs('null', '"style" and "font"', 'bar'),
makeArgs('null', '"style", "font", or "script"', 'bar'),
);
} else {
expect(mockError).not.toHaveBeenCalled();
Expand All @@ -2913,7 +3007,7 @@ describe('ReactDOMFloat', () => {
expect(mockError.mock.calls[scenarioNumber]).toEqual(
makeArgs(
'something with type "number"',
'"style" and "font"',
'"style", "font", or "script"',
'baz',
),
);
Expand All @@ -2930,7 +3024,7 @@ describe('ReactDOMFloat', () => {
expect(mockError.mock.calls[scenarioNumber]).toEqual(
makeArgs(
'something with type "object"',
'"style" and "font"',
'"style", "font", or "script"',
'qux',
),
);
Expand All @@ -2945,7 +3039,7 @@ describe('ReactDOMFloat', () => {
(mockError, scenarioNumber) => {
if (__DEV__) {
expect(mockError.mock.calls[scenarioNumber]).toEqual(
makeArgs('"bar"', '"style" and "font"', 'quux'),
makeArgs('"bar"', '"style", "font", or "script"', 'quux'),
);
} else {
expect(mockError).not.toHaveBeenCalled();
Expand Down