Skip to content

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented May 9, 2022

Hydrating into Document

This PR addressed the most fundamental issue identified in #22833. In that issue a hydration mismatch when hydrating into the Document caused a broken application because the client rendered fallback could not append any content to the Document.

This is caused by the fact that a Document can only have 1 element at a time, namely the documentElement which is usually <html> and by not removing it beforehand the append of a new <html> element fails.

Initially I looked into recycling the documentElement so it is never removed or appended but just moved around to whatever fiber needs it, staying in the Document tree as its root. This worked fine but added some code on hot paths and a bit of size.

I then looked at what would happen if we simply removed the documentElement (i.e. treat it like any other Element) and in Chrome, Safari, and Firefox (modern versions) there seems to be no issue with this. This PR is much smaller and simply updates clearContainer to properly clear the Document as a container so that a later appendChild of an <html> element will not violation Document invariants

Hydration into Document compatibility with 3rd party scripts and Extensions

The original issue shows an extension breaking a React application. This PR does not introduce compatibility with extensions or 3rd party scripts that modify the DOM before React hydrates, it simply inverts what gets broken. This change would result in (worst case) React breaking the extension rather than the extension breaking React.

createRoot for Document

as a consequence of the fix for hydration in Document using Document as a root should work now. This PR also adds back types for using createRoot on Document. It should be noted that doing so will effectively wipe out the entire document so third party styles and other DOM elements created by server response, 3rd party scripts, or extensions will be dropped. This is probably not that useful a feature but since it is supported by implementation with no special casing I added back in

gnoff added 2 commits May 9, 2022 09:02
Previously Document was not handled effectively as a container. in particual when hydrating if there was a fallback to client rendering React would attempt to append a new <html> element into the document before clearing out the existing one which errored leaving the application in brokent state.

The initial approach I took was to recycle the documentElement and never remove or append it, always just moving it to the right fiber and appending the right children (heady/body) as needed. However in testing a simple approach in modern browsers it seems like treating the documentElement like any other element works fine. This change modifies the clearContainer method to remove the documentElement if the container is a DOCUMENT_NODE. Once the container is cleared React can append a new documentElement via normal means.
previously rendering into Document was broken and only hydration worked because React did not properly deal with the documentElement and would error in a broken state if used that way. With the previous commit addressing this limitation this change re-adds Document as a valid container for createRoot.

It should be noted that if you use document with createRoot it will drop anything a 3rd party scripts adds the page before rendering for the first time.
@sizebot
Copy link

sizebot commented May 9, 2022

Comparing: d20c3af...0215c83

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.min.js = 131.58 kB 131.58 kB = 42.15 kB 42.15 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 136.82 kB 136.82 kB = 43.70 kB 43.68 kB
facebook-www/ReactDOM-prod.classic.js = 440.68 kB 440.68 kB = 80.35 kB 80.33 kB
facebook-www/ReactDOM-prod.modern.js = 425.89 kB 425.89 kB = 78.17 kB 78.15 kB
facebook-www/ReactDOMForked-prod.classic.js = 440.68 kB 440.68 kB = 80.35 kB 80.33 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 0215c83

@sebmarkbage sebmarkbage requested a review from eps1lon May 9, 2022 16:31
@eps1lon
Copy link
Collaborator

eps1lon commented May 10, 2022

This PR addressed the most fundamental issue identified in #22833.

Does this PR actually fix #22833?

@gnoff
Copy link
Collaborator Author

gnoff commented May 10, 2022

This PR addressed the most fundamental issue identified in #22833.

Does this PR actually fix #22833?

Yes, https://codesandbox.io/s/react-18-hydration-mismatch-in-document-fixed-24523-9n6zos?file=/package.json

There are hydration errors but the issue of unmounting the entire app is fixed. The hydration errors are expected if I understand the test case correctly

@CanRau
Copy link

CanRau commented May 12, 2022

Any eta on when this will be shipped?

@gaearon
Copy link
Collaborator

gaearon commented May 12, 2022

Hopefully within the next two weeks.

@ZipBrandon
Copy link

