Skip to content

Commit e66e7c0

Browse files
gnoffgnoff
committed
Improve Hydration tolerance in <head> and <body> + support itemScope for hoisting
One recent decision was to make elements using the `itemProp` prop not hoistable if they were inside and itemScope. This better fits with Microdata spec which allows for meta tags and other tag types usually reserved for the <head> to be used in the <body> when using itemScope. To implement this a number of small changes were necessary 1. HostContext in prod needed to expand beyond just tracking the element namespace for new element creation. It now tracks whether we are in an itemScope. To keep this efficient it is modeled as a bitmask. 2. To disambiguate what is and is not a potential instance in the DOM for hoistables the hydration algo was updated to skip past non-matching instances while attempting to claim the instance rather than ahead of time (getNextHydratable). 3. React will not consider an itemScope on <html>, <head>, or <body> as a valid scope for the hoisting opt-out. This is important as an invariant so we can make assumptiosn about certain tags in these scopes. This should not be a functional breaking change because if any of these tags have an itemScope then it can just be moved into the first node inside the <body> Since we were already updating the logic for hydration to better support itemScope opt-out I also changed the hydration behavior for suspected 3rd party nodes in <head> and <body>. Now if you are hydrating in either of those contexts hydration will skip past any non-matching nodes until it finds a match. This allows 3rd party scripts and extensions to inject nodes in either context that React does not expect and still avoid a hydation mismatch. This new algorithm isn't perfect and it is possible for a mismatch to occurr. The most glarying case may be if a 3rd party script prepends a <div> into <body> and you render a <div> in <body> in your app. there is nothing to signal to React that this div was 3rd party so it will claim is as the hydrated instance and hydration will almost certainly fail immediately afterwards. The expectation is that this is rare and that if falling back to client rendering is transparent to the user then there is not problem here. We will continue to evaluate this and may change the hydration matchign algorithm further to match user and developer expectations
1 parent b2ae9dd commit e66e7c0

16 files changed

+954
-315
lines changed

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

Lines changed: 85 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
* @flow
88
*/
99

10+
import type {ExoticNamespace} from '../shared/DOMNamespaces';
11+
1012
import {
1113
registrationNameDependencies,
1214
possibleRegistrationNames,
@@ -375,11 +377,11 @@ function updateDOMProperties(
375377
}
376378
}
377379

