fix: Hovering over mount in command log does not show component in AUT #24346
Conversation
Adding commit from another branch to the target branch :
|
Thanks for taking the time to open a PR!
|
amehta265
requested review from
a team and
marktnoonan
and removed request for
marktnoonan
amehta265
changed the title
|
@amehta265 I'm testing this out and don't see the update on this Vue 2 / Nuxt project, wonder if it renders content later than initial mount? Haven't dug in yet. But that might be expected for Nuxt.
|
|
@marktnoonan how are you getting the changes to |
|
@astone123 I was using the e2e test that Ankit set up. I'll try as you suggested! |
|
OK that worked. And just building the npm/vue package fixed the example in the test. I'm going to take another look at the tests, as it probably shouldn't have been passing before I built npm/vue?
|
…nto issue-24138 merging with remote branch
npm/vue2/src/index.ts
Outdated
| @@ -410,6 +412,17 @@ export const mount = ( | |||
|
|
|||
| Cypress.vue = VTUWrapper.vm | |||
| Cypress.vueWrapper = VTUWrapper | |||
| }).wait(0, { log: false }).then(() => { | |||
| if (optionsOrProps.log !== false) { | |||
| const observer = new MutationObserver(() => callbackFunc) | |||
There was a problem hiding this comment.
Can you explain the benefit of using MutationObserver over something like Vue.nextTick here? The observer seems complex, and to add it I think we would need more safety around its use. For example, what happens if a component throws an error before rendering? Is the observer ever disconnected?
Same comment for React, I was a bit hesitant but we could use act to let the component stabilize.
There was a problem hiding this comment.
I believe the implementation using the MutationObserver here was to generalize the solution across different frameworks. As soon as we get the children for the data-cy-root, the observer is disconnected as you will see in the callbackFunc.
I can see why we can implement framework specific solutions because only vue2 and react are impacted so I'll definitely look into the methods mentioned.
There was a problem hiding this comment.
As discussed, I have used Vue.nextTick() in npm/vue2 to ensure the hover on mount command functionality for vue2 projects and have used wait(fn, 0) in npm/react
…nto issue-24138 merging with remote branch
npm/vue2/src/index.ts
Outdated
| name: 'mount', | ||
| message: [message], | ||
| }).snapshot('mounted').end() | ||
| Vue.nextTick(() => { |
There was a problem hiding this comment.
I'd expect the nextTick to be wrapped within the chain so as to ensure the tick and log is captured before the next Cypress command is fired. You could convert this function to be async and await Vue.nextTick() as the end.
There was a problem hiding this comment.
I am not sure we need async/await - could just do it like this, right?
.then((win) => Vue.nextTick())
.then(() => {
/* stuff */
})If anyone was curious, under-the-hood nextTick is basically just Promise.resolve (with various fallbacks). It actually uses MutationObserver as a fallback, interestingly enough.
There was a problem hiding this comment.
As long as the .then(() => Vue.nextTick()) is done after the component creation I'm cool with it!
There was a problem hiding this comment.
This sounds good. I've made the necessary changes on the latest commit
| { projectName: 'nuxtjs-vue2-configured', test: 'Tutorial.cy' }, | ||
| ] | ||
|
|
||
| for (const { projectName, test } of PROJECTS) { |
There was a problem hiding this comment.
Tests are very thorough, and I know this is already ✔️ , but I wonder if we want to consider adding these to an existing test in the future? Scaffolding all these projects is pretty expensive (in terms of CI time) for something that could be exercised in one of the other tests we've got for the various frameworks.
|
Looks good, but CI has some fails - might want to verify if those are real fails (and fix it) or flake (if so - probably want to make sure we aren't merging flaky tests). |
User facing changelog
Previously when hovering over
mountin the reporter of a component test, the AUT shows a blank white view instead of the mounted component. This is not ideal if the user wants to see the component that has been mounted by themountfunction.This PR ensures that the snapshot of the mounted component is taken at the current time thereby guaranteeing component being displayed in the AUT
Additional details
vue2andreact18frameworks. For all other frameworks the intended behavior was happening i.e. The mount command shows the mounted component in the AUT.React18uses themakeMountFnfromreactunder the hood therefore changes had to be made to themountfunction insidenpm/react/createMountfor this framework..waitand amutationObserver. Using the.wait(0, ...)allows cypress to wait until the next tick in the event-loop to take a snapshot of the mounted component. This ensures that all the synchronous code (including the actual mounting of the component) has occurred before we attempt to take a snapshot. ThemutationObserverlistens to the target node (in our case thedata-cy-root) to check if any element is added to it. If all the elements are added we can be sure that the component exists on the AUT and can thereby take the screenshot.Steps to test
The spec file
reporter-ct-mount-hover.cy.tsiterates over several projects fromsystem-tests, scaffolds, opens, and runs a spec file within each project while hovering on the mount function in the command log to ensure the intended functionality.packages/appyarn cypress:openreporter-ct-mount-hover.cy.tsspec file.How has the user experience changed?
Before:
After:
PR Tasks
cypress-documentation?type definitions?