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

fix: remove last mounted component upon subsequent mount calls #24470

Merged
merged 11 commits into from
Nov 3, 2022
53 changes: 36 additions & 17 deletions npm/angular/src/mount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ window.Mocha['__zone_patch__'] = false
import 'zone.js/testing'

import { CommonModule } from '@angular/common'
import { Component, ErrorHandler, EventEmitter, Injectable, SimpleChange, SimpleChanges, Type } from '@angular/core'
import { Component, ErrorHandler, EventEmitter, Injectable, SimpleChange, SimpleChanges, Type, OnChanges } from '@angular/core'
import {
ComponentFixture,
getTestBed,
Expand Down Expand Up @@ -72,6 +72,23 @@ export interface MountConfig<T> extends TestModuleMetadata {
componentProperties?: Partial<{ [P in keyof T]: T[P] }>
}

let activeFixture: ComponentFixture<any> | null = null

function cleanup () {
// Not public, we need to call this to remove the last component from the DOM
try {
(getTestBed() as any).tearDownTestingModule()
} catch (e) {
const notSupportedError = new Error(`Failed to teardown component. The version of Angular you are using may not be officially supported.`)

;(notSupportedError as any).docsUrl = 'https://on.cypress.io/component-framework-configuration'
throw notSupportedError
}

getTestBed().resetTestingModule()
activeFixture = null
}

/**
* Type that the `mount` function returns
* @type MountResponse<T>
Expand Down Expand Up @@ -209,6 +226,8 @@ function setupFixture<T> (
): ComponentFixture<T> {
const fixture = getTestBed().createComponent(component)

setupComponent(config, fixture)

fixture.whenStable().then(() => {
fixture.autoDetectChanges(config.autoDetectChanges ?? true)
})
Expand All @@ -223,17 +242,18 @@ function setupFixture<T> (
* @param {ComponentFixture<T>} fixture Fixture for debugging and testing a component.
* @returns {T} Component being mounted
*/
function setupComponent<T extends { ngOnChanges? (changes: SimpleChanges): void }> (
function setupComponent<T> (
config: MountConfig<T>,
fixture: ComponentFixture<T>): T {
let component: T = fixture.componentInstance
fixture: ComponentFixture<T>,
): void {
let component = fixture.componentInstance as unknown as { [key: string]: any } & Partial<OnChanges>

if (config?.componentProperties) {
component = Object.assign(component, config.componentProperties)
}

if (config.autoSpyOutputs) {
Object.keys(component).forEach((key: string, index: number, keys: string[]) => {
Object.keys(component).forEach((key) => {
const property = component[key]

if (property instanceof EventEmitter) {
Expand All @@ -252,14 +272,12 @@ function setupComponent<T extends { ngOnChanges? (changes: SimpleChanges): void
acc[key] = new SimpleChange(null, value, true)

return acc
}, {})
}, {} as {[key: string]: SimpleChange})

if (Object.keys(componentProperties).length > 0) {
component.ngOnChanges(simpleChanges)
}
}

return component
}

/**
Expand Down Expand Up @@ -295,13 +313,18 @@ export function mount<T> (
component: Type<T> | string,
config: MountConfig<T> = { },
): Cypress.Chainable<MountResponse<T>> {
// Remove last mounted component if cy.mount is called more than once in a test
if (activeFixture) {
cleanup()
}

const componentFixture = initTestBed(component, config)
const fixture = setupFixture(componentFixture, config)
const componentInstance = setupComponent(config, fixture)

activeFixture = setupFixture(componentFixture, config)

const mountResponse: MountResponse<T> = {
fixture,
component: componentInstance,
fixture: activeFixture,
component: activeFixture.componentInstance,
}

const logMessage = typeof component === 'string' ? 'Component' : componentFixture.name
Expand Down Expand Up @@ -338,8 +361,4 @@ getTestBed().initTestEnvironment(
},
)

setupHooks(() => {
// Not public, we need to call this to remove the last component from the DOM
getTestBed()['tearDownTestingModule']()
getTestBed().resetTestingModule()
})
setupHooks(cleanup)
6 changes: 3 additions & 3 deletions npm/angular/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
"allowJs": true,
"declaration": true,
"outDir": "dist",
"strict": false,
"noImplicitAny": false,
"strict": true,
"baseUrl": "./",
"types": [
"cypress"
],
"allowSyntheticDefaultImports": true,
"esModuleInterop": true,
"moduleResolution": "node"
"moduleResolution": "node",
"noPropertyAccessFromIndexSignature": true,
},
"include": ["src/**/*.*"],
"exclude": ["src/**/*-spec.*"]
Expand Down
15 changes: 12 additions & 3 deletions npm/mount-utils/create-rollup-entry.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,19 @@ export function createEntries (options) {
console.log(`Building ${format}: ${finalConfig.output.file}`)

return finalConfig
}).concat({
}).concat([{
input,
output: [{ file: 'dist/index.d.ts', format: 'es' }],
plugins: [dts({ respectExternal: true })],
plugins: [
dts({ respectExternal: true }),
{
name: 'cypress-types-reference',
// rollup-plugin-dts does not add '// <reference types="cypress" />' like rollup-plugin-typescript2 did so we add it here.
renderChunk (...[code]) {
return `/// <reference types="cypress" />\n\n${code}`
},
},
],
external: config.external || [],
})
}])
}
3 changes: 3 additions & 0 deletions npm/react/src/mount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ export function mount (jsx: React.ReactNode, options: MountOptions = {}, rerende
Cypress.log({ name: 'warning', message })
}

// Remove last mounted component if cy.mount is called more than once in a test
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment only applicable in React 18 or should it be here as well?

