-
-
Notifications
You must be signed in to change notification settings - Fork 681
'Strict' option for vue/no-unused-properties #1636
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
Comments
I don't like the option to change the way it works, but I don't reject to add the option to change the way it works. However, the name of the option can be confusing. |
I'm not precious about the name of the option—
|
There are some patterns that this rule considers to have used all properties. e.g. <script>
/* eslint vue/no-unused-properties: [2, {groups:['data']}] */
export default {
data () {
return {
unusedData: 42
}
},
methods: {
fn () {
return this
}
}
}
</script> I think we need to allow the user to set whether to consider each as using all properties or not. The option we add is an option that controls the decision to access an unknown property, so I want the option name to be descriptive. |
I'm not sure I completely follow what you were trying to illustrate there—did you mean this new setting should be per group? If so, how about |
Although, I'm now not seeing the point of including |
I wanted to say that the last example I wrote would not be warned by You can try it DEMO. |
For example, if the option name was |
When you say ”how to handle I still argue that warning about unused properties according to the existing settings should not be affected by the code containing a line like |
I was misunderstanding your option suggestions, but I probably understood what you were saying. The current behavior is implemented to be false negatives to avoid false positives as much as possible when the property usage becomes untraceable. I think it's a user's preference how to handle some of these processes, so I think it should be possible to set each one. For example, how about adding the following option to control known processes? {
"vue/no-unused-properties": ["error", {
"stopReporting": [ // To avoid false positives, stop reporting if the specified process is found.
"unknownPropertyAccess", // e.g. this[unknown]
"returnInstance", // e.g. return this
// There may be other than these.
]
}]
} I think the settings you want are: {
"vue/no-unused-properties": ["error", {
"stopReporting": [] // Probably "strict" for you as it doesn't stop the reporting.
}]
} What do you think? (I'm not good at English, so option names can still be confusing to people 😓) |
If anyone wants to work on this change, open a PR. |
Right, so there would be a default value of I don't think I know the internals well enough to start on this, so hopefully some kind soul will jump in :). |
Ref vuejs#1636, this is my first stab at adding an option to not treat generic access to `this` as 'using' properties / data / computed, etc. If this is the right approach, I can try adding tests if needed. It's more general I think than what you'd proposed in that issue, but for my use case it works well.
What rule do you want to change?
vue/no-unused-properties
Does this change cause the rule to produce more or fewer warnings?
More when
strict: true
How will the change be implemented? (New option, new default behavior, etc.)?
I have argued that this should be the default behaviour, in line with externally-called methods being considered 'unused' unless explicitly labelled in JSDoc style. But, if this change is undesirable, a
strict: true
option to vue/no-unused-properties might be a compromise, with the possibility of makingstrict: true
the default in future.Please provide some example code that this change will affect:
What does the rule currently do for this code?
The presence of
this[handlerName]
(as noted in the original ticket) makes it almost impossible for static analysis to determine whether any particular method or property is actually used. The current behaviour appears to assume that all properties, methods, data, and computeds are therefore used, and the above code produces no warnings at all. Commenting outthis[handlerName]
restores the (in my opinion, correct) behaviour that only explicitly referenced properties are 'used'.What will the rule do after it's changed?
Warn about any properties not explicitly referenced, or which are labelled as referenced by the developer using either
// eslint-disable-next-line vue/no-unused-properties
or/** @public */
annotations.The text was updated successfully, but these errors were encountered: