-
-
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
Add rule vue/require-valid-default-prop
.
#119
Add rule vue/require-valid-default-prop
.
#119
Conversation
7e4c894
to
dd69ffc
Compare
vue/require-valid-default-prop
.vue/require-valid-default-prop
.
vue/require-valid-default-prop
.vue/require-valid-default-prop
.
21e7fc5
to
c4e7cc3
Compare
vue/require-valid-default-prop
.vue/require-valid-default-prop
.
@@ -0,0 +1,64 @@ | |||
# Enforces prop default values to be valid (require-valid-default-prop) | |||
|
|||
This rule is doing basic type checking between type and default value and inform about missuesed or invalid default values. |
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.
What do you think about:
This rule checks whether the default value of each prop is valid for the given type. It should report an error when default value for type `Array` or `Object` is not returned using function.
@@ -0,0 +1,64 @@ | |||
# Enforces prop default values to be valid (require-valid-default-prop) |
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.
props'
} | ||
} else if (node.type === 'ArrayExpression') { // Array | ||
return 'Array' | ||
} else if (node.type === 'ObjectExpression') { // Array |
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.
// Object
|
||
context.report({ | ||
node: def, | ||
message: "Type of default value prop '{{name}}' must be a {{types}}.", |
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.
Maybe: Type of the default value for '{{name}}' prop must be a {{types}}.
?
I think it sounds nice: Type of the default value for 'counter' prop must be a number.
props: { | ||
foo: { | ||
type: Array, | ||
default: [] |
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.
Did you think about supporting also:
{
type: Array,
default() {
return 100
}
}
? It should also throw an error I think
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.
for now i'm not checking what function is returning or even if function is returning something
{
type: Number,
default() { }
}
is valid
props: { | ||
foo: null, | ||
foo: Number, | ||
foo: [String, Number], |
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.
What if I use it like this:
foo: {
type: [Object, Number],
default: 10
},
bar: {
type: [Object, Number],
default: {}
}
? Will it throw an error?
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'm missing tests for that, i will add them
@michalsnik i added few more tests and i applied you suggestions about doc/message I'm treating function as always valid due to complexity of detecting type returned by it |
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'm fine with that for now :) LGTM 👍
* master: Add rule `vue/require-valid-default-prop`. (vuejs#119) 3.10.0 Update readme to 3.10.0 Chore: remove package-lock.json (vuejs#128) Fix: parserService must exist always (fixes vuejs#125) (vuejs#127) Add rule `require-render-return`. (vuejs#114) 3.9.0 Update package-lock Update: options for `no-duplicate-attributes` (fixes vuejs#112)(vuejs#113) New: add rule `attribute-hyphenation`. (fixes vuejs#92)(vuejs#95) Add namespace check of svg & mathML instead of tag names (vuejs#120)⚠️ Add support for deprecated state in update-rules⚠️ (vuejs#121) Add rules: `no-dupe-keys` and `no-reserved-keys`. (vuejs#88) Chore: Improve tests for name-property-casing & improve documentation (vuejs#115) New: add `require-prop-types` rule (fixes vuejs#19)(vuejs#85) Update: upgrade vue-eslint-parser (fixes vuejs#36, fixes vuejs#56, fixes vuejs#96) (vuejs#116)
This PR implements rules proposed in #117
TODO:
fixes #117