// React by default removes the last component when calling render, but we should remove the root
// to wipe away any state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only applicable to 18, react17 doesn't have the concept of root but just a rerender into the same element.

cleanup()

const internalOptions: InternalMountOptions = {
reactDom: ReactDOM,
render: (reactComponent: ReturnType<typeof React.createElement>, el: HTMLElement, reactDomToUse: typeof ReactDOM) => {
Expand Down
4 changes: 4 additions & 0 deletions npm/react18/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ const cleanup = () => {
}

export function mount (jsx: React.ReactNode, options: MountOptions = {}, rerenderKey?: string) {
// Remove last mounted component if cy.mount is called more than once in a test
// React by default removes the last component when calling render, but we should remove the root
// to wipe away any state
cleanup()
const internalOptions: InternalMountOptions = {
reactDom: ReactDOM,
render: (reactComponent: ReturnType<typeof React.createElement>, el: HTMLElement) => {
Expand Down
3 changes: 3 additions & 0 deletions npm/svelte/src/mount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ export function mount<T extends SvelteComponent> (
options: MountOptions<T> = {},
): Cypress.Chainable<MountReturn<T>> {
return cy.then(() => {
// Remove last mounted component if cy.mount is called more than once in a test
cleanup()

const target = getContainerEl()

injectStylesBeforeElement(options, document, target)
Expand Down
27 changes: 9 additions & 18 deletions npm/vue/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,24 +72,12 @@ type MountingOptions<Props, Data = {}> = Omit<VTUMountingOptions<Props, Data>, '

export type CyMountOptions<Props, Data = {}> = MountingOptions<Props, Data>

Cypress.on('run:start', () => {
// `mount` is designed to work with component testing only.
// it assumes ROOT_SELECTOR exists, which is not the case in e2e.
// if the user registers a custom command that imports `cypress/vue`,
// this event will be registered and cause an error when the user
// launches e2e (since it's common to use Cypress for both CT and E2E.
// https://github.com/cypress-io/cypress/issues/17438
if (Cypress.testingType !== 'component') {
return
}

Cypress.on('test:before:run', () => {
Cypress.vueWrapper?.unmount()
const el = getContainerEl()
const cleanup = () => {
Cypress.vueWrapper?.unmount()
Copy link
Contributor

Choose a reason for hiding this comment

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

If there's no Cypress.vueWrapper that would indicate it's the first mount, so possibly we just return here instead of touching the DOM?

Actually wondered, as part of cleanup do we want to unset Cypress.vueWrapper and Cypress.vue? Probably pointless since mount will overwrite them & cleanup is always called before mount.

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'm going to remove the innerHtml = '', more consistent with other frameworks. I'll add the cleanup as well.

const el = getContainerEl()

el.innerHTML = ''
})
})
el.innerHTML = ''
}

/**
* The types for mount have been copied directly from the VTU mount
Expand Down Expand Up @@ -378,6 +366,9 @@ export function mount<

// implementation
export function mount (componentOptions: any, options: any = {}) {
// Remove last mounted component if cy.mount is called more than once in a test
cleanup()

// TODO: get the real displayName and props from VTU shallowMount
const componentName = getComponentDisplayName(componentOptions)

Expand Down Expand Up @@ -484,4 +475,4 @@ export function mountCallback (
// import { registerCT } from 'cypress/<my-framework>'
// registerCT()
// Note: This would be a breaking change
setupHooks()
setupHooks(cleanup)
18 changes: 8 additions & 10 deletions npm/vue2/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
mount as testUtilsMount,
VueTestUtilsConfigOptions,
Wrapper,
enableAutoDestroy,
} from '@vue/test-utils'
import {
injectStylesBeforeElement,
Expand Down Expand Up @@ -266,6 +265,10 @@ declare global {
}
}

const cleanup = () => {
Cypress.vueWrapper?.destroy()
Copy link
Contributor

Choose a reason for hiding this comment

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

Any need to do innerHTML = '' for the root el here? Curious why Vue 3 does it but Vue 2 does not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

/**
* Direct Vue errors to the top error handler
* where they will fail Cypress test
Expand All @@ -280,14 +283,6 @@ function failTestOnVueError (err, vm, info) {
})
}

function registerAutoDestroy ($destroy: () => void) {
Cypress.on('test:before:run', () => {
$destroy()
})
}

enableAutoDestroy(registerAutoDestroy)

const injectStyles = (options: StyleOptions) => {
return injectStylesBeforeElement(options, document, getContainerEl())
}
Expand Down Expand Up @@ -336,6 +331,9 @@ export const mount = (
wrapper: Wrapper<Vue, Element>
component: Wrapper<Vue, Element>['vm']
}> => {
// Remove last mounted component if cy.mount is called more than once in a test
cleanup()

const options: Partial<MountOptions> = Cypress._.pick(
optionsOrProps,
defaultOptions,
Expand Down Expand Up @@ -442,4 +440,4 @@ export const mountCallback = (
// import { registerCT } from 'cypress/<my-framework>'
// registerCT()
// Note: This would be a breaking change
setupHooks()
setupHooks(cleanup)
2 changes: 1 addition & 1 deletion npm/webpack-dev-server/src/devServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ async function getPreset (devServerConfig: WebpackDevServerConfig): Promise<Opti
return { sourceWebpackModulesResult: sourceDefaultWebpackDependencies(devServerConfig) }

default:
throw new Error(`Unexpected framework ${(devServerConfig as any).framework}, please visit https://docs.cypress.io/guides/component-testing/component-framework-configuration to see a list of supported frameworks`)
throw new Error(`Unexpected framework ${(devServerConfig as any).framework}, please visit https://on.cypress.io/component-framework-configuration to see a list of supported frameworks`)
}
}

Expand Down
Loading