Skip to content

Commit 487535a

Browse files
gnoffgnoff
committed
remove the concept of itemScope and categorically treat itemProp as an opt out
1 parent c74769d commit 487535a

File tree

7 files changed

+213
-187
lines changed

7 files changed

+213
-187
lines changed

‎packages/react-dom-bindings/src/client/ReactDOMHostConfig.js

Lines changed: 129 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type {
1818
import type {ReactScopeInstance} from 'shared/ReactTypes';
1919
import type {AncestorInfoDev} from './validateDOMNesting';
2020

21+
import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
2122
import {
2223
precacheFiberNode,
2324
updateFiberProps,
@@ -111,7 +112,6 @@ export type Props = {
111112
left?: null | number,
112113
right?: null | number,
113114
top?: null | number,
114-
itemScope?: null | boolean,
115115
...
116116
};
117117
type RawProps = {
@@ -241,17 +241,15 @@ export function getChildHostContext(
241241
}
242242
}
243243

244-
const NoContext /* */ = 0b000000;
244+
const NoContext /* */ = 0b0000;
245245
// Element namespace contexts
246-
const HTMLNamespaceContext /* */ = 0b000001;
247-
const SVGNamespaceContext /* */ = 0b000010;
248-
const MathNamespaceContext /* */ = 0b000100;
249-
const AnyNamespaceContext /* */ = 0b000111;
246+
const HTMLNamespaceContext /* */ = 0b0001;
247+
const SVGNamespaceContext /* */ = 0b0010;
248+
const MathNamespaceContext /* */ = 0b0100;
249+
const AnyNamespaceContext /* */ = 0b0111;
250250

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

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

283275
const namespaceContext = context & AnyNamespaceContext;
284276
const otherContext = context & ~AnyNamespaceContext;
@@ -293,12 +285,9 @@ function getChildHostContextImpl(
293285
case 'math':
294286
return MathNamespaceContext | otherContext;
295287
case 'html':
296-
return HTMLNamespaceContext | otherContext | HoistContext;
297288
case 'head':
298289
case 'body':
299-
return (
300-
HTMLNamespaceContext | otherContext | HoistContext | HeadOrBodyInScope
301-
);
290+
return HTMLNamespaceContext | otherContext | HoistContext;
302291
default:
303292
return context;
304293
}
@@ -934,14 +923,12 @@ export function isHydratable(type: string, props: Props): boolean {
934923
// In certain contexts, namely <body> and <head>, we want to skip past Nodes that are in theory
935924
// hydratable but do not match the current Fiber being hydrated. We track the hydratable node we
936925
// are currently attempting using this module global. If the hydration is unsuccessful Fiber will
937-
// call getNextHydratableAfterFailedAttempt which uses this cursor to return the expected next
926+
// call getLastAttemptedHydratable which uses this cursor to return the expected next
938927
// hydratable.
939928
let hydratableNode: null | HydratableInstance = null;
940929

941-
export function getNextHydratableAfterFailedAttempt(): null | HydratableInstance {
942-
return hydratableNode === null
943-
? null
944-
: getNextHydratableSibling(hydratableNode);
930+
export function getLastAttemptedHydratable(): null | HydratableInstance {
931+
return hydratableNode;
945932
}
946933

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

@@ -980,7 +968,6 @@ export function getNextMatchingHydratableInstance(
980968
) {
981969
// This is either text or a tag type that differs from the tag we are trying to hydrate
982970
// or a Node we already bound to a hoistable. We skip past it.
983-
hydratableNode = getNextHydratableSibling(node);
984971
continue;
985972
} else {
986973
// We have an Element with the right type.
@@ -991,68 +978,135 @@ export function getNextMatchingHydratableInstance(
991978
// using high entropy attributes for certain types. This technique will fail for strange insertions like
992979
// extension prepending <div> in the <body> but that already breaks before and that is an edge case.
993980
switch (type) {
994-
// We make a leap and assume that no titles or metas will ever hydrate as components in the <head> or <body>
995-
// This is because the only way they would opt out of hoisting semantics is to be in svg, which is not possible
996-
// in these scopes, or to have an itemProp while being in an itemScope. We define <head>, <body>, and <html> as not
997-
// supporting itemScope to make this a strict guarantee. Because we can never be in this position we elide the check here.
998-
// case 'title':
999-
// case 'meta': {
1000-
// continue;
1001-
// }
981+
// case 'title': We assume all titles are matchable. You should only have one in the Document, at least in a hoistable scope
982+
// 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.
983+
case 'meta': {
984+
if (__DEV__) {
985+
checkAttributeStringCoercion(anyProps.content, 'content');
986+
}
987+
if (
988+
element.getAttribute('content') !==
989+
(anyProps.content == null ? null : '' + anyProps.content)
990+
) {
991+
// Content is coerced to a string because it is similar to value. If something with toString is provided
992+
// we want to get that string value before doing the comparison. The rest are less likely to be used in this fashion
993+
// and we just check identity
994+
continue;
995+
}
996+
if (
997+
element.getAttribute('name') !==
998+
(anyProps.name == null ? null : anyProps.name) ||
999+
element.getAttribute('property') !==
1000+
(anyProps.property == null ? null : anyProps.property) ||
1001+
element.getAttribute('itemprop') !==
1002+
(anyProps.itemProp == null ? null : anyProps.itemProp) ||
1003+
element.getAttribute('http-equiv') !==
1004+
(anyProps.httpEquiv == null ? null : anyProps.httpEquiv) ||
1005+
element.getAttribute('charset') !==
1006+
(anyProps.charSet == null ? null : anyProps.charSet)
1007+
) {
1008+
continue;
1009+
}
1010+
return element;
1011+
}
10021012
case 'link': {
10031013
const rel = element.getAttribute('rel');
10041014
if (
10051015
rel === 'stylesheet' &&
10061016
element.hasAttribute('data-precedence')
10071017
) {
1008-
// This is a stylesheet resource and necessarily not a target for hydration of a component
1018+
// This is a stylesheet resource
10091019
continue;
10101020
} else if (
1011-
rel !== (props: any).rel ||
1012-
element.getAttribute('href') !== (props: any).href
1021+
rel !== anyProps.rel ||
1022+
element.getAttribute('href') !==
1023+
(anyProps.href == null ? null : anyProps.href) ||
1024+
element.getAttribute('crossorigin') !==
1025+
(anyProps.crossOrigin == null ? null : anyProps.crossOrigin) ||
1026+
element.getAttribute('title') !==
1027+
(anyProps.title == null ? null : anyProps.title)
10131028
) {
1014-
// This link Node is definitely not a match for this rendered link
1029+
// rel + href should usually be enough to uniquely identify a link however crossOrigin can vary for rel preconnect
1030+
// and title could vary for rel alternate
10151031
continue;
10161032
}
1017-
break;
1033+
return element;
10181034
}
10191035
case 'style': {
10201036
if (element.hasAttribute('data-precedence')) {
1021-
// This i a style resource necessarily not a target for hydration of a component
1022-
continue;
1023-
} else if (
1024-
typeof props.children === 'string' &&
1025-
element.textContent !== props.children
1026-
) {
1027-
// The contents of this style Node do not match the contents of this rendered style
1037+
// This is a style resource
10281038
continue;
10291039
}
1040+
// We need to fall through to hit the textContent comparison case below. This switch is structured so that you either
1041+
// continue, or return, unless you need to do this child comparison.
10301042
break;
10311043
}
10321044
case 'script': {
1033-
if (element.hasAttribute('async')) {
1034-
// This is an async script resource and necessarily not a target for hydration of a compoennt
1045+
const srcAttr = element.getAttribute('src');
1046+
if (
1047+
srcAttr &&
1048+
element.hasAttribute('async') &&
1049+
!element.hasAttribute('itemprop')
1050+
) {
1051+
// This is an async script resource
10351052
continue;
10361053
} else if (
1037-
typeof (props: any).src === 'string' &&
1038-
element.getAttribute('src') !== (props: any).src
1054+
srcAttr !== (anyProps.src == null ? null : anyProps.src) ||
1055+
element.getAttribute('type') !==
1056+
(anyProps.type == null ? null : anyProps.type) ||
1057+
element.getAttribute('crossorigin') !==
1058+
(anyProps.crossOrigin == null ? null : anyProps.crossOrigin)
10391059
) {
10401060
// This script is for a different src
10411061
continue;
10421062
} else if (
1043-
typeof props.children === 'string' &&
1044-
element.textContent !== props.children
1063+
typeof anyProps.children === 'string' &&
1064+
element.textContent !== anyProps.children
10451065
) {
10461066
// This is an inline script with different text content
10471067
continue;
10481068
}
1069+
// We need to fall through to hit the textContent comparison case below. This switch is structured so that you either
1070+
// continue, or return, unless you need to do this child comparison.
10491071
break;
10501072
}
1073+
default: {
1074+
// We have excluded the most likely cases of mismatch between hoistable tags, 3rd party script inserted tags,
1075+
// and browser extension inserted tags. While it is possible this is not the right match it is a decent hueristic
1076+
// that should work in the vast majority of cases.
1077+
return element;
1078+
}
10511079
}
10521080

1053-
// We have excluded the most likely cases of mismatch between hoistable tags, 3rd party script inserted tags,
1054-
// and browser extension inserted tags. While it is possible this is not the right match it is a decent hueristic
1055-
// that should work in the vast majority of cases.
1081+
// The only branches that should fall through to here are those that need to check textContent against single value children
1082+
// in particular, <style>, and <script>
1083+
const children = props.children;
1084+
const innerHTML = props.dangerouslySetInnerHTML;
1085+
const html =
1086+
innerHTML && typeof innerHTML === 'object' ? innerHTML.__html : null;
1087+
1088+
// If we don't have a child, try html instead. We don't validate any of the props here because
1089+
// they will get validated during `hydrateInstance()`. Note that this only makes sense because
1090+
// <script> and <style> can only have text content. this is not suitable for hydrating arbitrary tags
1091+
// that might use dangerouslySetInnerHTML
1092+
const child =
1093+
children != null
1094+
? Array.isArray(children)
1095+
? children.length < 2
1096+
? children[0]
1097+
: null
1098+
: children
1099+
: html;
1100+
if (
1101+
typeof child !== 'function' &&
1102+
typeof child !== 'symbol' &&
1103+
child !== null &&
1104+
child !== undefined &&
1105+
// eslint-disable-next-line react-internal/safe-string-coercion
1106+
element.textContent !== '' + (child: any)
1107+
) {
1108+
continue;
1109+
}
10561110
return element;
10571111
}
10581112
}
@@ -1818,7 +1872,26 @@ export function isHostHoistableType(
18181872
}
18191873

18201874
// Global opt out of hoisting for anything in SVG Namespace or anything with an itemProp inside an itemScope
1821-
if (context & ItemInScope || context & SVGNamespaceContext) {
1875+
if (context & SVGNamespaceContext || props.itemProp != null) {
1876+
if (__DEV__) {
1877+
if (
1878+
outsideHostContainerContext &&
1879+
props.itemProp != null &&
1880+
(type === 'meta' ||
1881+
type === 'title' ||
1882+
type === 'style' ||
1883+
type === 'link' ||
1884+
type === 'script')
1885+
) {
1886+
console.error(
1887+
'Cannot render a <%s> outside the main document if it has an `itemProp` prop. `itemProp` suggests the tag belongs to an' +
1888+
' `itemScope` which can appear anywhere in the DOM. If you were intending for React to hoist this <%s> remove the `itemProp` prop.' +
1889+
' Otherwise, try moving this tag into the <head> or <body> of the Document.',
1890+
type,
1891+
type,
1892+
);
1893+
}
1894+
}
18221895
return false;
18231896
}
18241897

@@ -1999,12 +2072,6 @@ export function resolveSingletonInstance(
19992072
if (validateDOMNestingDev) {
20002073
validateDOMNesting(type, null, hostContextDev.ancestorInfo);
20012074
}
2002-
if (hostContextDev.context & ItemInScope && props.itemScope === true) {
2003-
console.error(
2004-
'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.',
2005-
type,
2006-
);
2007-
}
20082075
}
20092076
const ownerDocument = getOwnerDocumentFromRootContainer(
20102077
rootContainerInstance,

0 commit comments

Comments
 (0)