Skip to content
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

Rule Proposal: vue/no-shadow #101

Closed
mysticatea opened this issue Jul 20, 2017 · 4 comments · Fixed by #158
Closed

Rule Proposal: vue/no-shadow #101

mysticatea opened this issue Jul 20, 2017 · 4 comments · Fixed by #158

Comments

@mysticatea
Copy link
Member

Please describe what the rule should do:

no-shadow should report variable definitions of v-for directives or scope attributes if those shadows the variables in parent scopes.

What category of rule is this? (place an "X" next to just one item)

[ ] Enforces code style
[X] Warns about a potential error
[ ] Suggests an alternate way of doing something
[ ] Other (please specify:)

Provide 2-3 code examples that this rule will warn about:

<template>
    <ol v-for="i in 5">
        <ol v-for="i in 5"><!-- "i" is already declared in the upper scope. -->
            <li>item</li>
        </ol>
    </ol>
</template>
<script>
export default {
}
</script>
@michalsnik
Copy link
Member

no-template-shadow seems more accurate as there is already eslint's core rule with that name and it might be a little confusing how do they differ.

@armano2
Copy link
Contributor

armano2 commented Jul 24, 2017

@mysticatea i'm about to start working on this but can you explain me what do you mean by scope attributes,


edit. i will wait till @mysticatea finish his parser

@mysticatea
Copy link
Member Author

I'm sorry for the delay.

@michalsnik It's the good name.

@armano2 Each VElement node has all variables which are defined by their attributes. It includes v-for directive's and <template scope="foo"> attribute's (Scoped slots).

Incidentally, I think that this rule should consider the properties of components are defined as well.

<template>
    <ol v-for="i in 5"><!-- "i" is already declared in the upper scope. -->
    </ol>
</template>
<script>
export default {
    data() {
        return {i: 0}
    }
}
</script>

So I think the mostly logic of this rule will be shared with #98.

@armano2
Copy link
Contributor

armano2 commented Aug 14, 2017

@mysticatea @michalsnik if no one is working on it, i can take care of it

armano2 added a commit to armano2/eslint-plugin-vue that referenced this issue Aug 14, 2017
armano2 added a commit to armano2/eslint-plugin-vue that referenced this issue Sep 17, 2017
@michalsnik michalsnik added this to the v4.1.0 milestone Oct 8, 2017
@michalsnik michalsnik removed this from the v4.1.0 milestone Nov 26, 2017
michalsnik pushed a commit that referenced this issue Jul 30, 2018
* Add rule `no-template-shadow`.
fixes #101

* fix/merge changes with master

* Update tests suite, review suggestions

* Add more test cases

* Change no-template-shadow category to strongly-recommended
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants