types: fix mount infer prop type #274
Conversation
There was a problem hiding this comment.
LGTM.
I liked not being able to pass extra props, because it catches error like mount(Foo, { typo: value}) if the props is type. But I understand the reasoning.
It would be aswome to squash the 2 commits into one to have a clearer history. And maybe remove the commented tests, or even better, fix them instead of commenting them (because we won't remember why they are commented :) ).
test-dts/shallowMount.d-test.ts
Outdated
| props: { a: 'Hello', c: 2 } | ||
| }) | ||
| ) | ||
| // // can not receive extra props |
There was a problem hiding this comment.
How come we don't need these anymore? Shouldn't these continue to error out if we infer the props?
There was a problem hiding this comment.
This will make the passing props a bit more relaxed, is also the same behaviour as h#vue-next
There was a problem hiding this comment.
I think it would be better to uncomment the test and remove the expectError then, with a comment explaining that extra props is allowed as it is for h(). I don't like having commented code, because we will forget why ^^. And if the tests check that extra props are allowed, we are sure to avoid regressions in the future.
|
@cexbrayat any more comments or can you drop a ✅ |
|
Made the changes, also added the //https://github.com/vuejs/vue-next/blob/master/packages/runtime-core/src/vnode.ts#L87-L98
export type VNodeProps = {
key?: string | number
ref?: VNodeRef
// vnode hooks
onVnodeBeforeMount?: VNodeMountHook | VNodeMountHook[]
onVnodeMounted?: VNodeMountHook | VNodeMountHook[]
onVnodeBeforeUpdate?: VNodeUpdateHook | VNodeUpdateHook[]
onVnodeUpdated?: VNodeUpdateHook | VNodeUpdateHook[]
onVnodeBeforeUnmount?: VNodeMountHook | VNodeMountHook[]
onVnodeUnmounted?: VNodeMountHook | VNodeMountHook[]
} |
There was a problem hiding this comment.
LGTM, thanks @pikax !
@lmiller1990 It would be awesome to have a release with this change so we can check if everything is fine in our respective projects 😃
fix #237
Fix issue of not making default properties as optional.
Allow to pass extra props (same behaviour as
h)Commented on tests which prevented extra props.