Add ReactDOMClient to ServerIntegrationTestUtils #28130

rickhanlonii · 2024-01-27T22:57:50Z

Overview

Adds support for ReactDOMClient for ServerIntegration* tests.

Also converts tests that pass without any other changes. Will follow up with other PRs for more complex cases.

react-sizebot · 2024-01-27T23:06:14Z

Comparing: 9aef5d2...e204d76

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	=	176.59 kB	176.59 kB	=	55.01 kB	55.01 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	178.69 kB	178.69 kB	=	55.64 kB	55.64 kB
facebook-www/ReactDOM-prod.classic.js	=	573.52 kB	573.52 kB	=	100.99 kB	100.99 kB
facebook-www/ReactDOM-prod.modern.js	=	557.29 kB	557.29 kB	=	98.07 kB	98.07 kB
test_utils/ReactAllWarnings.js	Deleted	67.55 kB	0.00 kB	Deleted	16.54 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
test_utils/ReactAllWarnings.js	Deleted	67.55 kB	0.00 kB	Deleted	16.54 kB	0.00 kB

Generated by 🚫 dangerJS against e204d76

rickhanlonii · 2024-02-01T21:31:13Z

packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js

+        if (ReactDOMClient) {
+          ReactDOMClient.hydrateRoot(domElement, reactElement, {
+            onRecoverableError: () => {
+              // TODO: assert on recoverable error count.


Updated to add and ignore onRecoverableError errors. I started to add support for asserting this count, but it's going to require a lot more thought and tests to be updated so I'm not adding it here.

rickhanlonii · 2024-02-01T21:32:38Z

scripts/jest/shouldIgnoreConsoleError.js

+        ) !== -1
+      ) {
+        // This also gets logged by onRecoverableError, so we can ignore it.
+        return true;


There is still one error that is duplicated in console.error and onRecoverableError, so it's fine to ignore. There's a comment on that error to remove it:

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

Line 1682 in 7636126

// TODO: This gets logged by onRecoverableError, too, so we should be

## Overview Branched off #28130 ## React for count changing ### Before These tests are weird because on main they pass, but log to the console: ``` We expected 2 warning(s), but saw 1 warning(s). We saw these warnings: Warning: Expected server HTML to contain a matching <select> in <div>. at select ``` The other one is ignored. The `expect(console.errors).toBeCalledWith(2)` doesn't account for ignored calls, so the test passes with the two expected (the +1 is in the test utiles). The ignored warning is ``` Warning: ReactDOM.hydrate is no longer supported in React 18. Use hydrateRoot instead. ``` So the mismatch is in the ignored warnings. ### After After switching to `createRoot`, it still logs: ``` We expected 2 warning(s), but saw 1 warning(s). We saw these warnings: Warning: Expected server HTML to contain a matching <select> in <div>. at select ``` But the test fails due to an unexpected error count. The new ignored errors are: ``` Error: Uncaught [Error: Hydration failed because the initial UI does not match what was rendered on the server.] Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>. Error: Hydration failed because the initial UI does not match what was rendered on the server. Error: There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering. ``` These seem to be the correct warnings to fire in `createRoot`, so the fix is to update the number of warnings we expect.

## Overview Branched off #28130 In `hydrateRoot`, we now error if you pass `undefined`: ``` Warning: Must provide initial children as second argument to hydrateRoot. ``` So we expect 1 error for this now.

@eps1lon

## Overview Branched off #28130 ## ~Failing~ Fixed by @eps1lon The tests are currently failing because of two tests covering special characters. I've tried a few ways to fix, but I'm stuck and will need some help understanding why they fail and how to fix. --------- Co-authored-by: Sebastian Silbermann <sebastian.silbermann@klarna.com>

@eps1lon

## Overview Branched off #28130 ### ~Failing~ Fixed by @eps1lon Most of the tests pass, but there are 3 tests that have additional warnings due to client render error retries. For example, before we would log: ``` Warning: Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks. Warning: Expected server HTML to contain a matching text node for "0" in <div>. ``` And now we log ``` Warning: Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks. Warning: Expected server HTML to contain a matching text node for "0" in <div>. Warning: Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks. ``` We can't just update the expected error count for these tests, because the additional error only happens on the client. So I need some guidance on how to fix these. --------- Co-authored-by: Sebastian Silbermann <sebastian.silbermann@klarna.com>

## Overview Branched off #28130 ## Why In #24276 we changed the new root behavior to error when a client-render is forced for certain cases, so these now expect a mismatch even though they're using `suppressHydrationWarning`.

## Overview Branched off #28130 Converts to `createRoot`, with a few additional in-line conversions in each file.

## Overview Adds support for `ReactDOMClient` for `ServerIntegration*` tests. Also converts tests that pass without any other changes. Will follow up with other PRs for more complex cases.

## Overview Branched off #28130 ## React for count changing ### Before These tests are weird because on main they pass, but log to the console: ``` We expected 2 warning(s), but saw 1 warning(s). We saw these warnings: Warning: Expected server HTML to contain a matching <select> in <div>. at select ``` The other one is ignored. The `expect(console.errors).toBeCalledWith(2)` doesn't account for ignored calls, so the test passes with the two expected (the +1 is in the test utiles). The ignored warning is ``` Warning: ReactDOM.hydrate is no longer supported in React 18. Use hydrateRoot instead. ``` So the mismatch is in the ignored warnings. ### After After switching to `createRoot`, it still logs: ``` We expected 2 warning(s), but saw 1 warning(s). We saw these warnings: Warning: Expected server HTML to contain a matching <select> in <div>. at select ``` But the test fails due to an unexpected error count. The new ignored errors are: ``` Error: Uncaught [Error: Hydration failed because the initial UI does not match what was rendered on the server.] Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>. Error: Hydration failed because the initial UI does not match what was rendered on the server. Error: There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering. ``` These seem to be the correct warnings to fire in `createRoot`, so the fix is to update the number of warnings we expect.

## Overview Branched off #28130 In `hydrateRoot`, we now error if you pass `undefined`: ``` Warning: Must provide initial children as second argument to hydrateRoot. ``` So we expect 1 error for this now.

@eps1lon

## Overview Branched off #28130 ## ~Failing~ Fixed by @eps1lon The tests are currently failing because of two tests covering special characters. I've tried a few ways to fix, but I'm stuck and will need some help understanding why they fail and how to fix. --------- Co-authored-by: Sebastian Silbermann <sebastian.silbermann@klarna.com>

@eps1lon

## Overview Branched off #28130 ### ~Failing~ Fixed by @eps1lon Most of the tests pass, but there are 3 tests that have additional warnings due to client render error retries. For example, before we would log: ``` Warning: Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks. Warning: Expected server HTML to contain a matching text node for "0" in <div>. ``` And now we log ``` Warning: Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks. Warning: Expected server HTML to contain a matching text node for "0" in <div>. Warning: Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks. ``` We can't just update the expected error count for these tests, because the additional error only happens on the client. So I need some guidance on how to fix these. --------- Co-authored-by: Sebastian Silbermann <sebastian.silbermann@klarna.com>

## Overview Branched off #28130 ## Why In #24276 we changed the new root behavior to error when a client-render is forced for certain cases, so these now expect a mismatch even though they're using `suppressHydrationWarning`.

## Overview Branched off #28130 Converts to `createRoot`, with a few additional in-line conversions in each file.

## Overview Adds support for `ReactDOMClient` for `ServerIntegration*` tests. Also converts tests that pass without any other changes. Will follow up with other PRs for more complex cases.

## Overview Branched off facebook#28130 ## React for count changing ### Before These tests are weird because on main they pass, but log to the console: ``` We expected 2 warning(s), but saw 1 warning(s). We saw these warnings: Warning: Expected server HTML to contain a matching <select> in <div>. at select ``` The other one is ignored. The `expect(console.errors).toBeCalledWith(2)` doesn't account for ignored calls, so the test passes with the two expected (the +1 is in the test utiles). The ignored warning is ``` Warning: ReactDOM.hydrate is no longer supported in React 18. Use hydrateRoot instead. ``` So the mismatch is in the ignored warnings. ### After After switching to `createRoot`, it still logs: ``` We expected 2 warning(s), but saw 1 warning(s). We saw these warnings: Warning: Expected server HTML to contain a matching <select> in <div>. at select ``` But the test fails due to an unexpected error count. The new ignored errors are: ``` Error: Uncaught [Error: Hydration failed because the initial UI does not match what was rendered on the server.] Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>. Error: Hydration failed because the initial UI does not match what was rendered on the server. Error: There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering. ``` These seem to be the correct warnings to fire in `createRoot`, so the fix is to update the number of warnings we expect.

## Overview Branched off facebook#28130 In `hydrateRoot`, we now error if you pass `undefined`: ``` Warning: Must provide initial children as second argument to hydrateRoot. ``` So we expect 1 error for this now.

@eps1lon

## Overview Branched off facebook#28130 ## ~Failing~ Fixed by @eps1lon The tests are currently failing because of two tests covering special characters. I've tried a few ways to fix, but I'm stuck and will need some help understanding why they fail and how to fix. --------- Co-authored-by: Sebastian Silbermann <sebastian.silbermann@klarna.com>

@eps1lon

…8135) ## Overview Branched off facebook#28130 ### ~Failing~ Fixed by @eps1lon Most of the tests pass, but there are 3 tests that have additional warnings due to client render error retries. For example, before we would log: ``` Warning: Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks. Warning: Expected server HTML to contain a matching text node for "0" in <div>. ``` And now we log ``` Warning: Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks. Warning: Expected server HTML to contain a matching text node for "0" in <div>. Warning: Do not call Hooks inside useEffect(...), useMemo(...), or other built-in Hooks. ``` We can't just update the expected error count for these tests, because the additional error only happens on the client. So I need some guidance on how to fix these. --------- Co-authored-by: Sebastian Silbermann <sebastian.silbermann@klarna.com>

## Overview Branched off facebook#28130 ## Why In facebook#24276 we changed the new root behavior to error when a client-render is forced for certain cases, so these now expect a mismatch even though they're using `suppressHydrationWarning`.

…#28131) ## Overview Branched off facebook#28130 Converts to `createRoot`, with a few additional in-line conversions in each file.

## Overview Adds support for `ReactDOMClient` for `ServerIntegration*` tests. Also converts tests that pass without any other changes. Will follow up with other PRs for more complex cases. DiffTrain build for commit 6054be9.

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jan 27, 2024

rickhanlonii force-pushed the rh/server-test-utils branch from 806c110 to 233b65d Compare January 27, 2024 23:02

eps1lon approved these changes Jan 28, 2024

View reviewed changes

Add ReactDOMClient to ServerIntegrationTestUtils

e204d76

rickhanlonii force-pushed the rh/server-test-utils branch from 233b65d to e204d76 Compare February 1, 2024 20:53

rickhanlonii commented Feb 1, 2024

View reviewed changes

rickhanlonii merged commit 6054be9 into main Feb 1, 2024
36 checks passed

rickhanlonii deleted the rh/server-test-utils branch February 1, 2024 23:27

rickhanlonii added a commit that referenced this pull request Feb 2, 2024

Add ReactDOMClient to ServerIntegration tests (minor fixes) (#28131)

b25dcd3

## Overview Branched off #28130 Converts to `createRoot`, with a few additional in-line conversions in each file.

gaearon pushed a commit that referenced this pull request Feb 3, 2024

Add ReactDOMClient to ServerIntegration tests (minor fixes) (#28131)

4cc2e33

## Overview Branched off #28130 Converts to `createRoot`, with a few additional in-line conversions in each file.

gnoff mentioned this pull request Feb 5, 2024

Update React from 60a927d04 to 2bc7d336a vercel/next.js#61522

Merged

ztanner mentioned this pull request Mar 19, 2024

(backport) Update React from 60a927d04 to 14898b6a9 vercel/next.js#63442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ReactDOMClient to ServerIntegrationTestUtils #28130

Add ReactDOMClient to ServerIntegrationTestUtils #28130

rickhanlonii commented Jan 27, 2024

react-sizebot commented Jan 27, 2024 •

edited

Loading

rickhanlonii Feb 1, 2024

rickhanlonii Feb 1, 2024

Add ReactDOMClient to ServerIntegrationTestUtils #28130

Add ReactDOMClient to ServerIntegrationTestUtils #28130

Conversation

rickhanlonii commented Jan 27, 2024

Overview

react-sizebot commented Jan 27, 2024 • edited Loading

Critical size changes

Significant size changes

rickhanlonii Feb 1, 2024

Choose a reason for hiding this comment

rickhanlonii Feb 1, 2024

Choose a reason for hiding this comment

react-sizebot commented Jan 27, 2024 •

edited

Loading