Looking to check on an updated timeline estimate for this and 18.2? This PR resolves issues that I experience with SSR, but other libraries which mark React as a peer dependency are upset when I am having to npm i --force the next for react and react-dom. The sub-packages end up installing their own React dependencies which leads to the Rules of Hooks being broken (like https://github.com/plouc/nivo/blob/master/packages/bar/package.json) which I have to subsequently remove from its node_modules.

@gaearon
Copy link
Collaborator

gaearon commented Jun 3, 2022

You can use Yarn resolutions or npm overrides to force "deep" packages to use the same version, as far as I know.

CrispyBaguette pushed a commit to CrispyBaguette/wasm-palette-converter that referenced this pull request Nov 8, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [react](https://reactjs.org/) ([source](https://github.com/facebook/react/tree/HEAD/packages/react)) | dependencies | minor | [`18.1.0` -> `18.3.1`](https://renovatebot.com/diffs/npm/react/18.1.0/18.3.1) |
| [@types/react](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/react) ([source](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/react)) | dependencies | minor | [`18.0.9` -> `18.3.12`](https://renovatebot.com/diffs/npm/@types%2freact/18.0.9/18.3.12) |

---

### Release Notes

<details>
<summary>facebook/react (react)</summary>

### [`v18.3.1`](https://github.com/facebook/react/blob/HEAD/CHANGELOG.md#1831-April-26-2024)

[Compare Source](facebook/react@v18.3.0...v18.3.1)

-   Export `act` from `react` [f1338f](facebook/react@f1338f8)

### [`v18.3.0`](https://github.com/facebook/react/blob/HEAD/CHANGELOG.md#1830-April-25-2024)

[Compare Source](facebook/react@v18.2.0...v18.3.0)

This release is identical to 18.2 but adds warnings for deprecated APIs and other changes that are needed for React 19.

Read the [React 19 Upgrade Guide](https://react.dev/blog/2024/04/25/react-19-upgrade-guide) for more info.

##### React

-   Allow writing to `this.refs` to support string ref codemod [909071](facebook/react@9090712)
-   Warn for deprecated `findDOMNode` outside StrictMode [c3b283](facebook/react@c3b2839)
-   Warn for deprecated `test-utils` methods [d4ea75](facebook/react@d4ea75d)
-   Warn for deprecated Legacy Context outside StrictMode [415ee0](facebook/react@415ee0e)
-   Warn for deprecated string refs outside StrictMode [#&#8203;25383](facebook/react#25383)
-   Warn for deprecated `defaultProps` for function components [#&#8203;25699](facebook/react#25699)
-   Warn when spreading `key` [#&#8203;25697](facebook/react#25697)
-   Warn when using `act` from `test-utils` [d4ea75](facebook/react@d4ea75d)

##### React DOM

-   Warn for deprecated `unmountComponentAtNode` [8a015b](facebook/react@8a015b6)
-   Warn for deprecated `renderToStaticNodeStream` [#&#8203;28874](facebook/react#28874)

### [`v18.2.0`](https://github.com/facebook/react/blob/HEAD/CHANGELOG.md#1820-June-14-2022)

[Compare Source](facebook/react@v18.1.0...v18.2.0)

##### React DOM

-   Provide a component stack as a second argument to `onRecoverableError`. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24591](facebook/react#24591))
-   Fix hydrating into `document` causing a blank page on mismatch. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24523](facebook/react#24523))
-   Fix false positive hydration errors with Suspense. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24480](facebook/react#24480) and  [@&#8203;acdlite](https://github.com/acdlite) in [#&#8203;24532](facebook/react#24532))
-   Fix ignored `setState` in Safari when adding an iframe. ([@&#8203;gaearon](https://github.com/gaearon) in [#&#8203;24459](facebook/react#24459))

##### React DOM Server

-   Pass information about server errors to the client. ([@&#8203;salazarm](https://github.com/salazarm) and [@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24551](facebook/react#24551) and [#&#8203;24591](facebook/react#24591))
-   Allow to provide a reason when aborting the HTML stream. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24680](facebook/react#24680))
-   Eliminate extraneous text separators in the HTML where possible. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24630](facebook/react#24630))
-   Disallow complex children inside `<title>` elements to match the browser constraints. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24679](facebook/react#24679))
-   Fix buffering in some worker environments by explicitly setting `highWaterMark` to `0`. ([@&#8203;jplhomer](https://github.com/jplhomer) in [#&#8203;24641](facebook/react#24641))

##### Server Components (Experimental)

-   Add support for `useId()` inside Server Components. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24172](facebook/react#24172))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4xNDIuNSIsInVwZGF0ZWRJblZlciI6IjM4LjE0Mi41IiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=-->

Reviewed-on: https://gitea.bruyant.xyz/alexandre/PaletteSwitcher/pulls/49
Co-authored-by: Renovate <[email protected]>
Co-committed-by: Renovate <[email protected]>
CrispyBaguette pushed a commit to CrispyBaguette/wasm-palette-converter that referenced this pull request Nov 9, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [react-dom](https://reactjs.org/) ([source](https://github.com/facebook/react/tree/HEAD/packages/react-dom)) | dependencies | minor | [`18.1.0` -> `18.3.1`](https://renovatebot.com/diffs/npm/react-dom/18.1.0/18.3.1) |
| [@types/react-dom](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/react-dom) ([source](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/react-dom)) | devDependencies | minor | [`18.0.3` -> `18.3.1`](https://renovatebot.com/diffs/npm/@types%2freact-dom/18.0.3/18.3.1) |

---

### Release Notes

<details>
<summary>facebook/react (react-dom)</summary>

### [`v18.3.1`](https://github.com/facebook/react/blob/HEAD/CHANGELOG.md#1831-April-26-2024)

[Compare Source](facebook/react@v18.3.0...v18.3.1)

-   Export `act` from `react` [f1338f](facebook/react@f1338f8)

### [`v18.3.0`](https://github.com/facebook/react/blob/HEAD/CHANGELOG.md#1830-April-25-2024)

[Compare Source](facebook/react@v18.2.0...v18.3.0)

This release is identical to 18.2 but adds warnings for deprecated APIs and other changes that are needed for React 19.

Read the [React 19 Upgrade Guide](https://react.dev/blog/2024/04/25/react-19-upgrade-guide) for more info.

##### React

-   Allow writing to `this.refs` to support string ref codemod [909071](facebook/react@9090712)
-   Warn for deprecated `findDOMNode` outside StrictMode [c3b283](facebook/react@c3b2839)
-   Warn for deprecated `test-utils` methods [d4ea75](facebook/react@d4ea75d)
-   Warn for deprecated Legacy Context outside StrictMode [415ee0](facebook/react@415ee0e)
-   Warn for deprecated string refs outside StrictMode [#&#8203;25383](facebook/react#25383)
-   Warn for deprecated `defaultProps` for function components [#&#8203;25699](facebook/react#25699)
-   Warn when spreading `key` [#&#8203;25697](facebook/react#25697)
-   Warn when using `act` from `test-utils` [d4ea75](facebook/react@d4ea75d)

##### React DOM

-   Warn for deprecated `unmountComponentAtNode` [8a015b](facebook/react@8a015b6)
-   Warn for deprecated `renderToStaticNodeStream` [#&#8203;28874](facebook/react#28874)

### [`v18.2.0`](https://github.com/facebook/react/blob/HEAD/CHANGELOG.md#1820-June-14-2022)

[Compare Source](facebook/react@v18.1.0...v18.2.0)

##### React DOM

-   Provide a component stack as a second argument to `onRecoverableError`. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24591](facebook/react#24591))
-   Fix hydrating into `document` causing a blank page on mismatch. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24523](facebook/react#24523))
-   Fix false positive hydration errors with Suspense. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24480](facebook/react#24480) and  [@&#8203;acdlite](https://github.com/acdlite) in [#&#8203;24532](facebook/react#24532))
-   Fix ignored `setState` in Safari when adding an iframe. ([@&#8203;gaearon](https://github.com/gaearon) in [#&#8203;24459](facebook/react#24459))

##### React DOM Server

-   Pass information about server errors to the client. ([@&#8203;salazarm](https://github.com/salazarm) and [@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24551](facebook/react#24551) and [#&#8203;24591](facebook/react#24591))
-   Allow to provide a reason when aborting the HTML stream. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24680](facebook/react#24680))
-   Eliminate extraneous text separators in the HTML where possible. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24630](facebook/react#24630))
-   Disallow complex children inside `<title>` elements to match the browser constraints. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24679](facebook/react#24679))
-   Fix buffering in some worker environments by explicitly setting `highWaterMark` to `0`. ([@&#8203;jplhomer](https://github.com/jplhomer) in [#&#8203;24641](facebook/react#24641))

##### Server Components (Experimental)

-   Add support for `useId()` inside Server Components. ([@&#8203;gnoff](https://github.com/gnoff) in [#&#8203;24172](facebook/react#24172))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4xNDIuNyIsInVwZGF0ZWRJblZlciI6IjM4LjE0Mi43IiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=-->

Reviewed-on: https://gitea.bruyant.xyz/alexandre/PaletteSwitcher/pulls/50
Co-authored-by: Renovate <[email protected]>
Co-committed-by: Renovate <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants