-
-
Notifications
You must be signed in to change notification settings - Fork 679
⭐️New: Add rule no-template-shadow
.
#158
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 o 10000 ur terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
no-template-shadow
. fixes #101no-template-shadow
.
83a230a
to
b1d17ed
Compare
no-template-shadow
.no-template-shadow
.
no-template-shadow
.no-template-shadow
.
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.
Good job @armano2 Now we can finally push this forward :) Please update PR when you'll find time
|
||
:-1: Examples of **incorrect** code for this rule: | ||
|
||
```html |
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 think we should present two or three cases here. First - a simple template example, and second with shadowed data
or method
property declarations. e.g.:
1.
<template>
<div>
<div v-for="i in 5">
<div v-for="i in 10"></div>
</div>
</div>
</template>
<template>
<div>
<div v-for="i in 5"></div>
</div>
</template>
<script>
export default {
data () {
return {
i: 10
}
}
}
</script>
lib/rules/no-template-shadow.js
Outdated
meta: { | ||
docs: { | ||
description: 'Disallow variable declarations from shadowing variables declared in the outer scope.', | ||
category: 'Possible Errors', |
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 think it should belong to essential
group
@@ -26,7 +26,7 @@ module.exports = { | |||
* | |||
* @param {RuleContext} context The rule context to use parser services. | |||
* @param {Object} templateBodyVisitor The visitor to traverse the template body. | |||
* @param {Object} scriptVisitor The visitor to traverse the script. | |||
* @param {Object} [scriptVisitor] The visitor to traverse the script. |
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.
Why square brackets here?
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.
It indicates an optional param
http://usejsdoc.org/tags-param.html#optional-parameters-and-default-values
{ | ||
filename: 'test.vue', | ||
code: `<template> | ||
<div v-for="i in 5"></div> |
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.
Let's add case with i in f
, wdyt?
} | ||
</script>`, | ||
errors: [{ | ||
message: "Variable 'i' is already declared in the upper scope.", |
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.
Please add line on which the following error is expected
docs/rules/no-template-shadow.md
Outdated
@@ -0,0 +1,46 @@ | |||
# Disallow variable declarations from shadowing variables declared in the outer scope. (no-template-shadow) | |||
|
|||
`no-shadow` should report variable definitions of v-for directives or scope attributes if those shadows the variables in parent scopes. |
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.
no-template-shadow
no-template-shadow
.no-template-shadow
.
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.
This looks good to me! Even though there's not currently a rule for this in the style guide, I would place it in strongly-recommended
when it's considered stable.
We're still in v5 beta. Let's put it in |
fixes #101