378-
export function createElement(
380+
// Creates elements in the HTML namesapce
381+
export function createHTMLElement(
379382
type: string,
380383
props: Object,
381384
rootContainerElement: Element | Document | DocumentFragment,
382-
parentNamespace: string,
383385
): Element {
384386
let isCustomComponentTag;
385387

@@ -388,99 +390,102 @@ export function createElement(
388390
const ownerDocument: Document =
389391
getOwnerDocumentFromRootContainer(rootContainerElement);
390392
let domElement: Element;
391-
let namespaceURI = parentNamespace;
392-
if (namespaceURI === HTML_NAMESPACE) {
393-
namespaceURI = getIntrinsicNamespace(type);
393+
if (__DEV__) {
394+
isCustomComponentTag = isCustomComponent(type, props);
395+
// Should this check be gated by parent namespace? Not sure we want to
396+
// allow <SVG> or <mATH>.
397+
if (!isCustomComponentTag && type !== type.toLowerCase()) {
398+
console.error(
399+
'<%s /> is using incorrect casing. ' +
400+
'Use PascalCase for React components, ' +
401+
'or lowercase for HTML elements.',
402+
type,
403+
);
404+
}
394405
}
395-
if (namespaceURI === HTML_NAMESPACE) {
406+
407+
if (type === 'script') {
408+
// Create the script via .innerHTML so its "parser-inserted" flag is
409+
// set to true and it does not execute
410+
const div = ownerDocument.createElement('div');
396411
if (__DEV__) {
397-
isCustomComponentTag = isCustomComponent(type, props);
398-
// Should this check be gated by parent namespace? Not sure we want to
399-
// allow <SVG> or <mATH>.
400-
if (!isCustomComponentTag && type !== type.toLowerCase()) {
412+
if (enableTrustedTypesIntegration && !didWarnScriptTags) {
401413
console.error(
402-
'<%s /> is using incorrect casing. ' +
403-
'Use PascalCase for React components, ' +
404-
'or lowercase for HTML elements.',
405-
type,
414+
'Encountered a script tag while rendering React component. ' +
415+
'Scripts inside React components are never executed when rendering ' +
416+
'on the client. Consider using template tag instead ' +
417+
'(https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template).',
406418
);
419+
didWarnScriptTags = true;
407420
}
408421
}
409-
410-
if (type === 'script') {
411-
// Create the script via .innerHTML so its "parser-inserted" flag is
412-
// set to true and it does not execute
413-
const div = ownerDocument.createElement('div');
414-
if (__DEV__) {
415-
if (enableTrustedTypesIntegration && !didWarnScriptTags) {
416-
console.error(
417-
'Encountered a script tag while rendering React component. ' +
418-
'Scripts inside React components are never executed when rendering ' +
419-
'on the client. Consider using template tag instead ' +
420-
'(https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template).',
421-
);
422-
didWarnScriptTags = true;
423-
}
424-
}
425-
div.innerHTML = '<script><' + '/script>'; // eslint-disable-line
426-
// This is guaranteed to yield a script element.
427-
const firstChild = ((div.firstChild: any): HTMLScriptElement);
428-
domElement = div.removeChild(firstChild);
429-
} else if (typeof props.is === 'string') {
430-
domElement = ownerDocument.createElement(type, {is: props.is});
431-
} else {
432-
// Separate else branch instead of using `props.is || undefined` above because of a Firefox bug.
433-
// See discussion in https://github.com/facebook/react/pull/6896
434-
// and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
435-
domElement = ownerDocument.createElement(type);
436-
// Normally attributes are assigned in `setInitialDOMProperties`, however the `multiple` and `size`
437-
// attributes on `select`s needs to be added before `option`s are inserted.
438-
// This prevents:
439-
// - a bug where the `select` does not scroll to the correct option because singular
440-
// `select` elements automatically pick the first item #13222
441-
// - a bug where the `select` set the first item as selected despite the `size` attribute #14239
442-
// See https://github.com/facebook/react/issues/13222
443-
// and https://github.com/facebook/react/issues/14239
444-
if (type === 'select') {
445-
const node = ((domElement: any): HTMLSelectElement);
446-
if (props.multiple) {
447-
node.multiple = true;
448-
} else if (props.size) {
449-
// Setting a size greater than 1 causes a select to behave like `multiple=true`, where
450-
// it is possible that no option is selected.
451-
//
452-
// This is only necessary when a select in "single selection mode".
453-
node.size = props.size;
454-
}
422+
div.innerHTML = '<script><' + '/script>'; // eslint-disable-line
423+
// This is guaranteed to yield a script element.
424+
const firstChild = ((div.firstChild: any): HTMLScriptElement);
425+
domElement = div.removeChild(firstChild);
426+
} else if (typeof props.is === 'string') {
427+
domElement = ownerDocument.createElement(type, {is: props.is});
428+
} else {
429+
// Separate else branch instead of using `props.is || undefined` above because of a Firefox bug.
430+
// See discussion in https://github.com/facebook/react/pull/6896
431+
// and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
432+
domElement = ownerDocument.createElement(type);
433+
// Normally attributes are assigned in `setInitialDOMProperties`, however the `multiple` and `size`
434+
// attributes on `select`s needs to be added before `option`s are inserted.
435+
// This prevents:
436+
// - a bug where the `select` does not scroll to the correct option because singular
437+
// `select` elements automatically pick the first item #13222
438+
// - a bug where the `select` set the first item as selected despite the `size` attribute #14239
439+
// See https://github.com/facebook/react/issues/13222
440+
// and https://github.com/facebook/react/issues/14239
441+
if (type === 'select') {
442+
const node = ((domElement: any): HTMLSelectElement);
443+
if (props.multiple) {
444+
node.multiple = true;
445+
} else if (props.size) {
446+
// Setting a size greater than 1 causes a select to behave like `multiple=true`, where
447+
// it is possible that no option is selected.
448+
//
449+
// This is only necessary when a select in "single selection mode".
450+
node.size = props.size;
455451
}
456452
}
457-
} else {
458-
domElement = ownerDocument.createElementNS(namespaceURI, type);
459453
}
460454

461455
if (__DEV__) {
462-
if (namespaceURI === HTML_NAMESPACE) {
463-
if (
464-
!isCustomComponentTag &&
465-
// $FlowFixMe[method-unbinding]
466-
Object.prototype.toString.call(domElement) ===
467-
'[object HTMLUnknownElement]' &&
468-
!hasOwnProperty.call(warnedUnknownTags, type)
469-
) {
470-
warnedUnknownTags[type] = true;
471-
console.error(
472-
'The tag <%s> is unrecognized in this browser. ' +
473-
'If you meant to render a React component, start its name with ' +
474-
'an uppercase letter.',
475-
type,
476-
);
477-
}
456+
if (
457+
!isCustomComponentTag &&
458+
// $FlowFixMe[method-unbinding]
459+
Object.prototype.toString.call(domElement) ===
460+
'[object HTMLUnknownElement]' &&
461+
!hasOwnProperty.call(warnedUnknownTags, type)
462+
) {
463+
warnedUnknownTags[type] = true;
464+
console.error(
465+
'The tag <%s> is unrecognized in this browser. ' +
466+
'If you meant to render a React component, start its name with ' +
467+
'an uppercase letter.',
468+
type,
469+
);
478470
}
479471
}
480472

481473
return domElement;
482474
}
483475

476+
// Creates elements in the HTML either SVG or Math namespace
477+
export function createElementNS(
478+
namespaceURI: ExoticNamespace,
479+
type: string,
480+
rootContainerElement: Element | Document | DocumentFragment,
481+
): Element {
482+
// This
483+
const ownerDocument: Document =
484+
getOwnerDocumentFromRootContainer(rootContainerElement);
485+
486+
return ownerDocument.createElementNS(namespaceURI, type);
487+
}
488+
484489
export function createTextNode(
485490
text: string,
486491
rootContainerElement: Element | Document | DocumentFragment,
@@ -864,9 +869,9 @@ export function diffHydratedProperties(
864869
domElement: Element,
865870
tag: string,
866871
rawProps: Object,
867-
parentNamespace: string,
868872
isConcurrentMode: boolean,
869873
shouldWarnDev: boolean,
874+
namespaceDEV?: string,
870875
): null | Array<mixed> {
871876
let isCustomComponentTag;
872877
let extraAttributeNames: Set<string>;
@@ -1109,11 +1114,7 @@ export function diffHydratedProperties(
11091114
propertyInfo,
11101115
);
11111116
} else {
1112-
let ownNamespace = parentNamespace;
1113-
if (ownNamespace === HTML_NAMESPACE) {
1114-
ownNamespace = getIntrinsicNamespace(tag);
1115-
}
1116-
if (ownNamespace === HTML_NAMESPACE) {
1117+
if (namespaceDEV === HTML_NAMESPACE) {
11171118
// $FlowFixMe - Should be inferred as not undefined.
11181119
extraAttributeNames.delete(propKey.toLowerCase());
11191120
} else {

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

Lines changed: 13 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
getValueDescriptorExpectingObjectForWarning,
2222
getValueDescriptorExpectingEnumForWarning,
2323
} from '../shared/ReactDOMResourceValidation';
24-
import {createElement, setInitialProperties} from './ReactDOMComponent';
24+
import {createHTMLElement, setInitialProperties} from './ReactDOMComponent';
2525
import {
2626
checkAttributeStringCoercion,
2727
checkPropStringCoercion,
@@ -289,11 +289,10 @@ function preload(href: string, options: PreloadOptions) {
289289
preloadPropsMap.set(key, preloadProps);
290290

291291
if (null === ownerDocument.querySelector(preloadKey)) {
292-
const preloadInstance = createElement(
292+
const preloadInstance = createHTMLElement(
293293
'link',
294294
preloadProps,
295295
ownerDocument,
296-
HTML_NAMESPACE,
297296
);
298297
setInitialProperties(preloadInstance, 'link', preloadProps);
299298
markNodeAsResource(preloadInstance);
@@ -371,11 +370,10 @@ function preinit(href: string, options: PreinitOptions) {
371370
preloadPropsMap.set(key, preloadProps);
372371

373372
if (null === preloadDocument.querySelector(preloadKey)) {
374-
const preloadInstance = createElement(
373+
const preloadInstance = createHTMLElement(
375374
'link',
376375
preloadProps,
377376
preloadDocument,
378-
HTML_NAMESPACE,
379377
);
380378
setInitialProperties(preloadInstance, 'link', preloadProps);
381379
markNodeAsResource(preloadInstance);
@@ -417,12 +415,7 @@ function preinit(href: string, options: PreinitOptions) {
417415
if (preloadProps) {
418416
adoptPreloadPropsForStylesheet(stylesheetProps, preloadProps);
419417
}
420-
instance = createElement(
421-
'link',
422-
stylesheetProps,
423-
resourceRoot,
424-
HTML_NAMESPACE,
425-
);
418+
instance = createHTMLElement('link', stylesheetProps, resourceRoot);
426419
markNodeAsResource(instance);
427420
setInitialProperties(instance, 'link', stylesheetProps);
428421
insertStylesheet(instance, precedence, resourceRoot);
@@ -463,12 +456,7 @@ function preinit(href: string, options: PreinitOptions) {
463456
if (preloadProps) {
464457
adoptPreloadPropsForScript(scriptProps, preloadProps);
465458
}
466-
instance = createElement(
467-
'script',
468-
scriptProps,
469-
resourceRoot,
470-
HTML_NAMESPACE,
471-
);
459+
instance = createHTMLElement('script', scriptProps, resourceRoot);
472460
markNodeAsResource(instance);
473461
setInitialProperties(instance, 'link', scriptProps);
474462
(getDocumentFromRoot(resourceRoot).head: any).appendChild(instance);
@@ -703,11 +691,10 @@ function preloadStylesheet(
703691
null ===
704692
ownerDocument.querySelector(getPreloadStylesheetSelectorFromKey(key))
705693
) {
706-
const preloadInstance = createElement(
694+
const preloadInstance = createHTMLElement(
707695
'link',
708696
preloadProps,
709697
ownerDocument,
710-
HTML_NAMESPACE,
711698
);
712699
setInitialProperties(preloadInstance, 'link', preloadProps);
713700
markNodeAsResource(preloadInstance);
@@ -762,16 +749,12 @@ export function acquireResource(
762749
);
763750
if (instance) {
764751
resource.instance = instance;
752+
markNodeAsResource(instance);
765753
return instance;
766754
}
767755

768756
const styleProps = styleTagPropsFromRawProps(props);
769-
instance = createElement(
770-
'style',
771-
styleProps,
772-
hoistableRoot,
773-
HTML_NAMESPACE,
774-
);
757+
instance = createHTMLElement('style', styleProps, hoistableRoot);
775758

776759
markNodeAsResource(instance);
777760
setInitialProperties(instance, 'style', styleProps);
@@ -793,6 +776,7 @@ export function acquireResource(
793776
);
794777
if (instance) {
795778
resource.instance = instance;
779+
markNodeAsResource(instance);
796780
return instance;
797781
}
798782

@@ -803,12 +787,7 @@ export function acquireResource(
803787
}
804788

805789
// Construct and insert a new instance
806-
instance = createElement(
807-
'link',
808-
stylesheetProps,
809-
hoistableRoot,
810-
HTML_NAMESPACE,
811-
);
790+
instance = createHTMLElement('link', stylesheetProps, hoistableRoot);
812791
markNodeAsResource(instance);
813792
const linkInstance: HTMLLinkElement = (instance: any);
814793
(linkInstance: any)._p = new Promise((resolve, reject) => {
@@ -837,6 +816,7 @@ export function acquireResource(
837816
);
838817
if (instance) {
839818
resource.instance = instance;
819+
markNodeAsResource(instance);
840820
return instance;
841821
}
842822

@@ -848,12 +828,7 @@ export function acquireResource(
848828
}
849829

850830
// Construct and insert a new instance
851-
instance = createElement(
852-
'script',
853-
scriptProps,
854-
hoistableRoot,
855-
HTML_NAMESPACE,
856-
);
831+
instance = createHTMLElement('script', scriptProps, hoistableRoot);
857832
markNodeAsResource(instance);
858833
setInitialProperties(instance, 'link', scriptProps);
859834
(getDocumentFromRoot(hoistableRoot).head: any).appendChild(instance);
@@ -1092,7 +1067,7 @@ export function hydrateHoistable(
10921067
}
10931068

10941069
// There is no matching instance to hydrate, we create it now
1095-
const instance = createElement(type, props, ownerDocument, HTML_NAMESPACE);
1070+
const instance = createHTMLElement(type, props, ownerDocument);
10961071
setInitialProperties(instance, type, props);
10971072
precacheFiberNode(internalInstanceHandle, instance);
10981073
markNodeAsResource(instance);

0 commit comments

Comments
 (0)