-
Notifications
You must be signed in to change notification settings - Fork 47.1k
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
support ReactDOM.render(..., document) without crashing #26129
Conversation
Comparing: 758fc7f...3de6076 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow. People still use this api with the warning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still seeing this diff:
()"` |
-| `version=(numeric string)`| (changed, ssr mismatch)| `"42"` |
-| `version=(-1)`| (changed, ssr mismatch)| `"-1"` |
-| `version=(0)`| (changed, ssr mismatch)| `"0"` |-| `version=(integer)`| (changed, ssr mismatch)| `"1"` |
-| `version=(NaN)`| (changed, warning, ssr mismatch)| `"NaN"` | -| `version=(float)`| (changed, ssr mismatch)| `"99.99"` |
-| `version=(true)`| (initial, warning)| `<empty string>` |-| `version=(false)`| (initial, warning)| `<empty string>` |
-| `version=(string 'true')`| (changed, ssr mismatch)| `"true"` | -| `version=(string 'false')`| (changed, ssr mismatch)| `"false"` |
-| `version=(string 'on')`| (changed, ssr mismatch)| `"on"` |
-| `version=(string 'off')`| (changed, ssr mismatch)| `"off"` |
-| `version=(symbol)`| (initial, warning)| `<empty string>` |
-| `version=(function)`| (initial, warning)| `<empty string>` |
-| `version=(null)`| (initial)| `<empty string>` |
-| `version=(undefined)`| (initial)| `<empty string>` |
+| `version=(string)`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(empty string)`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(array with string)`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(empty array)`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(object)`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(numeric string)`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(-1)`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(0)`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(integer)`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(NaN)`| (initial, warning, ssr mismatch)| `<undefined>` |
+| `version=(float)`| (initial, ssr mismatch)| `<undefined>` | +| `version=(true)`| (initial, warning, ssr mismatch)| `<undefined>` |
+| `version=(false)`| (initial, warning, ssr mismatch)| `<undefined>` |
+| `version=(string 'true')`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(string 'false')`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(string 'on')`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(string 'off')`| (initial, ssr mismatch)| `<undefined>` |+| `version=(symbol)`| (initial, warning, ssr mismatch)| `<undefined>` |
+| `version=(function)`| (initial, warning, ssr mismatch)| `<undefined>` |
+| `version=(null)`| (initial, ssr mismatch)| `<undefined>` |
+| `version=(undefined)`| (initial, ssr mismatch)| `<undefined>` |
## `version` (on `<svg>` inside `<div>`)
| Test Case | Flags | Result |
which makes it seem like no value is rendered. Will just replace this test with one using lang
since version
is obsolete anyway.
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)
as reported in #26128
ReactDOM.render(..., document)
crashed whenenableHostSingletons
was on. This is because it had a different way of clearing the container thancreateRoot(document)
. I updated the legacy implementation to share the clearing behavior ofcreatRoot
which will preserve the singleton instances.I also removed the warning saying not to use
document.body
as a container