feat: support React 18

[lmiller1990]

• Closes Cypress Testing fails to build on React version 18 #21381
• Closes React 18 support - today logs a bunch of warnings for ReactDom.render being deprecated #21946

User facing changelog

Add support for React 18 to npm/react.

Additional details

• updated peerDependencies to allow installing @cypress/react with react@18
• some specific code in vite and webpack dev servers to account for the potentially not available react-dom/client code

The problem and solution is described in detail in the README in this repo which is an example project used for an internal presentation at Cypress.

Steps to test

• Build binary (local should be fine too, but binary is more "production like")
• Try using Cypress CT with a React 18 project (using new react-dom/client API)
  • For example
  • webpack: yarn create react-app my-app
  • vite: yarn create vite --template react my-vite-react-app
  • Point Cypress at your app
  • Configure React using Launchpad
  • Should work, no errors in console, etc.
• No warning/error in console about deprecated React 18 API etc.

How has the user experience changed?

PR Tasks

• Have tests been added/updated?
• Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
• Has a PR for user-facing changes been opened in cypress-documentation?
• Have API changes been updated in the type definitions?

[cypress-bot]

Thanks for taking the time to open a PR!

• Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
• Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

[cypress]

---

Test summary

37715 • 0 • 458 • 0 • 10

---

Run details

Project	cypress
Status	Passed
Commit	86653c7
Started	Jul 14, 2022 11:57 AM
Ended	Jul 14, 2022 12:17 PM
Duration	20:33 💡
OS	Linux Debian - 10.11
Browser	Multiple

View run in Cypress Dashboard ➡️

---

Flakiness

	e2e/origin/commands/assertions.cy.ts		1
1	cy.origin assertions > #consoleProps > .should() and .and()
	commands/actions/click.cy.js		1
1	... > scroll-behavior > can scroll to and click elements in html with scroll-behavior: smooth
	commands/xhr.cy.js		3
1	... > logs request + response headers
2	... > logs request + response headers
3	... > logs Method, Status, URL, and XHR
This comment includes only the first 5 flaky tests. See all 10 flaky tests in the Cypress Dashboard.

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[astone123]

Works well for me 👍🏻

[lmiller1990]

Something is not right here. I built the binary locally, works great - system tests, work great, but @astone123 and I ran into some weirdness when testing against the pre-release.

I got an issue with react@17 and vite@2. Some kind of error 504 loop. It seems to only happen when using React 17 (weird).

The Vite workaround is not working for whatever reason...

✘ [ERROR] Could not resolve "react-dom/client"

    node_modules/cypress/react/dist/cypress-react.esm-bundler.js:845:54:
      845 │ ...            : function () { return import('react-dom/client'); };
          ╵                                              ~~~~~~~~~~~~~~~~~~

  You can mark the path "react-dom/client" as external to exclude it from the bundle, which will remove this error. You can also add ".catch()" here to handle this failure at run-time instead of bundle-time.

Edit... removing the supportFile imports seems to fix it.

import { mount } from 'cypress/react'

Cypress.Commands.add('mount', mount)

And what do you know - the system tests are not doing this import! This is the problem.

Weirdly enough the system tests don't exhibit this, even with the import. The problem seems to a circular import?

Edit: related? vitejs/vite#8715 (comment)

A 504 here is normal after missing dependencies have been found. On-fly deps requests will respond with a 504 as these are outdated. The browser will reload the page and ask these again. Do you have code checking for these in the tests? It should ignore these 504 for deps.

[lmiller1990]

I'm going to leave my debugging notes, in case it helps someone.

Notes 1:

---

Sodatea from Vite team suggests looking here for 504 related:

• https://github.com/vitejs/vite/blob/cda5442fc5eae9a571536fb2fcea9d9f16c28365/packages/vite/src/node/server/middlewares/transform.ts#L203
• https://github.com/vitejs/vite/blob/cda5442fc5eae9a571536fb2fcea9d9f16c28365/packages/vite/src/node/plugins/importAnalysis.ts#L224-L230

Reproduction: https://github.com/lmiller1990/vite-react-17-error-with-cypress (uses this pre-release). This is only a problem for React 17 and Vite. And not happening in system-tests, only in prod. I don't understand why.

If I add React 18, it's fine. So... related? https://github.com/facebook/react/blob/v18.0.0/packages/react/package.json#L22-L30. React 17 does not do exports, so it's possible Vite does some kind of different module resolution.

---

Notes 2:
Ok the 504 is from this line.

if (e?.code === ERR_OUTDATED_OPTIMIZED_DEP) {
  // Skip if response has already been sent
  if (!res.writableEnded) {
    res.statusCode = 504 // status code request timeout
    res.end()
  }
}

I guess we either have 1) race condition or 2) loop. But why only manifesting with React 18? I guess React 18 can be optimized by Vite, but not React 17? 🤔

---

Notes 3:

I messed with node_modules/cypress/react/dist/cypress-react.esm-bundler.js. This is the core issue? importReactModules:

function importReactModules() {
   // ...
                    react = (_a = react === null || react === void 0 ? void 0 : react.default) !== null && _a !== void 0 ? _a : react;
                    reactDomImport = isLessThan18(react)
                        ? function () { return import('react-dom'); }
                        // @ts-ignore
                        : function () { return import('react-dom/client'); };
                    return [4 /*yield*/, reactDomImport()];
              
  // ...
}

