feat: split attrs and props mounting options

[cexbrayat]

This allows a slightly better type checking, even if defineComponent({ props: { a: String }}) sadly has $props typed as { a: string } & VNodeProps and VNodeProps allows anything. I'm not sure we can do much on VTU side for now.

See #90 for context

[pikax]

Can we have a test with attrs?

[dobromir-hristov]

I think mentally, users are probably used to having props and attrs separate. Most users would not know that for render funcs its the same prop.

I dont mind, the API change is small, feels clean and looks like it cleans up code as well :)

[lmiller1990]

This seems fine to me (I didn't even know about the attrs API until recently - I never use that). This is also closer to the VTU for Vue 2 API, where we had both props and attrs, right?

Agree we should add a test for attrs. This would go in a new file, since each wrapper API has it's own file, and make it easy to find the relevant tests.

[lmiller1990]

We could use this:

declare module '*.vue' {
  import { ComponentOptions } from 'vue'
  var component: ComponentOptions
  export default component
}

[lmiller1990]

Code seems fine, can we have a few simple tests in a mountingOptions/attrs.spec.ts file

[cexbrayat]

I pushed a few simple tests in a new commit and rebased on latest master. I kept the test in props.spec.ts that uses extra props as the behavior still exist, even if attrs should be preferred (until we find a way to properly type mount and forbid the "extra props as attrs" behavior).

[dobromir-hristov]

This is technically the right way to attach a listener, not through attrs, even though they are identical as of now.

[cexbrayat]

So your point is that we should keep the possibility of assigning extra props for this use-case?
There are several options:

• do not allow it as it is in this PR, forcing the user to cast the props as any in the test, if he/she really wants to use props. I can change the test with a cast to let it in the current file.
• allow it, but we lose a it of type safety as we can't make a difference between a badly written prop and an extra prop
• allow it via some other possibility (mountingProps.extraProps ?) as it is not the most common use-case
  What do you think would be the most sound strategy?

[dobromir-hristov]

So extra props would be attrs basically? If so, then I say just mention in the docs that people can use attrsfor that.

[cexbrayat]

Yes, we can nudge the developers in the direction we feel makes more sense in the docs, either:

• use attrs for extra props
• or use props but cast with as any to indicate this is intentional.

[dobromir-hristov]

Would this error out for transparent components without props defined?

[cexbrayat]

Can you give me an example of what you mean?

[dobromir-hristov]

If a component has no props defined, and passes them down via $attrs.

const Component = {
   template: `<div><ChildComponent v-bind="$attrs"/></div>`
}
const wrapper = mount(Component, { props: { a:'a' } })

expect(wrapper.findComponent(ChildComponent).props('a')).toBe('a')

[cexbrayat]

Ok, so yes: I expect this to error out if the props are not defined (as they end up with the never type). I think it makes sense to error out, as it is not the most common case, and let the developer use mount(Component, { props: { a:'a' } as any }) in that case. But I'm open to your suggestions if you think this is too constraint.

[dobromir-hristov]

This wont error out for plain JS use right?

Erroring out is technically wrong, as the Vue API allows you to pass props to that kind of components, but as long as we state the reasoning behind this, should be fine :)

You are right, its not a common use case and having type safety for the 80% of people is better, we just need to make sure these little cases have workarounds for the more novice TS users out there.

[cexbrayat]

Yes this is for TS developers only, JS will be fine, sorry if that wasn't clear :)

And even in TS it still works, it just forces the developers to cast the props if they want to use it with undeclared props.

IMO, TS developers will be happy to have intellisense/autocomplete/type-checking in tests, and won't be too lost if the compilation indicates that the props does not match the props of the component. But you're 100% right that we need to mention it the docs 👍

[cexbrayat]

@dobromir-hristov I pushed a new commit to reflect our discussion with an example of an explicit cast: it won't get lost in the Github comments ;)