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

feat: include component and wrapper in return type for vue mount adapter #24479

Merged

Conversation

lmiller1990
Copy link
Contributor

User facing changelog

Vue Mount adapter now returns an object with both the VueWrapper (wrapper) and component instance (component).

Additional details

Part of CT GA.

Steps to test

You could do const foo = cy.mount(SomeVueComponent) then observe both wrapper and component properties are available on the returned value. wrapper contains all of these methods from Vue Test Utils. Component contains various Vue related things.

How has the user experience changed?

You can access the component instance, which is more in line with what other mount adapters do.

PR Tasks

  • Have tests been added/updated?
  • Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • Has a PR for user-facing changes been opened in cypress-documentation? NOT YET
  • Have API changes been updated in the type definitions?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 1, 2022

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Nov 1, 2022



Test summary

9008 6 1476 0Flakiness 7


Run details

Project cypress
Status Failed
Commit 856f808
Started Nov 2, 2022 7:25 AM
Ended Nov 2, 2022 7:41 AM
Duration 16:26 💡
OS Linux Debian -
Browser Multiple

View run in Cypress Dashboard ➡️


Failures

cypress/e2e/commands/navigation.cy.js Failed
1 src/cy/commands/navigation > #visit > should eventually fail on assertion despite redirects
2 src/cy/commands/navigation > #page loading > emits 'page:loading' before and after initial visit
3 src/cy/commands/navigation > #page loading > emits during page navigation
4 src/cy/commands/navigation > #page loading > logs during page navigation
5 src/cy/commands/navigation > #page loading > logs during form submission and yields stale element
6 src/cy/commands/navigation > #form sub > logs 'form sub'

Flakiness

e2e/origin/cookie_behavior.cy.ts Flakiness
1 ... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
2 ... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)
3 ... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
4 ... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)
commands/xhr.cy.js Flakiness
1 src/cy/commands/xhr > abort > aborts xhrs currently in flight
This comment includes only the first 5 flaky tests. See all 7 flaky tests in the Cypress Dashboard.

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

@lmiller1990 lmiller1990 marked this pull request as ready for review November 1, 2022 03:04
@lmiller1990 lmiller1990 changed the base branch from develop to release/11.0.0 November 1, 2022 06:18
Copy link
Contributor

@ZachJW34 ZachJW34 left a comment

Choose a reason for hiding this comment

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

Couple small comments but looks and works great, will approve but would like to see comments addressed before merge

npm/vue2/src/index.ts Outdated Show resolved Hide resolved
npm/vue/src/index.ts Show resolved Hide resolved
@lmiller1990
Copy link
Contributor Author

Webkit system tests 🤷‍♂️

Everything else looks 💯

Copy link
Contributor

@marktnoonan marktnoonan left a comment

Choose a reason for hiding this comment

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

Works great, chaining off the mount to get the wrapper works nice, putting the component instance in an alias and grabbing it there works nicely too. Probably no need for users to rely on Cypress.vueWrapper any more which will be sweet.

@ZachJW34 ZachJW34 merged commit 33875d7 into release/11.0.0 Nov 2, 2022
@ZachJW34 ZachJW34 deleted the lmiller/issue-24342-update-vue-wrapper-return-type branch November 2, 2022 14:32
Copy link
Contributor

@warrensplayer warrensplayer left a comment

Choose a reason for hiding this comment

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

This PR already got merged while I was in the middle of reviewing, but will drop these comments here for future consideration.

Comment on lines 430 to 431
// by returning undefined we keep the previous subject
// which is the mounted component
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment no longer applicable

Comment on lines +132 to +135
): Cypress.Chainable<{
wrapper: VueWrapper<ComponentPublicInstance<V>>
component: VueWrapper<ComponentPublicInstance<V>>['vm']
}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Still working on my Typescript, but would it make sense to introduce a generic return type like:

type MountReturn<C> = {
  wrapper: VueWrapper<C>
  component: C
}

to use for the return of all the mount overloads?

Result

export function mount<V extends {}>(
  originalComponent: {
    new (...args: any[]): V
    __vccOpts: any
  },
  options?: MountingOptions<any> & Record<string, any>
): Cypress.Chainable<MountReturn<V>>

Would that work for all the overload cases?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cypress/vue should return object containing vueWrapper and component
4 participants