From 77447c6f2aef02374a6456ea7c565765507631c2 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 94f52ed46f318..d9ee68db00295 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 05b1326ac052b..4b5c3310ad5ad 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 43c85deda79b6..887b07cc1eab9 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 f451eb6623658..20e389a726a94 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 ab1b9dfefac13..2a35b100b7341 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(() => {