From f11a0e05c2c625e595b3ded9c719af7da7a4e163 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Fri, 21 Oct 2022 22:38:24 -0700 Subject: [PATCH] [Float] support all links as Resources (#25515) stacked on https://github.com/facebook/react/pull/25514 This PR adds support for any type of Link as long as it has a string rel and href and does not include an onLoad or onError property. The semantics for generic link resources matches other head resources, they will be inserted and removed as their ref counts go positive and back to zero. Keys are based on rel, href, sizes, and media. on the server preconnect and prefetch-dns are privileged and will emit near the start of the stream. --- .../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..91a37bfc1f0dc 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 'dns-prefetch': { + 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..7c31700cdafed 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 "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 + 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(() => {