-
Notifications
You must be signed in to change notification settings - Fork 246
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: integrated vue-component-type-helpers
#2026
Conversation
✅ Deploy Preview for vue-test-utils-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
I have removed node 14 test in this PR, because test failed, and vue-tsc is also given up node 14 after v.1.3 (vuejs/language-tools#2489). |
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.
Wow, great. Any recommendation for the release notes? Eg "Improve slot type inference when using TypeScript" would be correct?
Looks like this vastly simplifies the types, thanks!
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.
I like it. Good work 👍
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.
@johnsoncodehk Thanks for working on this
@pikax If you have some time Carlos, it would be great to have your input as you worked on the original types |
@lmiller1990 As this has the potential to be fairly breaking (I hope not), we should cut an alpha release for this to give it a try when merged |
Hmm good point on the breaking change - it would be really ideal not to. Test Utils has never had a breaking change - and if it's just type definitions, this doesn't feel like a very compelling reason to break that pattern. I hope we can reassemble a kind of |
Also merged the remaining overloads, since this is a breaking change anyway. Compared with the original PR, the change may be quite radical now, if this does not meet expectations, please feel free to revert |
Thanks @johnsoncodehk - please give us a bit to figure out how to handle a breaking change. We haven't had one before, so we don't have a process around it, but we can do so if it'll improve the library for everyone. |
Changes look good to me. |
@pikax thanks for reporting that, can you try this latest commit? Since I haven't worked on this repo in the past, these modifications may be rather naive, please feel free to edit the PR if found something else breaks. |
@johnsoncodehk I can confirm it's fixed, LGTM |
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 👍
Use
vue-component-type-helpers
instead of infer component props / slots fromDefineComponent
.vue-component-type-helpers
is simple in itself, but it behaves in line withvue-component-meta
.Fixes #1973