Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fiber][Dev] Relax dom nesting validation when the root is a Document, html tag, or body tag #32252

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Jan 28, 2025

followup to

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).

@gnoff gnoff requested a review from sebmarkbage January 28, 2025 19:29
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Jan 28, 2025
@gnoff gnoff changed the title Relax dom nesting at root [Fiber][Dev] Relax dom nesting validation when the root is a Document or html tag Jan 28, 2025
@gnoff gnoff changed the title [Fiber][Dev] Relax dom nesting validation when the root is a Document or html tag [Fiber][Dev] Relax dom nesting validation when the root is a Document, html tag, or body tag Jan 28, 2025
@react-sizebot
Copy link

react-sizebot commented Jan 28, 2025

Comparing: b48e739...365cde6

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 515.48 kB 515.48 kB = 92.04 kB 92.04 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 558.52 kB 558.52 kB = 99.27 kB 99.27 kB
facebook-www/ReactDOM-prod.classic.js = 636.88 kB 636.88 kB = 111.92 kB 111.92 kB
facebook-www/ReactDOM-prod.modern.js = 627.20 kB 627.20 kB = 110.34 kB 110.34 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 0a749ef

@gnoff gnoff force-pushed the relax-dom-nesting-at-root branch from aee09bb to 163de10 Compare January 28, 2025 20:29
@gnoff gnoff force-pushed the relax-dom-nesting-at-root branch from 163de10 to 8d78e37 Compare January 29, 2025 06:12
@gnoff gnoff force-pushed the relax-dom-nesting-at-root branch from 8d78e37 to aa7b5bd Compare January 29, 2025 06:24
@gnoff gnoff force-pushed the relax-dom-nesting-at-root branch from aa7b5bd to c30cf4c Compare January 29, 2025 06:35
@gnoff gnoff force-pushed the relax-dom-nesting-at-root branch 4 times, most recently from c15858c to 2d56804 Compare February 1, 2025 18:33
@gnoff gnoff force-pushed the relax-dom-nesting-at-root branch from 2d56804 to 8ebfb85 Compare February 1, 2025 18:41
@gnoff gnoff force-pushed the relax-dom-nesting-at-root branch from 8ebfb85 to ecf43ac Compare February 1, 2025 18:47
@gnoff gnoff requested a review from acdlite February 3, 2025 15:55
@gnoff gnoff force-pushed the relax-dom-nesting-at-root branch from ecf43ac to 49e0fae Compare February 4, 2025 20:40
@gnoff gnoff requested review from eps1lon and removed request for acdlite February 6, 2025 22:31
Copy link
Collaborator

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great test coverage.

@@ -138,6 +164,49 @@ describe('validateDOMNesting', () => {
'In HTML, <body> cannot be a child of <body>.\n' +
'This will cause a hydration error.\n' +
' in body (at **)',
'You are mounting a new body component when a previous one has not first unmounted.',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance we can skip this when we already warned about body > body?

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).
@gnoff gnoff force-pushed the relax-dom-nesting-at-root branch from 49e0fae to 0a749ef Compare February 6, 2025 22:59
@gnoff gnoff merged commit a0fdb63 into facebook:main Feb 6, 2025
191 checks passed
@gnoff gnoff deleted the relax-dom-nesting-at-root branch February 6, 2025 23:05
github-actions bot pushed a commit that referenced this pull request Feb 6, 2025
…, html tag, or body tag (#32252)

followup to
* #32069
* #32163
* #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).

DiffTrain build for [a0fdb63](a0fdb63)
github-actions bot pushed a commit that referenced this pull request Feb 6, 2025
…, html tag, or body tag (#32252)

followup to
* #32069
* #32163
* #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).

DiffTrain build for [a0fdb63](a0fdb63)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Feb 7, 2025
…, 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).

DiffTrain build for [a0fdb63](facebook@a0fdb63)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Feb 7, 2025
…, 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).

DiffTrain build for [a0fdb63](facebook@a0fdb63)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants