Skip to content

Commit

Permalink
[Fiber] relax DOM validation rules at root
Browse files Browse the repository at this point in the history
in react-dom in Dev we validate that the tag nesting is valid. This is motivated primarily because while browsers are tolerant to poor HTML there are many cases that if server rendered will be hydrated in a way that will break hydration.

With the changes to singleton scoping where the document body is now the implicit render/hydration context for arbitrary tags at the root we need to adjust the validation logic to allow for valid programs such as rendering divs as a child of a Document (since this div will actually insert into the body).
  • Loading branch information
gnoff committed Jan 28, 2025
1 parent c40b0ae commit 163de10
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 44 deletions.
4 changes: 2 additions & 2 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ function setProp(
case 'children': {
if (typeof value === 'string') {
if (__DEV__) {
validateTextNesting(value, tag);
validateTextNesting(value, tag, false);
}
// Avoid setting initial textContent when the text is empty. In IE11 setting
// textContent on a <textarea> will cause the placeholder to not
Expand All @@ -358,7 +358,7 @@ function setProp(
} else if (typeof value === 'number' || typeof value === 'bigint') {
if (__DEV__) {
// $FlowFixMe[unsafe-addition] Flow doesn't want us to use `+` operator with string and bigint
validateTextNesting('' + value, tag);
validateTextNesting('' + value, tag, false);
}
const canSetTextContent = tag !== 'body';
if (canSetTextContent) {
Expand Down
12 changes: 10 additions & 2 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,11 @@ export function createTextInstance(
const hostContextDev = ((hostContext: any): HostContextDev);
const ancestor = hostContextDev.ancestorInfo.current;
if (ancestor != null) {
validateTextNesting(text, ancestor.tag);
validateTextNesting(
text,
ancestor.tag,
hostContextDev.ancestorInfo.implicitRootScope,
);
}
}
const textNode: TextInstance = getOwnerDocumentFromRootContainer(
Expand Down Expand Up @@ -2027,7 +2031,11 @@ export function validateHydratableTextInstance(
const hostContextDev = ((hostContext: any): HostContextDev);
const ancestor = hostContextDev.ancestorInfo.current;
if (ancestor != null) {
return validateTextNesting(text, ancestor.tag);
return validateTextNesting(
text,
ancestor.tag,
hostContextDev.ancestorInfo.implicitRootScope,
);
}
}
return true;
Expand Down
63 changes: 53 additions & 10 deletions packages/react-dom-bindings/src/client/validateDOMNesting.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export type AncestorInfoDev = {

// <head> or <body>
containerTagInScope: ?Info,
implicitRootScope: boolean,
};

// This validation code was written based on the HTML5 parsing spec:
Expand Down Expand Up @@ -219,10 +220,11 @@ const emptyAncestorInfoDev: AncestorInfoDev = {
dlItemTagAutoclosing: null,

containerTagInScope: null,
implicitRootScope: false,
};

function updatedAncestorInfoDev(
oldInfo: ?AncestorInfoDev,
oldInfo: null | AncestorInfoDev,
tag: string,
): AncestorInfoDev {
if (__DEV__) {
Expand All @@ -238,14 +240,14 @@ function updatedAncestorInfoDev(
ancestorInfo.pTagInButtonScope = null;
}

// See rules for 'li', 'dd', 'dt' start tags in
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inbody
if (
specialTags.indexOf(tag) !== -1 &&
tag !== 'address' &&
tag !== 'div' &&
tag !== 'p'
) {
// See rules for 'li', 'dd', 'dt' start tags in
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inbody
ancestorInfo.listItemTagAutoclosing = null;
ancestorInfo.dlItemTagAutoclosing = null;
}
Expand Down Expand Up @@ -279,6 +281,17 @@ function updatedAncestorInfoDev(
ancestorInfo.containerTagInScope = info;
}

if (
oldInfo === null &&
(tag === '#document' || tag === 'html' || tag === 'body')
) {
// While <head> is also a singleton we don't want to support semantics where
// you can escape the head by rendering a body singleton so we treat it like a normal scope
ancestorInfo.implicitRootScope = true;
} else if (ancestorInfo.implicitRootScope === true) {
ancestorInfo.implicitRootScope = false;
}

return ancestorInfo;
} else {
return (null: any);
Expand All @@ -288,7 +301,11 @@ function updatedAncestorInfoDev(
/**
* Returns whether
*/
function isTagValidWithParent(tag: string, parentTag: ?string): boolean {
function isTagValidWithParent(
tag: string,
parentTag: ?string,
implicitRootScope: boolean,
): boolean {
// First, let's check if we're in an unusual parsing mode...
switch (parentTag) {
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inselect
Expand Down Expand Up @@ -363,10 +380,16 @@ function isTagValidWithParent(tag: string, parentTag: ?string): boolean {
);
// https://html.spec.whatwg.org/multipage/semantics.html#the-html-element
case 'html':
if (implicitRootScope) {
break;
}
return tag === 'head' || tag === 'body' || tag === 'frameset';
case 'frameset':
return tag === 'frame';
case '#document':
if (implicitRootScope) {
break;
}
return tag === 'html';
}

Expand All @@ -393,14 +416,11 @@ function isTagValidWithParent(tag: string, parentTag: ?string): boolean {
case 'rt':
return impliedEndTags.indexOf(parentTag) === -1;

case 'body':
case 'caption':
case 'col':
case 'colgroup':
case 'frameset':
case 'frame':
case 'head':
case 'html':
case 'tbody':
case 'td':
case 'tfoot':
Expand All @@ -412,6 +432,21 @@ function isTagValidWithParent(tag: string, parentTag: ?string): boolean {
// so we allow it only if we don't know what the parent is, as all other
// cases are invalid.
return parentTag == null;
case 'head':
return (implicitRootScope && parentTag !== 'head') || parentTag === null;
case 'html':
return (
(implicitRootScope &&
parentTag !== 'body' &&
parentTag !== 'html' &&
parentTag !== 'head') ||
parentTag === null
);
case 'body':
return (
(implicitRootScope && parentTag !== 'body' && parentTag !== 'head') ||
parentTag === null
);
}

return true;
Expand Down Expand Up @@ -513,7 +548,11 @@ function validateDOMNesting(
const parentInfo = ancestorInfo.current;
const parentTag = parentInfo && parentInfo.tag;

const invalidParent = isTagValidWithParent(childTag, parentTag)
const invalidParent = isTagValidWithParent(
childTag,
parentTag,
ancestorInfo.implicitRootScope,
)
? null
: parentInfo;
const invalidAncestor = invalidParent
Expand Down Expand Up @@ -594,9 +633,13 @@ function validateDOMNesting(
return true;
}

function validateTextNesting(childText: string, parentTag: string): boolean {
function validateTextNesting(
childText: string,
parentTag: string,
implicitRootScope: boolean,
): boolean {
if (__DEV__) {
if (isTagValidWithParent('#text', parentTag)) {
if (implicitRootScope || isTagValidWithParent('#text', parentTag)) {
return true;
}

Expand Down
12 changes: 0 additions & 12 deletions packages/react-dom/src/__tests__/ReactDOM-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -601,10 +601,6 @@ describe('ReactDOM', () => {
'<html lang="en"><head data-h=""><meta itemprop="" content="head"></head><body data-b=""><div>before</div><div>inside</div><div>after</div></body></html>',
);

// @TODO remove this warning check when we loosen the tag nesting restrictions to allow arbitrary tags at the
// root of the application
assertConsoleErrorDev(['In HTML, <div> cannot be a child of <#document>']);

await act(() => {
root.render(<App phase={1} />);
});
Expand Down Expand Up @@ -666,10 +662,6 @@ describe('ReactDOM', () => {
'<html><head data-h=""><meta itemprop="" content="head"></head><body data-b=""><div>before</div><div>inside</div><div>after</div></body></html>',
);

// @TODO remove this warning check when we loosen the tag nesting restrictions to allow arbitrary tags at the
// root of the application
assertConsoleErrorDev(['In HTML, <div> cannot be a child of <html>']);

await act(() => {
root.render(<App phase={1} />);
});
Expand Down Expand Up @@ -729,10 +721,6 @@ describe('ReactDOM', () => {
'<html><head data-h=""><meta itemprop="" content="head"></head><body><div>before</div><div>inside</div><div>after</div></body></html>',
);

// @TODO remove this warning check when we loosen the tag nesting restrictions to allow arbitrary tags at the
// root of the application
assertConsoleErrorDev(['In HTML, <head> cannot be a child of <body>']);

await act(() => {
root.render(<App phase={1} />);
});
Expand Down
2 changes: 0 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9004,7 +9004,6 @@ describe('ReactDOMFizzServer', () => {
</body>
</html>,
);
assertConsoleErrorDev(['In HTML, <div> cannot be a child of <#document>']);

root.unmount();
expect(getVisibleChildren(document)).toEqual(
Expand Down Expand Up @@ -10173,7 +10172,6 @@ describe('ReactDOMFizzServer', () => {
</body>
</html>,
);
assertConsoleErrorDev(['In HTML, <div> cannot be a child of <#document>']);

root.unmount();
expect(getVisibleChildren(document)).toEqual(
Expand Down
6 changes: 0 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMFloat-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -511,9 +511,6 @@ describe('ReactDOMFloat', () => {
'Cannot render <noscript> outside the main document. Try moving it into the root <head> tag.',
{withoutStack: true},
],
'In HTML, <noscript> cannot be a child of <#document>.\n' +
'This will cause a hydration error.\n' +
' in noscript (at **)',
]);

root.render(
Expand Down Expand Up @@ -577,9 +574,6 @@ describe('ReactDOMFloat', () => {
'Consider adding precedence="default" or moving it into the root <head> tag.',
{withoutStack: true},
],
'In HTML, <link> cannot be a child of <#document>.\n' +
'This will cause a hydration error.\n' +
' in link (at **)',
]);

root.render(
Expand Down
Loading

0 comments on commit 163de10

Please sign in to comment.