Update ReactDom.render call to eliminate the deprecation warnings

[southp]

At the moment, accessing a local calypso instance dumps a series of ReactDOM.render deprecation warnings:

While it is not causing any user facing issue, it creates noises to developers that would be the best to eliminate. It might be as simple as updating this line: https://github.com/Automattic/wp-calypso/blob/trunk/client/boot/common.js#L406 , but it might also be more involved than that. When in doubt, we should check in with @Automattic/team-calypso .

[jsnajdr]

It might be as simple as updating this line:

In addition to this line that does the initial render on boot, you also need to update at least the render and hydrate calls in client/controller/web-utils.js. This is the render call that every route handler does as clientRender.

Especially the hydrate call might be tricky to migrate, as the new hydrateRoot API is quite different.

By the way, the Stepper app has already been migrated in #93724, but it doesn't have any server side rendering.

[southp]

#95339 was deployed and then reverted by #95704 soon. The pre-release tests were consistently failing like this: https://teamcity.a8c.com/buildConfiguration/calypso_calypso_WebApp_Calypso_E2E_Pre_Release/13582370?hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildChangesSection=true&expandBuildDeploymentsSection=false&expandBuildProblemsSection=true&expandBuildTestsSection=true, even though I had run them on the PR manually before deploying.

I'm reopening the issue. However, I'm not sure when I'd be able to revisit it again :(

[jsnajdr]

No worries, we can try again next week 🙂 The concurrent mode in React 18 often slightly breaks something.

[southp]

I've created a PR that reverts the revert to test it out: #95750. It looks like I can actually reproduce the error pretty consistently on my machine.

e.g. Running yarn workspace wp-e2e-tests test -- specs/onboarding/signup__start-writing-tailored.ts gave me the same error we saw in the TeamCity link above:

Artifacts for signup__start-writing-tailored: /Users/southp/a8c/wp-calypso/test/e2e/results/signup__start-writing-tailored__2024-10-28_10-57-49-378-uW47tl
 FAIL  specs/onboarding/signup__start-writing-tailored.ts (12.189 s)
  Signup: Tailored Start Writing Flow
    ✓ Navigate to /setup/start-writing (1945 ms)
    ✓ Sign up with email (4520 ms)
    ✕ Publish first post (99 ms)
    ○ skipped Add blog name and description
    ○ skipped Ensure domain search is working
    ○ skipped Skip the domain selection step
    ○ skipped Select WordPress.com Free plan
    ○ skipped Launch the blog
    ○ skipped Ensure "Connect to social" navigates to Marketing page

  ● Signup: Tailored Start Writing Flow › Publish first post

    page.waitForURL: net::ERR_ABORTED; maybe frame was detached?
    =========================== logs ===========================
    waiting for navigation until "load"
    ============================================================

      154 |             // it is a fairly good stand-in.
      155 |             await Promise.all( [
    > 156 |                     this.page.waitForURL( /(\/post\/.+|\/page\/+|\/post-new.php)/, { timeout } ),
          |                               ^
      157 |                     this.page.waitForResponse( /.*posts.*/, { timeout } ),
      158 |             ] );
      159 |     }

      at EditorPage.waitForURL [as waitForEditorLoadedRequests] (../../packages/calypso-e2e/src/lib/pages/editor-page.ts:156:14)
      at EditorPage.waitForEditorLoadedRequests [as waitUntilLoaded] (../../packages/calypso-e2e/src/lib/pages/editor-page.ts:136:15)
      at Object.waitUntilLoaded (specs/onboarding/signup__start-writing-tailored.ts:42:20)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 6 skipped, 2 passed, 9 total
Snapshots:   0 total
Time:        14.797 s

That's good, since at least it's reproducible. It's actually so consistent that I'm wondering how I managed to get it pass on my previous PR. I'll see if I can find some time to look into this in the following days.

[southp]

I've run out of my time budget looking into this :( so I'm leaving some more breadcrumbs for whoever picking it up in the future.

My best guess is that the way our e2e code handles asynchronous operations will need to be updated to match the behaviors of React 18 concurrent mode. However, I don't know what exactly that is.

For example, in the particular test, specs/onboarding/signup__start-writing-tailored.ts, running it locally, I saw the route transition as:
/setup/start-writing/check-site -> /start/account/user-social -> /setup/start-writing/check-site/ -> /start/account/user-social -> (abort)

Note that even though we should have logged in at the user-social step, /setup/start-writing acted as if we didn't so redirection to the user-social step happened again. That's when the test script expected to see the core editor, hence aborting.

However, if I simply added a 1 sec pending promise like so:

diff --git a/test/e2e/specs/onboarding/signup__start-writing-tailored.ts b/test/e2e/specs/onboarding/signup__start-writing-tailored.ts
index 2cfee48c77d..87b0cdbe5dd 100644
--- a/test/e2e/specs/onboarding/signup__start-writing-tailored.ts
+++ b/test/e2e/specs/onboarding/signup__start-writing-tailored.ts 

@@ -38,6 +38,12 @@ describe( 'Signup: Tailored Start Writing Flow', () => {
 	} );
 
 	it( 'Publish first post', async function () {
+		await new Promise<void>( (resolve) => {
+			setTimeout( () => {
+				resolve();
+			}, 1000 );
+		} );
+
 		const editorPage = new EditorPage( page );
 		await editorPage.waitUntilLoaded();
 		await page.getByLabel( 'Close', { exact: true } ).click();

The test would pass just fine. Also note that, if I followed the steps in the test script manually, everything worked just fine as well.

Thus my assumption is that the React concurrent mode has changed some of the asynchronous behavior in a subtle way that's liklely not visible to users, but critical to the e2e tests. Whoever picks up this task next will need to figure out that subtlety, and fix the tests accordingly.