Skip to content
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

feat: support React 18 #22437

Closed
wants to merge 63 commits into from
Closed
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
39ca08b
fix: add tests for React 18, bump peerDeps
lmiller1990 Jun 15, 2022
c255b79
update deps
lmiller1990 Jun 16, 2022
79d817e
update tests
lmiller1990 Jun 16, 2022
5cc4a4b
revert changes
lmiller1990 Jun 16, 2022
29025a8
work on vite with React 18
lmiller1990 Jun 16, 2022
d7819a0
handle case of default import
lmiller1990 Jun 16, 2022
262c4f1
try work-around
lmiller1990 Jun 16, 2022
dd1e922
update dep installer and snapshots
lmiller1990 Jun 17, 2022
36de0e6
update snapshots
lmiller1990 Jun 17, 2022
0ca2f89
update vite test, skip ever-red system test
lmiller1990 Jun 17, 2022
c82188b
add component testing spec w/ binary
lmiller1990 Jun 17, 2022
d9424af
tests
lmiller1990 Jun 17, 2022
a36e2de
remove failed spec
lmiller1990 Jun 17, 2022
1d62cb9
use ignore plugin
lmiller1990 Jun 21, 2022
065d1a3
async import for react/react-dom
lmiller1990 Jun 21, 2022
e97690d
rejig code
lmiller1990 Jun 21, 2022
1958fee
update tests
lmiller1990 Jun 22, 2022
8c3fa4f
Merge remote-tracking branch 'origin/develop' into lmiller/21381-reac…
lmiller1990 Jun 22, 2022
dfd5b81
simplify react mount
lmiller1990 Jun 22, 2022
05f3988
fix vite dev server plugin
lmiller1990 Jun 22, 2022
16999b3
make ignore expression more generic
lmiller1990 Jun 22, 2022
12c2d82
update snapshot
lmiller1990 Jun 22, 2022
8b219f4
update deps
lmiller1990 Jun 22, 2022
552adb2
update test
lmiller1990 Jun 22, 2022
7114fb5
do not cache react and react-dom
lmiller1990 Jun 22, 2022
951df51
correctly return value from mountHook
lmiller1990 Jun 22, 2022
f9a3da9
increase timeout - slower due to not caching react/react-dom
lmiller1990 Jun 22, 2022
b2f80f0
add react deps
lmiller1990 Jun 22, 2022
4d8ebcf
update webpack-dev-server fresh snapshots
lmiller1990 Jun 23, 2022
ffe657f
update snapshots on linux
lmiller1990 Jun 23, 2022
0c7e0d1
clean stdout for snapshots
lmiller1990 Jun 23, 2022
e66c0ab
hack: work around regexp
lmiller1990 Jun 23, 2022
91e9c66
export webpack handler
lmiller1990 Jun 23, 2022
8c82f89
alias react-dom
lmiller1990 Jun 23, 2022
5d4fba6
special handling for next because next is special
lmiller1990 Jun 23, 2022
f550539
Merge branch 'develop' into lmiller/21381-react-18-with-ignore
lmiller1990 Jun 28, 2022
cbeddd4
fix test
lmiller1990 Jun 28, 2022
dd1421c
Merge branch 'lmiller/21381-react-18-with-ignore' of github.com:cypre…
lmiller1990 Jun 28, 2022
3f1fc26
fix test
lmiller1990 Jun 28, 2022
2c9f756
include react and react-dom dep
lmiller1990 Jun 28, 2022
1cad3aa
fix test
lmiller1990 Jun 28, 2022
e545b71
build binary
lmiller1990 Jul 1, 2022
00c104a
Merge remote-tracking branch 'origin/develop' into lmiller/21381-reac…
lmiller1990 Jul 1, 2022
c99ce39
move tests from e2e project to smaller one w/ react deps
lmiller1990 Jul 1, 2022
6184309
mac build
lmiller1990 Jul 1, 2022
395743a
fix: check for default on module imports
lmiller1990 Jul 8, 2022
bae3606
Merge branch 'develop' into lmiller/21381-react-18-with-ignore
lmiller1990 Jul 11, 2022
8b07baf
use 18.x for deps
lmiller1990 Jul 12, 2022
ddf3be9
refactor webpack react logic into function
lmiller1990 Jul 12, 2022
0ed5761
use semver lib
lmiller1990 Jul 12, 2022
e6441c8
correct semver import
lmiller1990 Jul 12, 2022
48f5fd5
try removing not necessary webpack module sourcing
lmiller1990 Jul 12, 2022
9ec43b4
fix: use sourced webpack IgnorePlugin (#22763)
ZachJW34 Jul 13, 2022
219c1b2
Merge remote-tracking branch 'origin/develop' into lmiller/21381-reac…
lmiller1990 Jul 13, 2022
88d1fba
add debugging
lmiller1990 Jul 14, 2022
4f1dd81
Merge branch 'lmiller/21381-react-18-with-ignore' of https://github.c…
lmiller1990 Jul 14, 2022
d9aefd5
update deps
lmiller1990 Jul 14, 2022
8c1f0ad
update test to use cy.mount
lmiller1990 Jul 14, 2022
ccf77bb
skip test
lmiller1990 Jul 14, 2022
f01cd86
fixing tests
lmiller1990 Jul 14, 2022
86653c7
use transform API to remove react-dom/client import
lmiller1990 Jul 14, 2022
58c4795
add additional check
lmiller1990 Jul 14, 2022
42c57f8
merge in origin/develop
lmiller1990 Jul 17, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions npm/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@
"vite-plugin-require-transform": "1.0.3"
},
"peerDependencies": {
"@types/react": "^16.9.16 || ^17.0.0",
"@types/react": "^16.9.16 || ^17.0.0 || ^18.0.0",
"cypress": "*",
"react": "^=16.x || ^=17.x",
"react-dom": "^=16.x || ^=17.x"
"react": "^=16.x || ^=17.x || ^=18",
BlueWinds marked this conversation as resolved.
Show resolved Hide resolved
"react-dom": "^=16.x || ^=17.x || ^=18"
},
"files": [
"dist"
Expand Down
2 changes: 2 additions & 0 deletions npm/react/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ function createEntry (options) {
external: [
'react',
'react-dom',
'react-dom/client',
],
plugins: [
resolve(), commonjs(),
Expand All @@ -36,6 +37,7 @@ function createEntry (options) {
globals: {
react: 'React',
'react-dom': 'ReactDOM',
'react-dom/client': 'ReactDOMClient',
},
},
}
Expand Down
147 changes: 95 additions & 52 deletions npm/react/src/mount.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react'
import * as ReactDOM from 'react-dom'
import type * as React from 'react'
import type * as ReactDOM from 'react-dom'
import getDisplayName from './getDisplayName'
import {
injectStylesBeforeElement,
Expand Down Expand Up @@ -41,6 +41,36 @@ const injectStyles = (options: MountOptions) => {
**/
export const mount = (jsx: React.ReactNode, options: MountOptions = {}) => _mount('mount', jsx, options)

const versionRe = /(\d.+?)\./
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple other npm packages are using the semver package already - could we leverage it to do this versioning parse/check?

If it doesn't make sense to pull in that dependency then I'm curious about the structure of this regex. Seems like it's doing a "find a number followed by any character one or more times followed by a period anywhere in the string" - should it be something more like /^(\d+)\./ to find a one-or-more digit number at the start of the string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had the same thought on the semver library - I am more conscious about adding dependencies that bundle each time the user runs a test, since they could impact spec time. We grab their internal regexp: https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string

I think our regexp do the same thing - I can definitely add some unit tests around this regexp, now I've figured out the hard parts w.r.t the dependencies and system tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up adding semver since we can just import function-by-function which is nice and small


function getMajorVersion (semver: string) {
const match = semver.match(versionRe)
const toNum = match?.[1] && parseInt(match?.[1])

if (!toNum) {
throw Error(`Could not extract major version from ${semver}`)
}

return toNum
}

async function importReactModules () {
const react = await import('react')
const majorVersion = getMajorVersion(react.version)
const reactDomImport = majorVersion <= 17
BlueWinds marked this conversation as resolved.
Show resolved Hide resolved
? () => import('react-dom')
// @ts-ignore
: () => import('react-dom/client')

const reactDom = await reactDomImport()

return {
react,
reactDom,
majorVersion,
}
}

let lastMountedReactDom: (typeof ReactDOM) | undefined

/**
Expand All @@ -63,10 +93,6 @@ const _mount = (type: 'mount' | 'rerender', jsx: React.ReactNode, options: Mount
return cy
.then(injectStyles(options))
.then(() => {
const reactDomToUse = options.ReactDom || ReactDOM

lastMountedReactDom = reactDomToUse

const el = getContainerEl()

if (!el) {
Expand All @@ -84,54 +110,71 @@ const _mount = (type: 'mount' | 'rerender', jsx: React.ReactNode, options: Mount
key,
}

const reactComponent = React.createElement(
options.strict ? React.StrictMode : React.Fragment,
props,
jsx,
)
// since we always surround the component with a fragment
// let's get back the original component
const userComponent = (reactComponent.props as {
key: string
children: React.ReactNode
}).children

reactDomToUse.render(reactComponent, el)

if (options.log !== false) {
Cypress.log({
name: type,
type: 'parent',
message: [message],
// @ts-ignore
$el: (el.children.item(0) as unknown) as JQuery<HTMLElement>,
consoleProps: () => {
return {
// @ts-ignore protect the use of jsx functional components use ReactNode
props: jsx.props,
description: type === 'mount' ? 'Mounts React component' : 'Rerenders mounted React component',
home: 'https://github.com/cypress-io/cypress',
}
},
}).snapshot('mounted').end()
const logMount = () => {
if (options.log !== false) {
Cypress.log({
name: type,
type: 'parent',
message: [message],
// @ts-ignore
$el: (el.children.item(0) as unknown) as JQuery<HTMLElement>,
consoleProps: () => {
return {
// @ts-ignore protect the use of jsx functional components use ReactNode
props: jsx.props,
description: type === 'mount' ? 'Mounts React component' : 'Rerenders mounted React component',
home: 'https://github.com/cypress-io/cypress',
}
},
}).snapshot('mounted').end()
}
}

return (
// Separate alias and returned value. Alias returns the component only, and the thenable returns the additional functions
cy.wrap<React.ReactNode>(userComponent, { log: false })
.as(displayName)
.then(() => {
return cy.wrap<MountReturn>({
component: userComponent,
rerender: (newComponent) => _mount('rerender', newComponent, options, key),
unmount: () => _unmount({ boundComponentMessage: jsxComponentName, log: true }),
}, { log: false })
})
// by waiting, we delaying test execution for the next tick of event loop
// and letting hooks and component lifecycle methods to execute mount
// https://github.com/bahmutov/cypress-react-unit-test/issues/200
.wait(0, { log: false })
)
return importReactModules()
.then(({ react, reactDom, majorVersion }) => {
const reactDomToUse = options.ReactDom || reactDom

lastMountedReactDom = reactDomToUse

const reactComponent = react.createElement(
options.strict ? react.StrictMode : react.Fragment,
props,
jsx,
)
// since we always surround the component with a fragment
// let's get back the original component
const userComponent = (reactComponent.props as {
key: string
children: React.ReactNode
}).children

if (majorVersion <= 17) {
reactDom.render(reactComponent, el)
} else {
const root = reactDom.createRoot(el)

root.render(reactComponent)
}

logMount()

return (
// Separate alias and returned value. Alias returns the component only, and the thenable returns the additional functions
cy.wrap<React.ReactNode>(userComponent, { log: false })
.as(displayName)
.then(() => {
return cy.wrap<MountReturn>({
component: userComponent,
rerender: (newComponent) => _mount('rerender', newComponent, options, key),
unmount: () => _unmount({ boundComponentMessage: jsxComponentName, log: true }),
}, { log: false })
})
// by waiting, we delaying test execution for the next tick of event loop
// and letting hooks and component lifecycle methods to execute mount
// https://github.com/bahmutov/cypress-react-unit-test/issues/200
.wait(0, { log: false })
)
})
// Bluebird types are terrible. I don't think the return type can be carried without this cast
}) as unknown as globalThis.Cypress.Chainable<MountReturn>
}
Expand Down
16 changes: 8 additions & 8 deletions npm/react/src/mountHook.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import * as React from 'react'

import { mount } from './mount'

type MountHookResult<T> = {
Expand Down Expand Up @@ -77,11 +75,13 @@ function TestHook ({ callback, onError, children }: TestHookProps) {
export const mountHook = <T>(hookFn: (...args: any[]) => T) => {
const { result, setValue, setError } = resultContainer<T>()

const componentTest: React.ReactElement = React.createElement(TestHook, {
callback: hookFn,
onError: setError,
children: setValue,
})
return import('react').then((reactMod) => {
const componentTest = reactMod.createElement(TestHook, {
callback: hookFn,
onError: setError,
children: setValue,
})

return mount(componentTest).then(() => result)
return mount(componentTest).then(() => result)
})
}
2 changes: 2 additions & 0 deletions npm/vite-dev-server/client/reactDomClientPlaceholder.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Placeholder ...
BlueWinds marked this conversation as resolved.
Show resolved Hide resolved
export default {}
2 changes: 2 additions & 0 deletions npm/vite-dev-server/src/plugins/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
export * from './inspect'

export * from './cypress'

export * from './react18'
31 changes: 31 additions & 0 deletions npm/vite-dev-server/src/plugins/react18.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import type { Plugin } from 'vite'
import path from 'path'

/**
* React 18 has changed it's API.
* https://reactjs.org/blog/2022/03/08/react-18-upgrade-guide.html#updates-to-client-rendering-apis
* We need to conditionally import `react-dom/client` in our React adapter:
*
* import('react-dom/client', (mod) => {...})
*
* In a browser environment, we can just try/catch, however Vite does some optimizations
* and will fail during this step if react-dom/client does not exist (React <= 17).
*
* To avoid this error and seamlessly support React 17 and 18 side by side we simply
* stub out the dynamic react-dom/client import and return a placeholder module.
*/
export const React18 = (projectRoot: string): Plugin => {
return {
name: 'cypress:missing-react-dom-client',
resolveId (source: string) {
if (source === 'react-dom/client') {
try {
return require.resolve('react-dom/client', { paths: [projectRoot] })
} catch (e) {
// This is not a react 18 project, need to stub out to avoid error
return path.resolve(__dirname, '..', '..', 'client', 'reactDomClientPlaceholder.js')
}
}
},
}
}
3 changes: 2 additions & 1 deletion npm/vite-dev-server/src/resolveConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import path from 'path'

import { configFiles } from './constants'
import type { ViteDevServerConfig } from './devServer'
import { Cypress, CypressInspect } from './plugins/index'
import { Cypress, CypressInspect, React18 } from './plugins/index'
import type { Vite } from './getVite'

const debug = debugFn('cypress:vite-dev-server:resolve-config')
Expand Down Expand Up @@ -84,6 +84,7 @@ export const createViteDevServerConfig = async (config: ViteDevServerConfig, vit
},
},
plugins: [
React18(config.cypressConfig.projectRoot),
Cypress(config, vite),
CypressInspect(config),
].filter((p) => p != null),
Expand Down
16 changes: 10 additions & 6 deletions npm/vite-dev-server/test/resolveConfig.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ describe('resolveConfig', function () {

const viteConfig = await createViteDevServerConfig(viteDevServerConfig, vite)

expect(viteConfig.plugins).to.have.lengthOf(1)
expect(viteConfig.plugins[0].name).to.equal('cypress:main')
expect(viteConfig.plugins).to.have.lengthOf(2)
expect(viteConfig.plugins[0].name).to.equal('cypress:missing-react-dom-client')
expect(viteConfig.plugins[1].name).to.equal('cypress:main')
})

context('with CYPRESS_INTERNAL_VITE_INSPECT provided', () => {
Expand All @@ -47,8 +48,10 @@ describe('resolveConfig', function () {

const viteConfig = await createViteDevServerConfig(viteDevServerConfig, vite)

expect(viteConfig.plugins).to.have.lengthOf(2)
expect(viteConfig.plugins[1].name).to.equal('cypress:inspect')
expect(viteConfig.plugins).to.have.lengthOf(3)
expect(viteConfig.plugins[0].name).to.equal('cypress:missing-react-dom-client')
expect(viteConfig.plugins[1].name).to.equal('cypress:main')
expect(viteConfig.plugins[2].name).to.equal('cypress:inspect')
})

it('should not add inspect plugin if not installed', async () => {
Expand All @@ -57,8 +60,9 @@ describe('resolveConfig', function () {

const viteConfig = await createViteDevServerConfig(viteDevServerConfig, vite)

expect(viteConfig.plugins).to.have.lengthOf(1)
expect(viteConfig.plugins[0].name).to.equal('cypress:main')
expect(viteConfig.plugins).to.have.lengthOf(2)
expect(viteConfig.plugins[0].name).to.equal('cypress:missing-react-dom-client')
expect(viteConfig.plugins[1].name).to.equal('cypress:main')
})
})
})
Expand Down
2 changes: 1 addition & 1 deletion npm/webpack-dev-server/src/helpers/nextHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ function getRunWebpackSpan (devServerConfig: WebpackDevServerConfig): { runWebpa

const originalModuleLoad = (Module as ModuleClass)._load

function sourceNextWebpackDeps (devServerConfig: WebpackDevServerConfig) {
export function sourceNextWebpackDeps (devServerConfig: WebpackDevServerConfig) {
const framework = sourceFramework(devServerConfig)!
const webpack = sourceNextWebpack(devServerConfig, framework)
const webpackDevServer = sourceWebpackDevServer(devServerConfig, framework)
Expand Down
Loading