fix(@cypress/react): Devtools unpredictable resets #15612
Conversation
|
Thanks for taking the time to open a PR!
|
|
Test summary
Run details
View run in Cypress Dashboard ➡️ Flakiness
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 |
||||||||||||||||||||||||||
|
I confirmed you can open/close the devtools many times without problem. I see we will have the bug where it renders a
|
There was a problem hiding this comment.
The fix seems fine, I found another bug (perhaps existing, perhaps regression) and added a comment.
I'll leave fixing and/or merging up to @dmtrKovalenko. For now ✔️ for the fix.
|
Thanks 🙏 for finding a regression. |
|
This issue reproducing again in |
|
Hey, @elevatebart I removed a bunch of code from your PR #15275 because it was a reason of duplicated components in devtools. Let me know if I broke something |
|
@dmtrKovalenko Seems that removing some clean up breaks a spec in Vue where it wants to reset the state each test. I am not sure how important this test actually is, though. Can give it another test when you are happy with it, pls let me know. |
| // 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 |
There was a problem hiding this comment.
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
This PR reduces unpredictable resets of the dev tools plugin. The main problem for our UI plugin architecture right now is our react components structure which sometimes recreates the whole subtree (of react-split-pane) and the root element of devtools gets recreated and replaced with a completely new one – so we see it empty.
Devtools will flush also if the bridge is stale, but this happens only if we are not touching nor dev tools nor react app for some period of time, so I don't think this a problem.