Skip to content

Commit

Permalink
Improve Hydration tolerance in <head> and <body> + support itemScope …
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@gnoff
gnoff committed Mar 3, 2023
1 parent 1f1f8eb commit c74769d
Show file tree
Hide file tree
Showing 16 changed files with 997 additions and 337 deletions.
171 changes: 86 additions & 85 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* @flow
*/

import type {ExoticNamespace} from '../shared/DOMNamespaces';

import {
registrationNameDependencies,
possibleRegistrationNames,
Expand Down Expand Up @@ -55,7 +57,7 @@ import {
setValueForStyles,
validateShorthandPropertyCollisionInDev,
} from './CSSPropertyOperations';
import {HTML_NAMESPACE, getIntrinsicNamespace} from '../shared/DOMNamespaces';
import {HTML_NAMESPACE} from '../shared/DOMNamespaces';
import {
getPropertyInfo,
shouldIgnoreAttribute,
Expand Down Expand Up @@ -375,11 +377,11 @@ function updateDOMProperties(
}
}

export function createElement(
// Creates elements in the HTML namesapce
export function createHTMLElement(
type: string,
props: Object,
rootContainerElement: Element | Document | DocumentFragment,
parentNamespace: string,
): Element {
let isCustomComponentTag;

Expand All @@ -388,99 +390,102 @@ export function createElement(
const ownerDocument: Document =
getOwnerDocumentFromRootContainer(rootContainerElement);
let domElement: Element;
let namespaceURI = parentNamespace;
if (namespaceURI === HTML_NAMESPACE) {
namespaceURI = getIntrinsicNamespace(type);
if (__DEV__) {
isCustomComponentTag = isCustomComponent(type, props);
// Should this check be gated by parent namespace? Not sure we want to
// allow <SVG> or <mATH>.
if (!isCustomComponentTag && type !== type.toLowerCase()) {
console.error(
'<%s /> is using incorrect casing. ' +
'Use PascalCase for React components, ' +
'or lowercase for HTML elements.',
type,
);
}
}
if (namespaceURI === HTML_NAMESPACE) {

if (type === 'script') {
// Create the script via .innerHTML so its "parser-inserted" flag is
// set to true and it does not execute
const div = ownerDocument.createElement('div');
if (__DEV__) {
isCustomComponentTag = isCustomComponent(type, props);
// Should this check be gated by parent namespace? Not sure we want to
// allow <SVG> or <mATH>.
if (!isCustomComponentTag && type !== type.toLowerCase()) {
if (enableTrustedTypesIntegration && !didWarnScriptTags) {
console.error(
'<%s /> is using incorrect casing. ' +
'Use PascalCase for React components, ' +
'or lowercase for HTML elements.',
type,
'Encountered a script tag while rendering React component. ' +
'Scripts inside React components are never executed when rendering ' +
'on the client. Consider using template tag instead ' +
'(https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template).',
);
didWarnScriptTags = true;
}
}

if (type === 'script') {
// Create the script via .innerHTML so its "parser-inserted" flag is
// set to true and it does not execute
const div = ownerDocument.createElement('div');
if (__DEV__) {
if (enableTrustedTypesIntegration && !didWarnScriptTags) {
console.error(
'Encountered a script tag while rendering React component. ' +
'Scripts inside React components are never executed when rendering ' +
'on the client. Consider using template tag instead ' +
'(https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template).',
);
didWarnScriptTags = true;
}
}
div.innerHTML = '<script><' + '/script>'; // eslint-disable-line
// This is guaranteed to yield a script element.
const firstChild = ((div.firstChild: any): HTMLScriptElement);
domElement = div.removeChild(firstChild);
} else if (typeof props.is === 'string') {
domElement = ownerDocument.createElement(type, {is: props.is});
} else {
// Separate else branch instead of using `props.is || undefined` above because of a Firefox bug.
// See discussion in https://github.com/facebook/react/pull/6896
// and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
domElement = ownerDocument.createElement(type);
// Normally attributes are assigned in `setInitialDOMProperties`, however the `multiple` and `size`
// attributes on `select`s needs to be added before `option`s are inserted.
// This prevents:
// - a bug where the `select` does not scroll to the correct option because singular
// `select` elements automatically pick the first item #13222
// - a bug where the `select` set the first item as selected despite the `size` attribute #14239
// See https://github.com/facebook/react/issues/13222
// and https://github.com/facebook/react/issues/14239
if (type === 'select') {
const node = ((domElement: any): HTMLSelectElement);
if (props.multiple) {
node.multiple = true;
} else if (props.size) {
// Setting a size greater than 1 causes a select to behave like `multiple=true`, where
// it is possible that no option is selected.
//
// This is only necessary when a select in "single selection mode".
node.size = props.size;
}
div.innerHTML = '<script><' + '/script>'; // eslint-disable-line
// This is guaranteed to yield a script element.
const firstChild = ((div.firstChild: any): HTMLScriptElement);
domElement = div.removeChild(firstChild);
} else if (typeof props.is === 'string') {
domElement = ownerDocument.createElement(type, {is: props.is});
} else {
// Separate else branch instead of using `props.is || undefined` above because of a Firefox bug.
// See discussion in https://github.com/facebook/react/pull/6896
// and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
domElement = ownerDocument.createElement(type);
// Normally attributes are assigned in `setInitialDOMProperties`, however the `multiple` and `size`
// attributes on `select`s needs to be added before `option`s are inserted.
// This prevents:
// - a bug where the `select` does not scroll to the correct option because singular
// `select` elements automatically pick the first item #13222
// - a bug where the `select` set the first item as selected despite the `size` attribute #14239
// See https://github.com/facebook/react/issues/13222
// and https://github.com/facebook/react/issues/14239
if (type === 'select') {
const node = ((domElement: any): HTMLSelectElement);
if (props.multiple) {
node.multiple = true;
} else if (props.size) {
// Setting a size greater than 1 causes a select to behave like `multiple=true`, where
// it is possible that no option is selected.
//
// This is only necessary when a select in "single selection mode".
node.size = props.size;
}
}
} else {
domElement = ownerDocument.createElementNS(namespaceURI, type);
}

if (__DEV__) {
if (namespaceURI === HTML_NAMESPACE) {
if (
!isCustomComponentTag &&
// $FlowFixMe[method-unbinding]
Object.prototype.toString.call(domElement) ===
'[object HTMLUnknownElement]' &&
!hasOwnProperty.call(warnedUnknownTags, type)
) {
warnedUnknownTags[type] = true;
console.error(
'The tag <%s> is unrecognized in this browser. ' +
'If you meant to render a React component, start its name with ' +
'an uppercase letter.',
type,
);
}
if (
!isCustomComponentTag &&
// $FlowFixMe[method-unbinding]
Object.prototype.toString.call(domElement) ===
'[object HTMLUnknownElement]' &&
!hasOwnProperty.call(warnedUnknownTags, type)
) {
warnedUnknownTags[type] = true;
console.error(
'The tag <%s> is unrecognized in this browser. ' +
'If you meant to render a React component, start its name with ' +
'an uppercase letter.',
type,
);
}
}

return domElement;
}

// Creates elements in the HTML either SVG or Math namespace
export function createElementNS(
namespaceURI: ExoticNamespace,
type: string,
rootContainerElement: Element | Document | DocumentFragment,
): Element {
// This
const ownerDocument: Document =
getOwnerDocumentFromRootContainer(rootContainerElement);

return ownerDocument.createElementNS(namespaceURI, type);
}

export function createTextNode(
text: string,
rootContainerElement: Element | Document | DocumentFragment,
Expand Down Expand Up @@ -864,9 +869,9 @@ export function diffHydratedProperties(
domElement: Element,
tag: string,
rawProps: Object,
parentNamespace: string,
isConcurrentMode: boolean,
shouldWarnDev: boolean,
namespaceDEV?: string,
): null | Array<mixed> {
let isCustomComponentTag;
let extraAttributeNames: Set<string>;
Expand Down Expand Up @@ -1109,11 +1114,7 @@ export function diffHydratedProperties(
propertyInfo,
);
} else {
let ownNamespace = parentNamespace;
if (ownNamespace === HTML_NAMESPACE) {
ownNamespace = getIntrinsicNamespace(tag);
}
if (ownNamespace === HTML_NAMESPACE) {
if (namespaceDEV === HTML_NAMESPACE) {
// $FlowFixMe - Should be inferred as not undefined.
extraAttributeNames.delete(propKey.toLowerCase());
} else {
Expand Down
Loading

0 comments on commit c74769d

Please sign in to comment.