Skip to content

Commit a0fdb63

Browse files
gnoffgnoff
authored
[Fiber][Dev] Relax dom nesting validation when the root is a Document, html tag, or body tag (facebook#32252)
followup to * facebook#32069 * facebook#32163 * facebook#32224 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).
1 parent b48e739 commit a0fdb63

File tree

7 files changed

+175
-49
lines changed

7 files changed

+175
-49
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ function setProp(
344344
case 'children': {
345345
if (typeof value === 'string') {
346346
if (__DEV__) {
347-
validateTextNesting(value, tag);
347+
validateTextNesting(value, tag, false);
348348
}
349349
// Avoid setting initial textContent when the text is empty. In IE11 setting
350350
// textContent on a <textarea> will cause the placeholder to not
@@ -358,7 +358,7 @@ function setProp(
358358
} else if (typeof value === 'number' || typeof value === 'bigint') {
359359
if (__DEV__) {
360360
// $FlowFixMe[unsafe-addition] Flow doesn't want us to use `+` operator with string and bigint
361-
validateTextNesting('' + value, tag);
361+
validateTextNesting('' + value, tag, false);
362362
}
363363
const canSetTextContent = tag !== 'body';
364364
if (canSetTextContent) {

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,11 @@ export function createTextInstance(
595595
const hostContextDev = ((hostContext: any): HostContextDev);
596596
const ancestor = hostContextDev.ancestorInfo.current;
597597
if (ancestor != null) {
598-
validateTextNesting(text, ancestor.tag);
598+
validateTextNesting(
599+
text,
600+
ancestor.tag,
601+
hostContextDev.ancestorInfo.implicitRootScope,
602+
);
599603
}
600604
}
601605
const textNode: TextInstance = getOwnerDocumentFromRootContainer(
@@ -2046,7 +2050,11 @@ export function validateHydratableTextInstance(
20462050
const hostContextDev = ((hostContext: any): HostContextDev);
20472051
const ancestor = hostContextDev.ancestorInfo.current;
20482052
if (ancestor != null) {
2049-
return validateTextNesting(text, ancestor.tag);
2053+
return validateTextNesting(
2054+
text,
2055+
ancestor.tag,
2056+
hostContextDev.ancestorInfo.implicitRootScope,
2057+
);
20502058
}
20512059
}
20522060
return true;
@@ -2394,8 +2402,15 @@ export function acquireSingletonInstance(
23942402
internalInstanceHandle: Object,
23952403
): void {
23962404
if (__DEV__) {
2397-
const currentInstanceHandle = getInstanceFromNodeDOMTree(instance);
2398-
if (currentInstanceHandle) {
2405+
if (
2406+
// If this instance is the container then it is invalid to acquire it as a singleton however
2407+
// the DOM nesting validation will already warn for this and the message below isn't semantically
2408+
// aligned with the actual fix you need to make so we omit the warning in this case
2409+
!isContainerMarkedAsRoot(instance) &&
2410+
// If this instance isn't the root but is currently owned by a different HostSingleton instance then
2411+
// we we need to warn that you are rendering more than one singleton at a time.
2412+
getInstanceFromNodeDOMTree(instance)
2413+
) {
23992414
const tagName = instance.tagName.toLowerCase();
24002415
console.error(
24012416
'You are mounting a new %s component when a previous one has not first unmounted. It is an' +

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

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ export type AncestorInfoDev = {
7171

7272
// <head> or <body>
7373
containerTagInScope: ?Info,
74+
implicitRootScope: boolean,
7475
};
7576

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

221222
containerTagInScope: null,
223+
implicitRootScope: false,
222224
};
223225

224226
function updatedAncestorInfoDev(
225-
oldInfo: ?AncestorInfoDev,
227+
oldInfo: null | AncestorInfoDev,
226228
tag: string,
227229
): AncestorInfoDev {
228230
if (__DEV__) {
@@ -238,14 +240,14 @@ function updatedAncestorInfoDev(
238240
ancestorInfo.pTagInButtonScope = null;
239241
}
240242

241-
// See rules for 'li', 'dd', 'dt' start tags in
242-
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inbody
243243
if (
244244
specialTags.indexOf(tag) !== -1 &&
245245
tag !== 'address' &&
246246
tag !== 'div' &&
247247
tag !== 'p'
248248
) {
249+
// See rules for 'li', 'dd', 'dt' start tags in
250+
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inbody
249251
ancestorInfo.listItemTagAutoclosing = null;
250252
ancestorInfo.dlItemTagAutoclosing = null;
251253
}
@@ -279,6 +281,17 @@ function updatedAncestorInfoDev(
279281
ancestorInfo.containerTagInScope = info;
280282
}
281283

284+
if (
285+
oldInfo === null &&
286+
(tag === '#document' || tag === 'html' || tag === 'body')
287+
) {
288+
// While <head> is also a singleton we don't want to support semantics where
289+
// you can escape the head by rendering a body singleton so we treat it like a normal scope
290+
ancestorInfo.implicitRootScope = true;
291+
} else if (ancestorInfo.implicitRootScope === true) {
292+
ancestorInfo.implicitRootScope = false;
293+
}
294+
282295
return ancestorInfo;
283296
} else {
284297
return (null: any);
@@ -288,7 +301,11 @@ function updatedAncestorInfoDev(
288301
/**
289302
* Returns whether
290303
*/
291-
function isTagValidWithParent(tag: string, parentTag: ?string): boolean {
304+
function isTagValidWithParent(
305+
tag: string,
306+
parentTag: ?string,
307+
implicitRootScope: boolean,
308+
): boolean {
292309
// First, let's check if we're in an unusual parsing mode...
293310
switch (parentTag) {
294311
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inselect
@@ -363,10 +380,22 @@ function isTagValidWithParent(tag: string, parentTag: ?string): boolean {
363380
);
364381
// https://html.spec.whatwg.org/multipage/semantics.html#the-html-element
365382
case 'html':
383+
if (implicitRootScope) {
384+
// When our parent tag is html and we're in the root scope we will actually
385+
// insert most tags into the body so we need to fall through to validating
386+
// the specific tag with "in body" parsing mode below
387+
break;
388+
}
366389
return tag === 'head' || tag === 'body' || tag === 'frameset';
367390
case 'frameset':
368391
return tag === 'frame';
369392
case '#document':
393+
if (implicitRootScope) {
394+
// When our parent is the Document and we're in the root scope we will actually
395+
// insert most tags into the body so we need to fall through to validating
396+
// the specific tag with "in body" parsing mode below
397+
break;
398+
}
370399
return tag === 'html';
371400
}
372401

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

396-
case 'body':
397425
case 'caption':
398426
case 'col':
399427
case 'colgroup':
400428
case 'frameset':
401429
case 'frame':
402-
case 'head':
403-
case 'html':
404430
case 'tbody':
405431
case 'td':
406432
case 'tfoot':
@@ -412,6 +438,24 @@ function isTagValidWithParent(tag: string, parentTag: ?string): boolean {
412438
// so we allow it only if we don't know what the parent is, as all other
413439
// cases are invalid.
414440
return parentTag == null;
441+
case 'head':
442+
// We support rendering <head> in the root when the container is
443+
// #document, <html>, or <body>.
444+
return implicitRootScope || parentTag === null;
445+
case 'html':
446+
// We support rendering <html> in the root when the container is
447+
// #document
448+
return (
449+
(implicitRootScope && parentTag === '#document') || parentTag === null
450+
);
451+
case 'body':
452+
// We support rendering <body> in the root when the container is
453+
// #document or <html>
454+
return (
455+
(implicitRootScope &&
456+
(parentTag === '#document' || parentTag === 'html')) ||
457+
parentTag === null
458+
);
415459
}
416460

417461
return true;
@@ -513,7 +557,11 @@ function validateDOMNesting(
513557
const parentInfo = ancestorInfo.current;
514558
const parentTag = parentInfo && parentInfo.tag;
515559

516-
const invalidParent = isTagValidWithParent(childTag, parentTag)
560+
const invalidParent = isTagValidWithParent(
561+
childTag,
562+
parentTag,
563+
ancestorInfo.implicitRootScope,
564+
)
517565
? null
518566
: parentInfo;
519567
const invalidAncestor = invalidParent
@@ -594,9 +642,13 @@ function validateDOMNesting(
594642
return true;
595643
}
596644

597-
function validateTextNesting(childText: string, parentTag: string): boolean {
645+
function validateTextNesting(
646+
childText: string,
647+
parentTag: string,
648+
implicitRootScope: boolean,
649+
): boolean {
598650
if (__DEV__) {
599-
if (isTagValidWithParent('#text', parentTag)) {
651+
if (implicitRootScope || isTagValidWithParent('#text', parentTag, false)) {
600652
return true;
601653
}
602654

‎packages/react-dom/src/__tests__/ReactDOM-test.js

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -601,10 +601,6 @@ describe('ReactDOM', () => {
601601
'<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>',
602602
);
603603

604-
// @TODO remove this warning check when we loosen the tag nesting restrictions to allow arbitrary tags at the
605-
// root of the application
606-
assertConsoleErrorDev(['In HTML, <div> cannot be a child of <#document>']);
607-
608604
await act(() => {
609605
root.render(<App phase={1} />);
610606
});
@@ -666,10 +662,6 @@ describe('ReactDOM', () => {
666662
'<html><head data-h=""><meta itemprop="" content="head"></head><body data-b=""><div>before</div><div>inside</div><div>after</div></body></html>',
667663
);
668664

669-
// @TODO remove this warning check when we loosen the tag nesting restrictions to allow arbitrary tags at the
670-
// root of the application
671-
assertConsoleErrorDev(['In HTML, <div> cannot be a child of <html>']);
672-
673665
await act(() => {
674666
root.render(<App phase={1} />);
675667
});
@@ -729,10 +721,6 @@ describe('ReactDOM', () => {
729721
'<html><head data-h=""><meta itemprop="" content="head"></head><body><div>before</div><div>inside</div><div>after</div></body></html>',
730722
);
731723

732-
// @TODO remove this warning check when we loosen the tag nesting restrictions to allow arbitrary tags at the
733-
// root of the application
734-
assertConsoleErrorDev(['In HTML, <head> cannot be a child of <body>']);
735-
736724
await act(() => {
737725
root.render(<App phase={1} />);
738726
});

‎packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9004,7 +9004,6 @@ describe('ReactDOMFizzServer', () => {
90049004
</body>
90059005
</html>,
90069006
);
9007-
assertConsoleErrorDev(['In HTML, <div> cannot be a child of <#document>']);
90089007

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

1017810176
root.unmount();
1017910177
expect(getVisibleChildren(document)).toEqual(

‎packages/react-dom/src/__tests__/ReactDOMFloat-test.js

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -511,9 +511,6 @@ describe('ReactDOMFloat', () => {
511511
'Cannot render <noscript> outside the main document. Try moving it into the root <head> tag.',
512512
{withoutStack: true},
513513
],
514-
'In HTML, <noscript> cannot be a child of <#document>.\n' +
515-
'This will cause a hydration error.\n' +
516-
' in noscript (at **)',
517514
]);
518515

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

585579
root.render(

0 commit comments

Comments
 (0)