vuejs / vetur

Vue tooling for VS Code.
https://vuejs.github.io/vetur/
MIT License
5.75k stars 593 forks source link

Prop validation treats optional props as required #3084

Open last-partizan opened 3 years ago

last-partizan commented 3 years ago

Info

Problem

When using following prop definition propName: String, vetur considers it required. But for vue it's optional.

image

https://github.com/vuejs/vetur/blob/master/server/src/modes/script/componentInfo.ts#L371 I think problem is here. And i can try to fix it myself and make PR, if this is really a bug and not desired behaviour. For me it looks like a bug.

Reproducible Case

https://github.com/last-partizan/vetur-props-validation-bug

This is Test.vue ```vue ``` This is App.vue ```vue ``` This is vetur warning: ` misses props: foo`
yooouuri commented 3 years ago

Same here:

export default defineComponent({
 props: {
    noCloseOnBackdrop: {
      type: Boolean,
      required: false
    }
  }
})

image

It looks like this had to do with noCloseOnBackdrop is in camelCase and no-close-on-backdorp is used...

last-partizan commented 3 years ago

@yooouuri that's strange.

my minimal example

export default {
  props: {
    foo: String,
    bar: {type: String, required: false},
    camelCaseBar: {type: String, required: false},
  }
}

triggers warning only for foo prop. Could you check my repo with Reproducible Case?

yoyo930021 commented 3 years ago

https://vuejs.github.io/vetur/guide/interpolation.html#prop-validation

https://github.com/vuejs/vetur/issues/2141#issuecomment-674117099

It is by design.

last-partizan commented 3 years ago

Hmm, why such design decision?

For me it looks counter-intuitive. Props are optional, but vetur requires me to pass them.

I understand, it's better to explicitly set required: true or false, but that's job for eslint. It already has vue/require-prop-types for catching props: ["foo"]. And, if implemented this way - will show warnings only in relevant place (component itself, not everywhere where it's used).

yoyo930021 commented 3 years ago

Please set "vetur.validation.templateProps": false when you don't need it.

Another information: https://github.com/vuejs/vetur/issues/2235#issuecomment-688013007

last-partizan commented 3 years ago

Oh, i see people having the same problem as me.

And i see

But I think this case might be taken as required: false.
I will talk to @octref.

But looks like nothing was changed?

I think this feature would be useful to more people, when it works as expected. And expected way - is to treat optional props as optional.

I would like to use it, but in current state it gives more trouble that benefits.

I know it's opensource project, and i don't want to bother you with my wishes. I'm offering help, i could implement fixes needed (maybe with some guiding).

But we need come to conclusion about how this should work, so everyone could benefit from it.

https://github.com/vuejs/vetur/issues/2235#issuecomment-818990826

Here is good point, i totally agree with it, but if we need to preserve current behaviuor, i could offer following chages:

Create new option: "vetur.validation.templatePropsValidationLevel": "standard" | "strict".

@yoyo930021 what do you think?

Maybe @octref has something to say about this?

yoyo930021 commented 3 years ago

Oh, i see people having the same problem as me.

And i see

But I think this case might be taken as required: false. I will talk to @octref.

But looks like nothing was changed?

I think this feature would be useful to more people, when it works as expected. And expected way - is to treat optional props as optional.

I would like to use it, but in current state it gives more trouble that benefits.

I know it's opensource project, and i don't want to bother you with my wishes. I'm offering help, i could implement fixes needed (maybe with some guiding).

But we need come to conclusion about how this should work, so everyone could benefit from it.

#2235 (comment)

Here is good point, i totally agree with it, but if we need to preserve current behaviuor, i could offer following chages:

Create new option: "vetur.validation.templatePropsValidationLevel": "standard" | "strict".

* `standard` validation behaves as everyone expects - errors when required props are missing

* `strict` - same as above, + warning about missing optional (but without explicit `required` field). Current behaviour.

@yoyo930021 what do you think?

Maybe @octref has something to say about this?

I respect @octref This feature is design by him.

vis97c commented 2 years ago

Any update on this?

marcomilone commented 2 years ago

I'm popping here to tell you that IMHO is a wrong design decision.

Regards,

☮😀