-
Notifications
You must be signed in to change notification settings - Fork 257
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(stubs): Allow to stub directives (fixes #1800) #1804
Conversation
✅ Deploy Preview for vue-test-utils-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
* this adds stubbing directives functionality to @vue/test-utils
}, | ||
template: '<div v-my-directive>text</div>', | ||
directives: { MyDirective } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
naive question: shouldn't that use a "real" script setup
SFC like other tests do? That would seem more bulletproof and close to reality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Why not, I have no strong opinion on this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💣 Actually this one demonstrated that directives stubbing won't work with <script setup>
. I will probably go ahead and create an issue for <script setup>
compiler (I believe we could easily do that there), but for now I just capture current state of the things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this test should be updated to reflect that it does not currently work right?
And ideally, reference the issue you'll create for the compiler, so we can fix it when possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cexbrayat nope. { setup() ... { ... }}
will still work, <script setup>
will not. I've added link to vuejs/core#6887 with my proposal to docs to reflect that
* fix wording * fix tests * improve types
}, | ||
template: '<div v-my-directive>text</div>', | ||
directives: { MyDirective } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this test should be updated to reflect that it does not currently work right?
And ideally, reference the issue you'll create for the compiler, so we can fix it when possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Ideally I'd like another pair of eyes on this before merging it
@freakzlike @lmiller1990 would you mind taking a look when you have some time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good to me!
We're adding new vnode transformer (related to directives) and re-use our architecture for searching stubs to make sure that we still could find elements even after stubbing directives on them