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.

Sign up for GitHub

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

Jump to bottom

fix(@cypress/react): Devtools unpredictable resets #15612

Merged
merged 15 commits into from
Mar 25, 2021
Merged

fix(@cypress/react): Devtools unpredictable resets #15612

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
29b8f8f
fix: Devtools unpredictable resets
dmtrKovalenko Mar 22, 2021
46d53a2
Merge branch 'develop' into feat/react-devtools-resets
dmtrKovalenko Mar 22, 2021
6de75b9
Remove cleaning up from webpack-dev-server
dmtrKovalenko Mar 23, 2021
f38d40d
Merge branch 'develop' into feat/react-devtools-resets
dmtrKovalenko Mar 23, 2021
e220db0
Fix lint errors
dmtrKovalenko Mar 23, 2021
90512bf
Get back observer
dmtrKovalenko Mar 23, 2021
02f26e3
fix: bring back cleanup (#15634)
elevatebart Mar 24, 2021
643d83f
fix: wait for fw teardown to do html teardown
elevatebart Mar 24, 2021
7cd14ef
fix: port responsibility of teardown to frameworks
elevatebart Mar 24, 2021
b5be9d7
chore: add comments
elevatebart Mar 24, 2021
343e404
fix: typings
elevatebart Mar 24, 2021
0e750df
fix: react unmount cannot be called in the right hook
elevatebart Mar 24, 2021
d9e6ad4
Merge branch 'develop' into feat/react-devtools-resets
elevatebart Mar 24, 2021
8c5befe
run dtslint
elevatebart Mar 24, 2021
9c611c0
Merge branch 'develop' into feat/react-devtools-resets
elevatebart Mar 24, 2021
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
19 changes: 19 additions & 0 deletions npm/webpack-dev-server/src/aut-runner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
/* eslint-disable no-console */
/*eslint-env browser */

function removeAllChildrenButId (id: string) {
Array.from(document.body.children).forEach((child) => {
if (child.id !== id) {
document.body.removeChild(child)
}
})
}

function appendTargetIfNotExists (id: string, tag = 'div', parent = document.body) {
let node = document.getElementById(id)

Expand All @@ -21,11 +29,22 @@ export function init (importPromises, parent = (window.opener || window.parent))
Cypress.onSpecWindow(window, importPromises)
Cypress.action('app:window:before:load', window)

elevatebart marked this conversation as resolved.
Show resolved Hide resolved
// In this variable, we save head
// innerHTML to account for loader installed styles
let headInnerHTML = ''

// before the run starts save
Cypress.on('run:start', () => {
headInnerHTML = document.head.innerHTML
})

// Before all tests we are mounting the root element, **not beforeEach**
// Cleaning up platform between tests is the responsibility of the specific adapter
// because unmounting react/vue component should be done using specific framework API
// (for devtools and to get rid of global event listeners from previous tests.)
Cypress.on('test:before:run', () => {
document.head.innerHTML = headInnerHTML
Copy link
Contributor Author

@dmtrKovalenko dmtrKovalenko Mar 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, @elevatebart I appreciate your help, but I still don't understand why this should be a part of webpack-dev-server. It looks like not the job of the dev server at all.

I understand that turning this on requires additional import and we can make a specific entry point for this. Ideally, it should be something like:

// support.js file
import { cleanup } from '@cypress/ct-core'

beforeEach(() => { 
  cleanup()
}) 
// and in @cypress{react/vue} we can do
import { cleanup } from '@cypress/ct-core'

beforeEach(() => {
  framworkSpecificUnmount()
  cleanup()
}

It's something @JessicaSachs was talking about for a long time

If somebody is going to not use our framework – let him clean up his code on his own. It just looks like trying to solve a problem that does not exist.

For reference: the testing library provides auto cleanup just like we do, but if you do can also use it manually I think for tests like this

https://testing-library.com/docs/svelte-testing-library/api/#cleanup

removeAllChildrenButId('__cy_root')
appendTargetIfNotExists('__cy_root')
elevatebart marked this conversation as resolved.
Show resolved Hide resolved
})

Expand Down