fix(component-testing): Correctly unmount react components #15250
Conversation
|
Thanks for taking the time to open a PR!
|
| // 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.) |
There was a problem hiding this comment.
The comment is a little confusing to me - Just for my understanding, unmounting the component won't actually get rid of the global listener (REACT_DEVTOOLS_GLOBAL_HOOK) though, right? It will just make sure the component is no longer present in the devtools.
There was a problem hiding this comment.
I mean if you are doing document.addEventListener inside the useEffect and unsubscribing in return
There was a problem hiding this comment.
Right, sure. Makes sense. One thing we might need to be careful is if the devtools do window.addEventListener('postMessage'). Vue DevTools do this - we need to clean it up because window.addEventListener` will not replace the previous one but duplicate it.
| @@ -16,22 +16,6 @@ export function setupHooks (rootId: string) { | |||
| } | |||
There was a problem hiding this comment.
Though this is not part of the fix, this clean-up is a very good idea.
I am going to check for the same issues in the vue side.
| // 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.) | ||
| before(() => { | ||
| appendTargetIfNotExists('__cy_root') |
There was a problem hiding this comment.
We probably need to fix vue with using this root too. Right now, @cypress/vue's mount function creates it's very own root.
There was a problem hiding this comment.
I did change this so it is matching the old react behavior (that I deleted here) – but yes, I do want to test this in scope of devtools PR
This PR fixes how we unmounting and clearing root elements before the test. Cleaning up the platform between tests is the responsibility of the specific adapter because unmounting react/vue component should be done using a specific framework API (for better dev tools integration and to get rid of global event listeners from previous tests.) Also, this matches the real lifecycle of components in the user applications.
This fixes duplicated root elements for react because when we are doing unmount react cleans up information in devtools as well. The same approach should work for vue using
$destroy.