-
Notifications
You must be signed in to change notification settings - Fork 668
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(types): add typescript declarations #15
Conversation
Rethinking about |
types/index.d.ts
Outdated
readonly element: HTMLElement | ||
readonly options: WrapperOptions | ||
|
||
find<R extends Vue> (selector: Selector): Wrapper<R> |
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.
We can separate find
to more overloadings. Thus for VClass and CompoentOption there is no need to manually cat.
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.
Thanks for pointing this out.
By the way, I realized a type parameter could not be inferred from other type parameter so we have to manually set a component type if we use component constructors. Does this impossible until microsoft/TypeScript#14400 is solved, right?
// find<R extends Vue, Ctor extends VueClass<R> = VueClass<R>> (selector: Ctor): Wrapper<R>
// `find`'s type parameters will be `<Vue, typeof ClassComponent>`
const found = wrapper.find(ClassComponent)
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.
Exactly. VueJS really needs it.
attachedToDocument: boolean | ||
} | ||
|
||
interface MountOptions<V extends Vue> extends ComponentOptions<V> { |
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 wonder the status of vuejs/vue#5887.
That PR will break this declaration. But 5887 does not seem to have activities...
@ktsn @yyx990803 What's your opinion?
I can change 5887 to have a compatible version by changing ComponentOptions, though. @DanielRosenwasser
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 think vuejs/vue#5887 will not be merged soon since we need to have some migration time for the new declaration. So we can use the current declaration for now.
interface MountOptions<V extends Vue> extends ComponentOptions<V> { | ||
attachToDocument?: boolean | ||
clone?: boolean | ||
context?: VNodeData |
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 is only used by FunctionalComponent, can we make two separate options for different usage?
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 thought about that but I ended up to choose type declaration simplicity. Also I have no idea to declare if a constructor of functional component is passed to mount
/shallow
🤔
types/index.d.ts
Outdated
* If it is an array of string, the specified children are replaced by blank components | ||
*/ | ||
type Stubs = { | ||
[key: string]: Component | string | boolean |
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.
[key: string]: Component | string | true
https://github.com/vuejs/vue-test-utils/blob/master/src/lib/stub-components.js#L28-L31
types/index.d.ts
Outdated
localVue?: typeof Vue | ||
intercept?: object | ||
slots?: Slots | ||
stub?: Stubs |
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 wonder why this API is called stub
rather than stubs
, since it can stub multiple components? @eddyerburgh
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 think you're right, stubs is more appropriate 👍
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.
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.
Updated 👍
clone?: boolean | ||
context?: VNodeData | ||
localVue?: typeof Vue | ||
intercept?: object |
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.
@eddyerburgh I wonder why intercept is by default global.
https://github.com/vuejs/vue-test-utils/blob/master/docs/en/api/mount.md
vue-test-utils/src/lib/create-instance.js
Line 40 in a2137fa
if (options.intercept) { |
I know this option is used to be global
. But since it is changed its, it seems better to modify localVue by default. Since one mount returns one wrapper, it feels more natural that intercept only modify the wrapper.
If users want to modify Vue's prototype globally, we can, if strongly desired, provide another global API like mount. But fairly users can just Vue.install
by themselves.
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.
It doesn't add them globally, it's just a poorly named function. I'll refactor to make that clear
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've refactored in 086f8ae
@ktsn and @HerringtonDarkholme are heros, just sayin' |
This PR adds TypeScript declarations of vue-test-utils.
I wrote these declarations by reading the current docs and implementation but not sure that the declaration is correct. So please let me know if somethings are wrong 😉
It would be easier to confirm by seeing test files under
types/test
. They include actual usage of vue-test-utils API in TypeScript.@HerringtonDarkholme It would be appreciated if you can review this PR 🙂
Directory structure
types/
index.d.ts
All declarations of vue-test-utils are in this file. This should be published in npm.tsconfig.json
TS compiler options are defined in this filetest/
includes test files for typingsAllow annotating type for
mount
/shallow
One of challenge when using vue-test-utils would be how to declare
wrapper.vm
to an actual component type. For example, if we simply declare it asVue
, we cannot test component properties/methods since they are not declared inVue
.To solve this, I declared
mount
/shallow
type that can be receive a component type via generics.Added npm script to test typings
I've added
npm run test:types
to run the typings test. It also be included innpm run test
.