Skip to content

Commit c1f8a52

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 4111002 commit c1f8a52

16 files changed

+997
-337
lines changed

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

Lines changed: 86 additions & 85 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,
@@ -55,7 +57,7 @@ import {
5557
setValueForStyles,
5658
validateShorthandPropertyCollisionInDev,
5759
} from './CSSPropertyOperations';
58-
import {HTML_NAMESPACE, getIntrinsicNamespace} from '../shared/DOMNamespaces';
60+
import {HTML_NAMESPACE} from '../shared/DOMNamespaces';
5961
import {
6062
getPropertyInfo,
6163
shouldIgnoreAttribute,
@@ -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 {

0 commit comments

Comments
 (0)