feat: Explain how to replace attrs with props

[dobromir-hristov]

This is pretty self explanatory but I wanted to make sure it works.

[afontcu]

Nice clarification! I love when tests work as an additional documentation.

[afontcu]

Suggested change

	template: '<button @click="$emit(\'customEvent\', true)">Click</button>'
	template: `<button @click="$emit('customEvent', true)">Click</button>`

[cexbrayat]

I think this is pretty clear, however I think it breaks the dev experience a little bit.

Let me explain: to support this use-case, I had to relax the props mounting option, that's why its type is PublicProps extends TestedComponent['$props'] and not TestedComponent['$props'].

So the current typings catch mount(Comp, { props: { a: badType}}). But we could only allow props to be TestedComponent['$props'] and we would also catch mount(Comp, { props: { badProp: 1}}). It would also allow developers to have a better autocompletion as the keys of props would only be the possible props, whereas it currently accept everything. It would also forbid to pass rpops to a component that declared it has none.

The downside would be to re-introduce the attrs back, to allow for setting the attributes, but IMHO it makes more sense than throwing everything in props without type safety.

WDYT?

[lmiller1990]

I did not realize this would impact the type safety. Hm. I didn't look too deeply into this PR since it was just adding what seemed to be basic tests.

So you are saying that having the attributes function on VueWrapper makes it so we can't have proper type-safety on the props mounting option? Since attrs and props get mixed together?

See what @dobromir-hristov says, since he added this test and may have specific use case.

PS: I do not fully understand the down side - if we do TestedComponent['$props'], what exactly do we lose out on?

[cexbrayat]

Sorry if this wasn't clear.

We have two options.

1. Mix attrs and props together in props, and add the extra props as attributes. this is what is currently done. So the types of props in MountingOptions can't be TestedComponent['$props'] because that would not allow props that aren't declared in the component (so no possibility of declaring extra props and have them as attributes). That's why the type of props currently extends TestedComponent['$props']

const Comp = defineComponent({ props: { a: String }});
mount(Comp, { props: { a: 'hello' }}) //OK
mount(Comp, { props: { a: 2 }}) // does not compile
mount(Comp, { props: { b: 2 }}) // OK, and b becomes an attribute
// but we don't know if the user _really_ meant b, or just meant a but made a mistake

2. separate props and attrs. this allow to catch potential error as we can strictly type props to be just the declared props in the component. And attrs can be whatever:

const Comp = defineComponent({ props: { a: String }});
mount(Comp, { props: { a: 'hello' }}) //OK
mount(Comp, { props: { a: 2 }}) // does not compile
mount(Comp, { props: { b: 2 }}) // does not compile
mount(Comp, { attrs: { b: 2 }}) // OK, and b becomes an attribute
// no mistake possible, type safety, autocompletion

When I wrote #74 I already thought aboutf that, but wanted to keep things readable and to talk with the team about the possibility to re-introduce attrs.

[dobromir-hristov]

I just did what Vue does. I don't use TS, so to me it was never a problem.

You can get attrs back, you just need to merge it into props. It will diverge from the render api, but I don't think people care.

As I said, I dont have an opinion on this.

[afontcu]

I do not have a strong opinion either, and to be honest I didn't even think that merging props and attrs would be detrimental for typing – which makes total sense now that you have exposed it. I'm ok with splitting them if the API stays quite simple and we benefit autocompletion and type safety 👍

[lmiller1990]

I did not even realize we had an attrs mounting option despite working on this lib for years. That is why I was confused.

I think it's fine to split them out. @cexbrayat are you interested in making this change?

[cexbrayat]

Sure I'll take a look and open a PR. I'm not sure this is way better, but in my experience, it always pays to have more type-safety. It's not always a pleasure to fight with vue typings but I'll see what I can come up with ;)

[cexbrayat]

So I tried something in #99

Sadly, I had not realized that for component declared with definedComponent({ props: {a : String}}), TestedComponent['$props'] type is { a: string } & VNodeProps. And VNodeProps allows any property by its definition, so I don't think we can be as strict as I hoped.
That might still be a bit better, but I'll let you judge: I don't mind if you prefer sticking with the current option.