Skip to content

Commit

Permalink
[Float] handle resource Resource creation inside svg context (#25599)
Browse files Browse the repository at this point in the history 
`title` is a valid element descendent of `svg`. this PR adds a
prohibition on turning titles in svg into Resources.

This PR also adds additional warnings if you render something that is
almost a Resource inside an svg.
  • Loading branch information
@gnoff
gnoff authored Nov 1, 2022
1 parent 36426e6 commit 5f7ef8c
Show file tree
Hide file tree
Showing 4 changed files with 449 additions and 21 deletions.
100 changes: 81 additions & 19 deletions packages/react-dom-bindings/src/client/ReactDOMFloatClient.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import {
getHostContext,
} from 'react-reconciler/src/ReactFiberHostContext';
import {getResourceFormOnly} from './validateDOMNesting';
import {getNamespace} from './ReactDOMHostConfig';
import {SVG_NAMESPACE} from '../shared/DOMNamespaces';

// 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
Expand Down Expand Up @@ -201,6 +203,28 @@ function getCurrentResourceRoot(): null | FloatRoot {
return currentContainer ? currentContainer.getRootNode() : null;
}

// This resource type constraint can be loosened. It really is everything except PreloadResource
// because that is the only one that does not have an optional instance type. Expand as needed.
function resetInstance(resource: ScriptResource | HeadResource) {
resource.instance = undefined;
}

export function clearRootResources(rootContainer: Container): void {
const rootNode: FloatRoot = (rootContainer.getRootNode(): any);
const resources = getResourcesFromRoot(rootNode);

// We can't actually delete the resource cache because this function is called
// during commit after we have rendered. Instead we detatch any instances from
// the Resource object if they are going to be cleared

// Styles stay put
// Scripts get reset
resources.scripts.forEach(resetInstance);
// Head Resources get reset
resources.head.forEach(resetInstance);
// lastStructuredMeta stays put
}

// Preloads are somewhat special. Even if we don't have the Document
// used by the root that is rendering a component trying to insert a preload
// we can still seed the file cache by doing the preload on any document we have
Expand Down Expand Up @@ -1077,7 +1101,14 @@ function acquireHeadResource(resource: HeadResource): Instance {
props,
root,
);
insertResourceInstanceBefore(root, instance, titles.item(0));
const firstTitle = titles[0];
insertResourceInstanceBefore(
root,
instance,
firstTitle && firstTitle.namespaceURI !== SVG_NAMESPACE
? firstTitle
: null,
);
break;
}
case 'meta': {
Expand Down Expand Up @@ -1397,16 +1428,21 @@ function insertResourceInstanceBefore(

export function isHostResourceType(type: string, props: Props): boolean {
let resourceFormOnly: boolean;
let namespace: string;
if (__DEV__) {
const hostContext = getHostContext();
resourceFormOnly = getResourceFormOnly(hostContext);
namespace = getNamespace(hostContext);
}
switch (type) {
case 'base':
case 'meta':
case 'title': {
case 'meta': {
return true;
}
case 'title': {
const hostContext = getHostContext();
return getNamespace(hostContext) !== SVG_NAMESPACE;
}
case 'link': {
const {onLoad, onError} = props;
if (onLoad || onError) {
Expand All @@ -1417,6 +1453,11 @@ export function isHostResourceType(type: string, props: Props): boolean {
' Try removing onLoad={...} and onError={...} or moving it into the root <head> tag or' +
' somewhere in the <body>.',
);
} else if (namespace === SVG_NAMESPACE) {
console.error(
'Cannot render a <link> with onLoad or onError listeners as a descendent of <svg>.' +
' Try removing onLoad={...} and onError={...} or moving it above the <svg> ancestor.',
);
}
}
return false;
Expand All @@ -1426,11 +1467,18 @@ export function isHostResourceType(type: string, props: Props): boolean {
const {href, precedence, disabled} = props;
if (__DEV__) {
validateLinkPropsForStyleResource(props);
if (typeof precedence !== 'string' && resourceFormOnly) {
console.error(
'Cannot render a <link rel="stylesheet" /> outside the main document without knowing its precedence.' +
' Consider adding precedence="default" or moving it into the root <head> tag.',
);
if (typeof precedence !== 'string') {
if (resourceFormOnly) {
console.error(
'Cannot render a <link rel="stylesheet" /> outside the main document without knowing its precedence.' +
' Consider adding precedence="default" or moving it into the root <head> tag.',
);
} else if (namespace === SVG_NAMESPACE) {
console.error(
'Cannot render a <link rel="stylesheet" /> as a descendent of an <svg> element without knowing its precedence.' +
' Consider adding precedence="default" or moving it above the <svg> ancestor.',
);
}
}
}
return (
Expand All @@ -1450,17 +1498,31 @@ export function isHostResourceType(type: string, props: Props): boolean {
// precedence with these for style resources
const {src, async, onLoad, onError} = props;
if (__DEV__) {
if (async !== true && resourceFormOnly) {
console.error(
'Cannot render a sync or defer <script> outside the main document without knowing its order.' +
' Try adding async="" or moving it into the root <head> tag.',
);
} else if ((onLoad || onError) && resourceFormOnly) {
console.error(
'Cannot render a <script> with onLoad or onError listeners outside the main document.' +
' Try removing onLoad={...} and onError={...} or moving it into the root <head> tag or' +
' somewhere in the <body>.',
);
if (async !== true) {
if (resourceFormOnly) {
console.error(
'Cannot render a sync or defer <script> outside the main document without knowing its order.' +
' Try adding async="" or moving it into the root <head> tag.',
);
} else if (namespace === SVG_NAMESPACE) {
console.error(
'Cannot render a sync or defer <script> as a descendent of an <svg> element.' +
' Try adding async="" or moving it above the ancestor <svg> element.',
);
}
} else if (onLoad || onError) {
if (resourceFormOnly) {
console.error(
'Cannot render a <script> with onLoad or onError listeners outside the main document.' +
' Try removing onLoad={...} and onError={...} or moving it into the root <head> tag or' +
' somewhere in the <body>.',
);
} else if (namespace === SVG_NAMESPACE) {
console.error(
'Cannot render a <script> with onLoad or onError listeners as a descendent of an <svg> element.' +
' Try removing onLoad={...} and onError={...} or moving it above the ancestor <svg> element.',
);
}
}
}
return (async: any) && typeof src === 'string' && !onLoad && !onError;
Expand Down
33 changes: 32 additions & 1 deletion packages/react-dom-bindings/src/client/ReactDOMHostConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ import {ConcurrentMode, NoMode} from 'react-reconciler/src/ReactTypeOfMode';
import {
prepareToRenderResources,
cleanupAfterRenderResources,
clearRootResources,
isHostResourceType,
} from './ReactDOMFloatClient';

Expand Down Expand Up @@ -219,6 +220,16 @@ export function getPublicInstance(instance: Instance): Instance {
return instance;
}

export function getNamespace(hostContext: HostContext): string {
if (__DEV__) {
const hostContextDev: HostContextDev = (hostContext: any);
return hostContextDev.namespace;
} else {
const hostContextProd: HostContextProd = (hostContext: any);
return hostContextProd;
}
}

export function prepareForCommit(containerInfo: Container): Object | null {
eventsEnabled = ReactBrowserEventEmitterIsEnabled();
selectionInformation = getSelectionInformation();
Expand Down Expand Up @@ -715,11 +726,18 @@ export function clearContainer(container: Container): void {
if (enableHostSingletons) {
const nodeType = container.nodeType;
if (nodeType === DOCUMENT_NODE) {
clearRootResources(container);
clearContainerSparingly(container);
} else if (nodeType === ELEMENT_NODE) {
switch (container.nodeName) {
case 'HEAD': {
// If we are clearing document.head as a container we are essentially clearing everything
// that was hoisted to the head and should forget the instances that will no longer be in the DOM
clearRootResources(container);
// fall through to clear child contents
}
// eslint-disable-next-line-no-fallthrough
case 'HTML':
case 'HEAD':
case 'BODY':
clearContainerSparingly(container);
return;
Expand Down Expand Up @@ -910,6 +928,19 @@ function getNextHydratable(node) {
if (nodeType === ELEMENT_NODE) {
const element: Element = (node: any);
switch (element.tagName) {
// This is subtle. in SVG scope the title tag is case sensitive. we don't want to skip
// titles in svg but we do want to skip them outside of svg. there is an edge case where
// you could do `React.createElement('TITLE', ...)` inside an svg scope but the SSR serializer
// will still emit lowercase. Practically speaking the only time the DOM will have a non-uppercased
// title tagName is if it is inside an svg.
// Other Resource types like META, BASE, LINK, and SCRIPT should be treated as resources even inside
// svg scope because they are invalid otherwise. We still don't need to handle the lowercase variant
// because if they are present in the DOM already they would have been hoisted outside the SVG scope
// as Resources. So while it would be correct to skip a <link> inside <svg> and this algorithm won't
// skip that link because the tagName will not be uppercased it functionally is irrelevant. If one
// tries to render incompatible types such as a non-resource stylesheet inside an svg the server will
// emit that invalid html and hydration will fail. In Dev this will present warnings guiding the
// developer on how to fix.
case 'TITLE':
case 'META':
case 'BASE':
Expand Down
4 changes: 4 additions & 0 deletions packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1371,6 +1371,7 @@ function pushTitle(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
responseState: ResponseState,
insertionMode: InsertionMode,
noscriptTagInScope: boolean,
): ReactNodeList {
if (__DEV__) {
Expand Down Expand Up @@ -1415,6 +1416,8 @@ function pushTitle(

if (
enableFloat &&
// title is valid in SVG so we avoid resour
insertionMode !== SVG_MODE &&
!noscriptTagInScope &&
resourcesFromElement('title', props)
) {
Expand Down Expand Up @@ -1926,6 +1929,7 @@ export function pushStartInstance(
target,
props,
responseState,
formatContext.insertionMode,
formatContext.noscriptTagInScope,
)
: pushStartTitle(target, props, responseState);
Expand Down
Loading

1 comment on commit 5f7ef8c

@netrolite
Copy link

Choose a reason for hiding this comment

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

cool

Please sign in to comment.