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

Bug(@next): legacy ReactDOM.render crashes when rendering into document container #26128

Closed
eps1lon opened this issue Feb 8, 2023 · 3 comments

Comments

@eps1lon
Copy link
Collaborator

eps1lon commented Feb 8, 2023

React version: 18.3.0-next-4bf2113a1-20230206

Steps To Reproduce

  1. ReactDOM.render into a document container

Link to code example: https://codesandbox.io/s/react-next-legacy-render-crashes-when-rendering-html-jqdut3?file=/src/index.js

The current behavior

Throws with

React expected an <html> element (document.documentElement) to exist in the Document but one was not found. React never removes the documentElement for any Document it renders into so the cause is likely in some other script running on this page.

ReactDOM.render clears the container before rendering into it. But with the new HostSingletons (#25426) we expect an existing documentElement.

The odd part is that it seems like #25426 affected the @next release even though enableHostSingletons is disabled for that release.

/cc @gnoff

The expected behavior

No crash like in [email protected]: https://codesandbox.io/s/react-next-legacy-render-crashes-when-rendering-html-forked-977biy

@eps1lon eps1lon changed the title Bug: legacy ReactDOM.render crashes when rendering into document container Bug(@next): legacy ReactDOM.render crashes when rendering into document container Feb 8, 2023
@gnoff
Copy link
Collaborator

gnoff commented Feb 8, 2023

The odd part is that it seems like #25426 affected the @next release even though enableHostSingletons is disabled for that release.

I do believe the flag is on for the oss-stable build so it makes sense the code is showing up. I'll add a test and fix once I repro

@eps1lon
Copy link
Collaborator Author

eps1lon commented Feb 8, 2023

I do believe the flag is on for the oss-stable build so it makes sense the code is showing up.

Oh right. It was initially shipped with __EXPERIMENTAL__ but now it's just true.

gnoff added a commit that referenced this issue Feb 8, 2023
as reported in #26128 `ReactDOM.render(..., document)` crashed when
`enableHostSingletons` was on. This is because it had a different way of
clearing the container than `createRoot(document)`. I updated the legacy
implementation to share the clearing behavior of `creatRoot` which will
preserve the singleton instances.

I also removed the warning saying not to use `document.body` as a
container
github-actions bot pushed a commit that referenced this issue Feb 8, 2023
as reported in #26128 `ReactDOM.render(..., document)` crashed when
`enableHostSingletons` was on. This is because it had a different way of
clearing the container than `createRoot(document)`. I updated the legacy
implementation to share the clearing behavior of `creatRoot` which will
preserve the singleton instances.

I also removed the warning saying not to use `document.body` as a
container

DiffTrain build for [a3152ed](a3152ed)
[View git log for this commit](https://github.com/facebook/react/commits/a3152eda5f89e20f056521855f7fa101ce50e4c3)
@eps1lon
Copy link
Collaborator Author

eps1lon commented Feb 9, 2023

Fixed in #26129

@eps1lon eps1lon closed this as completed Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants