Skip to content

Commit

Permalink
remove the concept of itemScope and categorically treat itemProp as a…
Browse files Browse the repository at this point in the history 
…n opt out
  • Loading branch information
@gnoff
gnoff committed Mar 4, 2023
1 parent c74769d commit 487535a
Show file tree
Hide file tree
Showing 7 changed files with 213 additions and 187 deletions.
191 changes: 129 additions & 62 deletions packages/react-dom-bindings/src/client/ReactDOMHostConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {
import type {ReactScopeInstance} from 'shared/ReactTypes';
import type {AncestorInfoDev} from './validateDOMNesting';

import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
import {
precacheFiberNode,
updateFiberProps,
Expand Down Expand Up @@ -111,7 +112,6 @@ export type Props = {
left?: null | number,
right?: null | number,
top?: null | number,
itemScope?: null | boolean,
...
};
type RawProps = {
Expand Down Expand Up @@ -241,17 +241,15 @@ export function getChildHostContext(
}
}

const NoContext /* */ = 0b000000;
const NoContext /* */ = 0b0000;
// Element namespace contexts
const HTMLNamespaceContext /* */ = 0b000001;
const SVGNamespaceContext /* */ = 0b000010;
const MathNamespaceContext /* */ = 0b000100;
const AnyNamespaceContext /* */ = 0b000111;
const HTMLNamespaceContext /* */ = 0b0001;
const SVGNamespaceContext /* */ = 0b0010;
const MathNamespaceContext /* */ = 0b0100;
const AnyNamespaceContext /* */ = 0b0111;

// Ancestor tag/attribute contexts
const HoistContext /* */ = 0b001000; // When we are directly inside the HostRoot or inside certain tags like <html>, <head>, and <body>
const ItemInScope /* */ = 0b010000; // When we are inside an itemScope deeper than <body>
const HeadOrBodyInScope /* */ = 0b100000; // When we are inside a <head> or <body>
const HoistContext /* */ = 0b1000; // When we are directly inside the HostRoot or inside certain tags like <html>, <head>, and <body>

function getNamespaceContext(namespaceURI: string) {
switch (namespaceURI) {
Expand All @@ -272,13 +270,7 @@ function getChildHostContextImpl(
props: null | Props,
) {
// We assume any child context is not a HoistContext. It will get re-added later if certain circumstances warrant it
let context = parentContext & ~HoistContext;
// We need to determine if we are now in an itemScope.
if (props && props.itemScope === true && parentContext & HeadOrBodyInScope) {
// We only allow itemscopes deeper than <head> or <body>. This is helpful to disambiguate
// resources that need to hoist vs those that do not
context |= ItemInScope;
}
const context = parentContext & ~HoistContext;

const namespaceContext = context & AnyNamespaceContext;
const otherContext = context & ~AnyNamespaceContext;
Expand All @@ -293,12 +285,9 @@ function getChildHostContextImpl(
case 'math':
return MathNamespaceContext | otherContext;
case 'html':
return HTMLNamespaceContext | otherContext | HoistContext;
case 'head':
case 'body':
return (
HTMLNamespaceContext | otherContext | HoistContext | HeadOrBodyInScope
);
return HTMLNamespaceContext | otherContext | HoistContext;
default:
return context;
}
Expand Down Expand Up @@ -934,14 +923,12 @@ export function isHydratable(type: string, props: Props): boolean {
// In certain contexts, namely <body> and <head>, we want to skip past Nodes that are in theory
// hydratable but do not match the current Fiber being hydrated. We track the hydratable node we
// are currently attempting using this module global. If the hydration is unsuccessful Fiber will
// call getNextHydratableAfterFailedAttempt which uses this cursor to return the expected next
// call getLastAttemptedHydratable which uses this cursor to return the expected next
// hydratable.
let hydratableNode: null | HydratableInstance = null;

export function getNextHydratableAfterFailedAttempt(): null | HydratableInstance {
return hydratableNode === null
? null
: getNextHydratableSibling(hydratableNode);
export function getLastAttemptedHydratable(): null | HydratableInstance {
return hydratableNode;
}

export function getNextMatchingHydratableInstance(
Expand All @@ -950,6 +937,7 @@ export function getNextMatchingHydratableInstance(
props: Props,
hostContext: HostContext,
): null | Instance {
const anyProps = (props: any);
// We set this first because it must always be set on every invocation
hydratableNode = instance;

Expand Down Expand Up @@ -980,7 +968,6 @@ export function getNextMatchingHydratableInstance(
) {
// This is either text or a tag type that differs from the tag we are trying to hydrate
// or a Node we already bound to a hoistable. We skip past it.
hydratableNode = getNextHydratableSibling(node);
continue;
} else {
// We have an Element with the right type.
Expand All @@ -991,68 +978,135 @@ export function getNextMatchingHydratableInstance(
// using high entropy attributes for certain types. This technique will fail for strange insertions like
// extension prepending <div> in the <body> but that already breaks before and that is an edge case.
switch (type) {
// We make a leap and assume that no titles or metas will ever hydrate as components in the <head> or <body>
// This is because the only way they would opt out of hoisting semantics is to be in svg, which is not possible
// in these scopes, or to have an itemProp while being in an itemScope. We define <head>, <body>, and <html> as not
// supporting itemScope to make this a strict guarantee. Because we can never be in this position we elide the check here.
// case 'title':
// case 'meta': {
// continue;
// }
// case 'title': We assume all titles are matchable. You should only have one in the Document, at least in a hoistable scope
// and if you are a HostComponent with type title we must either be in an <svg> context or this title must have an `itemProp` prop.
case 'meta': {
if (__DEV__) {
checkAttributeStringCoercion(anyProps.content, 'content');
}
if (
element.getAttribute('content') !==
(anyProps.content == null ? null : '' + anyProps.content)
) {
// Content is coerced to a string because it is similar to value. If something with toString is provided
// we want to get that string value before doing the comparison. The rest are less likely to be used in this fashion
// and we just check identity
continue;
}
if (
element.getAttribute('name') !==
(anyProps.name == null ? null : anyProps.name) ||
element.getAttribute('property') !==
(anyProps.property == null ? null : anyProps.property) ||
element.getAttribute('itemprop') !==
(anyProps.itemProp == null ? null : anyProps.itemProp) ||
element.getAttribute('http-equiv') !==
(anyProps.httpEquiv == null ? null : anyProps.httpEquiv) ||
element.getAttribute('charset') !==
(anyProps.charSet == null ? null : anyProps.charSet)
) {
continue;
}
return element;
}
case 'link': {
const rel = element.getAttribute('rel');
if (
rel === 'stylesheet' &&
element.hasAttribute('data-precedence')
) {
// This is a stylesheet resource and necessarily not a target for hydration of a component
// This is a stylesheet resource
continue;
} else if (
rel !== (props: any).rel ||
element.getAttribute('href') !== (props: any).href
rel !== anyProps.rel ||
element.getAttribute('href') !==
(anyProps.href == null ? null : anyProps.href) ||
element.getAttribute('crossorigin') !==
(anyProps.crossOrigin == null ? null : anyProps.crossOrigin) ||
element.getAttribute('title') !==
(anyProps.title == null ? null : anyProps.title)
) {
// This link Node is definitely not a match for this rendered link
// rel + href should usually be enough to uniquely identify a link however crossOrigin can vary for rel preconnect
// and title could vary for rel alternate
continue;
}
break;
return element;
}
case 'style': {
if (element.hasAttribute('data-precedence')) {
// This i a style resource necessarily not a target for hydration of a component
continue;
} else if (
typeof props.children === 'string' &&
element.textContent !== props.children
) {
// The contents of this style Node do not match the contents of this rendered style
// This is a style resource
continue;
}
// We need to fall through to hit the textContent comparison case below. This switch is structured so that you either
// continue, or return, unless you need to do this child comparison.
break;
}
case 'script': {
if (element.hasAttribute('async')) {
// This is an async script resource and necessarily not a target for hydration of a compoennt
const srcAttr = element.getAttribute('src');
if (
srcAttr &&
element.hasAttribute('async') &&
!element.hasAttribute('itemprop')
) {
// This is an async script resource
continue;
} else if (
typeof (props: any).src === 'string' &&
element.getAttribute('src') !== (props: any).src
srcAttr !== (anyProps.src == null ? null : anyProps.src) ||
element.getAttribute('type') !==
(anyProps.type == null ? null : anyProps.type) ||
element.getAttribute('crossorigin') !==
(anyProps.crossOrigin == null ? null : anyProps.crossOrigin)
) {
// This script is for a different src
continue;
} else if (
typeof props.children === 'string' &&
element.textContent !== props.children
typeof anyProps.children === 'string' &&
element.textContent !== anyProps.children
) {
// This is an inline script with different text content
continue;
}
// We need to fall through to hit the textContent comparison case below. This switch is structured so that you either
// continue, or return, unless you need to do this child comparison.
break;
}
default: {
// We have excluded the most likely cases of mismatch between hoistable tags, 3rd party script inserted tags,
// and browser extension inserted tags. While it is possible this is not the right match it is a decent hueristic
// that should work in the vast majority of cases.
return element;
}
}

// We have excluded the most likely cases of mismatch between hoistable tags, 3rd party script inserted tags,
// and browser extension inserted tags. While it is possible this is not the right match it is a decent hueristic
// that should work in the vast majority of cases.
// The only branches that should fall through to here are those that need to check textContent against single value children
// in particular, <style>, and <script>
const children = props.children;
const innerHTML = props.dangerouslySetInnerHTML;
const html =
innerHTML && typeof innerHTML === 'object' ? innerHTML.__html : null;

// If we don't have a child, try html instead. We don't validate any of the props here because
// they will get validated during `hydrateInstance()`. Note that this only makes sense because
// <script> and <style> can only have text content. this is not suitable for hydrating arbitrary tags
// that might use dangerouslySetInnerHTML
const child =
children != null
? Array.isArray(children)
? children.length < 2
? children[0]
: null
: children
: html;
if (
typeof child !== 'function' &&
typeof child !== 'symbol' &&
child !== null &&
child !== undefined &&
// eslint-disable-next-line react-internal/safe-string-coercion
element.textContent !== '' + (child: any)
) {
continue;
}
return element;
}
}
Expand Down Expand Up @@ -1818,7 +1872,26 @@ export function isHostHoistableType(
}

// Global opt out of hoisting for anything in SVG Namespace or anything with an itemProp inside an itemScope
if (context & ItemInScope || context & SVGNamespaceContext) {
if (context & SVGNamespaceContext || props.itemProp != null) {
if (__DEV__) {
if (
outsideHostContainerContext &&
props.itemProp != null &&
(type === 'meta' ||
type === 'title' ||
type === 'style' ||
type === 'link' ||
type === 'script')
) {
console.error(
'Cannot render a <%s> outside the main document if it has an `itemProp` prop. `itemProp` suggests the tag belongs to an' +
' `itemScope` which can appear anywhere in the DOM. If you were intending for React to hoist this <%s> remove the `itemProp` prop.' +
' Otherwise, try moving this tag into the <head> or <body> of the Document.',
type,
type,
);
}
}
return false;
}

Expand Down Expand Up @@ -1999,12 +2072,6 @@ export function resolveSingletonInstance(
if (validateDOMNestingDev) {
validateDOMNesting(type, null, hostContextDev.ancestorInfo);
}
if (hostContextDev.context & ItemInScope && props.itemScope === true) {
console.error(
'React expected the <%s> element to not have an `itemScope` prop but found this prop instead. React does not support itemScopes on <html>, <head>, or <body> because it interferes with hoisting certain tags. Try moving the itemScope to an element just inside the <body> instead.',
type,
);
}
}
const ownerDocument = getOwnerDocumentFromRootContainer(
rootContainerInstance,
Expand Down
Loading

0 comments on commit 487535a

Please sign in to comment.