From 9f7339aa0488a07da66bd8caa60bcdb0d33d324d Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 19 Oct 2022 13:17:35 -0700 Subject: [PATCH 1/2] Support all links as potential Resources every link that does not have an onLoad or onError prop and has a string href and string rel can be a resource on the server some of these generic link resources are privileged such as preconnect and prefetch-dns where they will be ordered at a higher priority for flushing. These generic link types key off rel, href, sizes, and media. In addition to these changes. preloads can now work for any as type. If the type is not for a recognized resource type then they fall back to generic link resource behavior. --- .../src/client/ReactDOMFloatClient.js | 103 ++++++++-- .../src/server/ReactDOMFloatServer.js | 50 ++++- .../src/server/ReactDOMServerFormatConfig.js | 24 +++ .../src/shared/ReactDOMResourceValidation.js | 62 +++--- .../src/__tests__/ReactDOMFloat-test.js | 182 +++++++++++++++++- 5 files changed, 371 insertions(+), 50 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js b/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js index 67c44b102074d..c9681bb27c836 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js +++ b/packages/react-dom-bindings/src/client/ReactDOMFloatClient.js @@ -13,7 +13,7 @@ import ReactDOMSharedInternals from 'shared/ReactDOMSharedInternals.js'; const {Dispatcher} = ReactDOMSharedInternals; import {DOCUMENT_NODE} from '../shared/HTMLNodeType'; import { - validateUnmatchedLinkResourceProps, + warnOnMissingHrefAndRel, validatePreloadResourceDifference, validateURLKeyedUpdatedProps, validateStyleResourceDifference, @@ -54,7 +54,7 @@ type StyleProps = { 'data-precedence': string, [string]: mixed, }; -export type StyleResource = { +type StyleResource = { type: 'style', // Ref count for resource @@ -79,7 +79,7 @@ type ScriptProps = { src: string, [string]: mixed, }; -export type ScriptResource = { +type ScriptResource = { type: 'script', src: string, props: ScriptProps, @@ -88,12 +88,10 @@ export type ScriptResource = { root: FloatRoot, }; -export type HeadResource = TitleResource | MetaResource; - type TitleProps = { [string]: mixed, }; -export type TitleResource = { +type TitleResource = { type: 'title', props: TitleProps, @@ -105,7 +103,7 @@ export type TitleResource = { type MetaProps = { [string]: mixed, }; -export type MetaResource = { +type MetaResource = { type: 'meta', matcher: string, property: ?string, @@ -117,8 +115,23 @@ export type MetaResource = { root: Document, }; +type LinkProps = { + href: string, + rel: string, + [string]: mixed, +}; +type LinkResource = { + type: 'link', + props: LinkProps, + + count: number, + instance: ?Element, + root: Document, +}; + type Props = {[string]: mixed}; +type HeadResource = TitleResource | MetaResource | LinkResource; type Resource = StyleResource | ScriptResource | PreloadResource | HeadResource; export type RootResources = { @@ -617,8 +630,30 @@ export function getResource( return null; } default: { + const {href, sizes, media} = pendingProps; + if (typeof rel === 'string' && typeof href === 'string') { + const sizeKey = + '::sizes:' + (typeof sizes === 'string' ? sizes : ''); + const mediaKey = + '::media:' + (typeof media === 'string' ? media : ''); + const key = 'rel:' + rel + '::href:' + href + sizeKey + mediaKey; + const headRoot = getDocumentFromRoot(resourceRoot); + const headResources = getResourcesFromRoot(headRoot).head; + let resource = headResources.get(key); + if (!resource) { + resource = { + type: 'link', + props: Object.assign({}, pendingProps), + count: 0, + instance: null, + root: headRoot, + }; + headResources.set(key, resource); + } + return resource; + } if (__DEV__) { - validateUnmatchedLinkResourceProps(pendingProps, currentProps); + warnOnMissingHrefAndRel(pendingProps, currentProps); } return null; } @@ -710,6 +745,7 @@ function scriptPropsFromRawProps(rawProps: ScriptQualifyingProps): ScriptProps { export function acquireResource(resource: Resource): Instance { switch (resource.type) { case 'title': + case 'link': case 'meta': { return acquireHeadResource(resource); } @@ -732,6 +768,7 @@ export function acquireResource(resource: Resource): Instance { export function releaseResource(resource: Resource): void { switch (resource.type) { + case 'link': case 'title': case 'meta': { return releaseHeadResource(resource); @@ -1050,6 +1087,41 @@ function acquireHeadResource(resource: HeadResource): Instance { insertResourceInstanceBefore(root, instance, insertBefore); break; } + case 'link': { + const linkProps: LinkProps = (props: any); + const limitedEscapedRel = escapeSelectorAttributeValueInsideDoubleQuotes( + linkProps.rel, + ); + const limitedEscapedHref = escapeSelectorAttributeValueInsideDoubleQuotes( + linkProps.href, + ); + let selector = `link[rel="${limitedEscapedRel}"][href="${limitedEscapedHref}"]`; + if (typeof linkProps.sizes === 'string') { + const limitedEscapedSizes = escapeSelectorAttributeValueInsideDoubleQuotes( + linkProps.sizes, + ); + selector += `[sizes="${limitedEscapedSizes}"]`; + } + if (typeof linkProps.media === 'string') { + const limitedEscapedMedia = escapeSelectorAttributeValueInsideDoubleQuotes( + linkProps.media, + ); + selector += `[media="${limitedEscapedMedia}"]`; + } + const existingEl = root.querySelector(selector); + if (existingEl) { + instance = resource.instance = existingEl; + markNodeAsResource(instance); + return instance; + } + instance = resource.instance = createResourceInstance( + type, + props, + root, + ); + insertResourceInstanceBefore(root, instance, null); + return instance; + } default: { throw new Error( `acquireHeadResource encountered a resource type it did not expect: "${type}". This is a bug in React.`, @@ -1265,26 +1337,27 @@ export function isHostResourceType(type: string, props: Props): boolean { return true; } case 'link': { + const {onLoad, onError} = props; + if (onLoad || onError) { + return false; + } switch (props.rel) { case 'stylesheet': { if (__DEV__) { validateLinkPropsForStyleResource(props); } - const {href, precedence, onLoad, onError, disabled} = props; + const {href, precedence, disabled} = props; return ( typeof href === 'string' && typeof precedence === 'string' && - !onLoad && - !onError && disabled == null ); } - case 'preload': { - const {href, onLoad, onError} = props; - return !onLoad && !onError && typeof href === 'string'; + default: { + const {rel, href} = props; + return typeof href === 'string' && typeof rel === 'string'; } } - return false; } case 'script': { // We don't validate because it is valid to use async with onLoad/onError unlike combining diff --git a/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js b/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js index e71f8e465ce85..18ac6988c04ac 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js +++ b/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js @@ -89,8 +89,20 @@ type MetaResource = { flushed: boolean, }; +type LinkProps = { + href: string, + rel: string, + [string]: mixed, +}; +type LinkResource = { + type: 'link', + props: LinkProps, + + flushed: boolean, +}; + export type Resource = PreloadResource | StyleResource | ScriptResource; -export type HeadResource = TitleResource | MetaResource; +export type HeadResource = TitleResource | MetaResource | LinkResource; export type Resources = { // Request local cache @@ -101,6 +113,7 @@ export type Resources = { // Flushing queues for Resource dependencies charset: null | MetaResource, + preconnects: Set, fontPreloads: Set, // usedImagePreloads: Set, precedences: Map>, @@ -131,6 +144,7 @@ export function createResources(): Resources { // cleared on flush charset: null, + preconnects: new Set(), fontPreloads: new Set(), // usedImagePreloads: new Set(), precedences: new Map(), @@ -697,10 +711,11 @@ export function resourcesFromLink(props: Props): boolean { const resources = currentResources; const {rel, href} = props; - if (!href || typeof href !== 'string') { + if (!href || typeof href !== 'string' || !rel || typeof rel !== 'string') { return false; } + let key = ''; switch (rel) { case 'stylesheet': { const {onLoad, onError, precedence, disabled} = props; @@ -813,10 +828,37 @@ export function resourcesFromLink(props: Props): boolean { return true; } } - return false; + break; } } - return false; + if (props.onLoad || props.onError) { + return false; + } + + const sizes = typeof props.sizes === 'string' ? props.sizes : ''; + const media = typeof props.media === 'string' ? props.media : ''; + key = + 'rel:' + rel + '::href:' + href + '::sizes:' + sizes + '::media:' + media; + let resource = resources.headsMap.get(key); + if (!resource) { + resource = { + type: 'link', + props: Object.assign({}, props), + flushed: false, + }; + resources.headsMap.set(key, resource); + switch (rel) { + case 'preconnect': + case 'prefetch-dns': { + resources.preconnects.add(resource); + break; + } + default: { + resources.headResources.add(resource); + } + } + } + return true; } // Construct a resource from link props. diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index 181ac82f9194f..c62945a02520a 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -2340,6 +2340,7 @@ export function writeInitialResources( const { charset, + preconnects, fontPreloads, precedences, usedStylePreloads, @@ -2356,6 +2357,13 @@ export function writeInitialResources( resources.charset = null; } + preconnects.forEach(r => { + // font preload Resources should not already be flushed so we elide this check + pushLinkImpl(target, r.props, responseState); + r.flushed = true; + }); + preconnects.clear(); + fontPreloads.forEach(r => { // font preload Resources should not already be flushed so we elide this check pushLinkImpl(target, r.props, responseState); @@ -2418,6 +2426,10 @@ export function writeInitialResources( pushSelfClosing(target, r.props, 'meta', responseState); break; } + case 'link': { + pushLinkImpl(target, r.props, responseState); + break; + } } r.flushed = true; }); @@ -2450,6 +2462,7 @@ export function writeImmediateResources( const { charset, + preconnects, fontPreloads, usedStylePreloads, scripts, @@ -2465,6 +2478,13 @@ export function writeImmediateResources( resources.charset = null; } + preconnects.forEach(r => { + // font preload Resources should not already be flushed so we elide this check + pushLinkImpl(target, r.props, responseState); + r.flushed = true; + }); + preconnects.clear(); + fontPreloads.forEach(r => { // font preload Resources should not already be flushed so we elide this check pushLinkImpl(target, r.props, responseState); @@ -2507,6 +2527,10 @@ export function writeImmediateResources( pushSelfClosing(target, r.props, 'meta', responseState); break; } + case 'link': { + pushLinkImpl(target, r.props, responseState); + break; + } } r.flushed = true; }); diff --git a/packages/react-dom-bindings/src/shared/ReactDOMResourceValidation.js b/packages/react-dom-bindings/src/shared/ReactDOMResourceValidation.js index 8d5fda5c243d6..bd6f527cecc6c 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMResourceValidation.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMResourceValidation.js @@ -11,7 +11,7 @@ import hasOwnProperty from 'shared/hasOwnProperty'; type Props = {[string]: mixed}; -export function validateUnmatchedLinkResourceProps( +export function warnOnMissingHrefAndRel( pendingProps: Props, currentProps: ?Props, ) { @@ -24,34 +24,52 @@ export function validateUnmatchedLinkResourceProps( const originalRelStatement = getValueDescriptorExpectingEnumForWarning( currentProps.rel, ); - const pendingRelStatement = getValueDescriptorExpectingEnumForWarning( + const pendingRel = getValueDescriptorExpectingEnumForWarning( pendingProps.rel, ); - const pendingHrefStatement = - typeof pendingProps.href === 'string' - ? ` and the updated href is "${pendingProps.href}"` - : ''; - console.error( - 'A 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' + - ' rel for this instance was %s. The updated rel is %s%s.', - originalResourceName, - originalRelStatement, - pendingRelStatement, - pendingHrefStatement, + const pendingHref = getValueDescriptorExpectingEnumForWarning( + pendingProps.href, ); + if (typeof pendingProps.rel !== 'string') { + console.error( + 'A previously rendered as a %s with rel "%s" but was updated with an invalid rel: %s. When a link' + + ' does not have a valid rel prop it is not represented in the DOM. If this is intentional, instead' + + ' do not render the anymore.', + originalResourceName, + originalRelStatement, + pendingRel, + ); + } else if (typeof pendingProps.href !== 'string') { + console.error( + 'A previously rendered as a %s but was updated with an invalid href prop: %s. When a link' + + ' does not have a valid href prop it is not represented in the DOM. If this is intentional, instead' + + ' do not render the anymore.', + originalResourceName, + pendingHref, + ); + } } else { - const pendingRelStatement = getValueDescriptorExpectingEnumForWarning( + const pendingRel = getValueDescriptorExpectingEnumForWarning( pendingProps.rel, ); - console.error( - 'A is rendering as a Resource but has an invalid rel property. The rel encountered is %s.' + - ' This is a bug in React.', - pendingRelStatement, + const pendingHref = getValueDescriptorExpectingEnumForWarning( + pendingProps.href, ); + if (typeof pendingProps.rel !== 'string') { + console.error( + 'A is rendering with an invalid rel: %s. When a link' + + ' does not have a valid rel prop it is not represented in the DOM. If this is intentional, instead' + + ' do not render the anymore.', + pendingRel, + ); + } else if (typeof pendingProps.href !== 'string') { + console.error( + 'A is rendering with an invalid href: %s. When a link' + + ' does not have a valid href prop it is not represented in the DOM. If this is intentional, instead' + + ' do not render the anymore.', + pendingHref, + ); + } } } } diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 0ef44d9743319..4112a1e194224 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -389,29 +389,29 @@ describe('ReactDOMFloat', () => { root.render(
+
, ); expect(Scheduler).toFlushWithoutYielding(); root.render(
- + {}} href="bar" /> + {}} />
, ); expect(() => { expect(Scheduler).toFlushWithoutYielding(); - }).toErrorDev( - 'Warning: A previously rendered as a Resource with href "foo" 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' + - ' rel for this instance was "stylesheet". The updated rel is "author" and the updated href is "bar".', - ); + }).toErrorDev([ + 'Warning: A previously rendered as a Resource with href "foo" with rel ""stylesheet"" but was updated with an invalid rel: something with type "function". When a link does not have a valid rel prop it is not represented in the DOM. If this is intentional, instead do not render the anymore.', + 'Warning: A previously rendered as a Resource with href "bar" but was updated with an invalid href prop: something with type "function". When a link does not have a valid href prop it is not represented in the DOM. If this is intentional, instead do not render the anymore.', + ]); expect(getMeaningfulChildren(document)).toEqual( + +
@@ -941,6 +941,170 @@ describe('ReactDOMFloat', () => { }); describe('head resources', () => { + // @gate enableFloat + it('supports preconnects, prefetc-dns, and arbitrary other link types', async () => { + await actIntoEmptyDocument(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + + + +
hello world
+ + + + + + + + + + + + + , + ); + pipe(writable); + }); + // "preconnect" and "prefetch-dns" get hoisted to the front. + // All other generic links (not styles, or typed preloads) + // get emitted after styles and other higher priority Resources + // Sizes and Media are part of generic link keys + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + + + + + + + + +
hello world
+ + , + ); + + const root = ReactDOMClient.hydrateRoot( + document, + + + +
hello world
+ + + + + + + + + + + + + , + ); + expect(Scheduler).toFlushWithoutYielding(); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + + + + + + + + +
hello world
+ + , + ); + + root.render( + + + +
hello world
+ + , + ); + expect(Scheduler).toFlushWithoutYielding(); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + +
hello world
+ + , + ); + }); + // @gate enableFloat + it('can render icons and apple-touch-icons as resources', async () => { + await actIntoEmptyDocument(() => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + <> + + + + +
hello world
+ + + + , + ); + pipe(writable); + }); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + +
hello world
+ + , + ); + + ReactDOMClient.hydrateRoot( + document, + + + + + +
hello world
+ + , + ); + expect(Scheduler).toFlushWithoutYielding(); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + +
hello world
+ + , + ); + }); + // @gate enableFloat it('can hydrate the right instances for deeply nested structured metas', async () => { await actIntoEmptyDocument(() => { From 8d26e62b2987740e8bd27f3e63a2483dff943e62 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Fri, 21 Oct 2022 15:32:52 -0700 Subject: [PATCH 2/2] prefetch-dns -> dns-prefetch --- .../src/server/ReactDOMFloatServer.js | 2 +- packages/react-dom/src/__tests__/ReactDOMFloat-test.js | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js b/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js index 18ac6988c04ac..91a37bfc1f0dc 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js +++ b/packages/react-dom-bindings/src/server/ReactDOMFloatServer.js @@ -849,7 +849,7 @@ export function resourcesFromLink(props: Props): boolean { resources.headsMap.set(key, resource); switch (rel) { case 'preconnect': - case 'prefetch-dns': { + case 'dns-prefetch': { resources.preconnects.add(resource); break; } diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 4112a1e194224..7c31700cdafed 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -955,7 +955,7 @@ describe('ReactDOMFloat', () => { - + @@ -965,7 +965,7 @@ describe('ReactDOMFloat', () => { ); pipe(writable); }); - // "preconnect" and "prefetch-dns" get hoisted to the front. + // "preconnect" and "dns-prefetch" get hoisted to the front. // All other generic links (not styles, or typed preloads) // get emitted after styles and other higher priority Resources // Sizes and Media are part of generic link keys @@ -973,7 +973,7 @@ describe('ReactDOMFloat', () => { - + @@ -1001,7 +1001,7 @@ describe('ReactDOMFloat', () => { - + @@ -1014,7 +1014,7 @@ describe('ReactDOMFloat', () => { - +