I'm almost 100% sure my plugin isn't getting called and/or applied. Real question is why isn't CI failing?
When using <=17, the mere presence of react-dom/client causes a endless 504. Changing to react-dom stops the 504. Either the Vite plugin I wrote isn't working, or something else is occurring.

---

Notes 4:

I am not sure why this works in the monorepo - maybe something to do with our symlinks. I cannot get this working in a production environment! I am starting to wonder if we should scrap the complexity and just have a cypress/react18 adapter with no dynamic imports or need to modifying the dev-server module resolutions.

---

Notes 5:

Hmm maybe just

optimizeDeps: {
  exclude: ["react-dom/client"]
}

Can work? It seems to work in when running against the built binary, but NOT in system tests. The different between the two is not at all clear, but we need to find out so we can reliably test this.

This works in a built binary, but seems not to work in system tests. Probably due to vitejs/vite#8186 and vitejs/vite#6582. Seems Vite is quite different in prod (after deployed) vs in system tests.

I got this to work! Maybe we can use this approach. We transform and eliminate the react-dom/client import for React <= 17.

import { defineConfig } from 'vite'
import path from 'path'
import react from '@vitejs/plugin-react'

let shouldTransform = false
try {
  const react = require('react')
  shouldTransform = react.version.startsWith('17')
} catch (e) {
  // not using react
}

function plugin () {
  return {
    enforce: 'pre',
    transform: (code, id) => {
      if (shouldTransform && id.includes('cypress_react.js')) {
        // remove problematic code via transform!
        return code.replace('react-dom/client', 'react-dom')
      }
    }
  }
}

const exclude = []

if (shouldTransform) {
  // So Vite won't crawl and optimize.
  exclude.push('react-dom/client')
}

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [plugin(), react()],
  optimizeDeps: {
    exclude
  }
})

Trying it out: 86653c7

[ZachJW34]

In response to #22633 (comment):

The only reason we are concerned about the behavior of what bundlers/preprocessors will do is because of the bundler hacks we are doing in order to support React17 and React18 in a single package. Ideally, a single package is best as it provides the user with the best experience ("it just works" idealogy), but as you pointed out we are violating this since e2e tests will fail due to the webpack-preprocessor not having the included bundler hacks if it happens to import the mount.

In Slack you mentioned publishing another package (@cypress/react18) and I am leaning in that direction. Generally, I don't think we should introduce bundler hacks to support multi-version support. If we can get away with it without the hacks then sure, but this PR is leading us down a rabbit hole and I think that's a sign to go forward with separating the support into multiple packages.

As we expand our framework support, I fear these hacks will continue to grow. @lmiller1990 what are your thoughts on proceeding with what we have now vs publishing multiple packages? If we decide on the multiple packages, we will have some things to think through with regards to versioning.

[BlueWinds]

In Slack you mentioned publishing another package (@cypress/react18) and I am leaning in that direction. Generally, I don't think we should introduce bundler hacks to support multi-version support. If we can get away with it without the hacks then sure, but this PR is leading us down a rabbit hole and I think that's a sign to go forward with separating the support into multiple packages.

This is the path we've gone down with the vue packages already (vue (for vue3) / vue2) - when we need to support another version, we release another module. I personally would have preferred we go with vueN (supporting version N) rather than vue (supporting vue3) and vueN (supporting older version N), but regardless of the naming, I like the idea of multiple packages when we need to support major versions of component libraries.

[lmiller1990]

I too am leaning towards two packages. I (finally) solved all the problems here, but what a mess - just because we can, doesn't mean we should, comes to mind.

Main question to answer: when does cypress/react18 become the "main" package (ala cypress/react) and the current cypress/react become cypress/react-17? cypress/vue points to the main version on npm - you'd expect cypress/react to do so, but it won't - this is confusing and breaks our convention.

Maybe we alias cypress/vue to cypress/vue3 and promote importing the major you want?

Also we can loop in Jordan for Angular - Angular drops a new major once every 6 months? We should fine a pattern that fits angular.

[BlueWinds]

Maybe we alias cypress/vue to cypress/vue3 and promote importing the major you want?

That seems like a good way forward - don't break anyone, create universal support for a new pattern.

Also we can loop in Jordan for Angular - Angular drops a new major once every 6 months? We should fine a pattern that fits angular.

Hm, I know very little about angular - getting someone who does to chime in sounds like a good step forward. Shouldn't assume that every package operates the same.

[lmiller1990]

@BlueWinds ok, I'll prepare a tech brief and include you and Jordan and Zach.

[astone123]

With this approach, what happens if a breaking change comes out of a minor or patch release from react/vue/angular?

Will we internalize those changes in the major package, or will we create a new package for the patch/minor?

[lmiller1990]

@astone123 I'm unsure about how to handle minor breaking changes, if the ever happen. I've added this concern as an outstanding question in the technical brief (Cypress internal team only, but sure would be nice to find a way to make our technical breakdowns more public some day).

Historically neither Vue or React has had breaking change w.r.t render APIs in a minor version. Makes sense - this would be a lot of apps, so it's unlikely, but definitely something we need to consider. We could likely apply what we learned in this PR to that situation if it ever occurs - runtime transform via webpack/vite, although I hope it won't come to that.