Rule proposal: `vue/no-duplicate-class-names`
Closed this issue · 4 comments
Hello I was looking for a rule to make sure I don't add duplicated class names in my code but I couldn't find any so I created a custom rule for my project but would be nice to have support for it in this plugin.
Please describe what the rule should do:
This rule can be used to void duplication of class names inside class attributes.
What category should the rule belong to?
- Enforces code style (layout)
- Warns about a potential error (problem)
- Suggests an alternate way of doing something (suggestion)
- Other (please specify:)
Provide 2-3 code examples that this rule should warn about:
<template>
<!-- ✓ GOOD -->
<div class="foo"></div>
<!-- ✗ BAD -->
<div class="foo foo"></div>
</template><template>
<!-- ✓ GOOD -->
<div :class="{ 'foo': true }"></div>
<!-- ✗ BAD -->
<div :class="{ 'foo foo': true }"></div>
</template><template>
<!-- ✓ GOOD -->
<div :class="['foo']"></div>
<!-- ✗ BAD -->
<div :class="['foo foo']"></div>
</template>I'd say this rule should check duplicates within each class attribute separately so it should not report cases where a class appears once in class="" and once in :class=""
And also should not report duplicates withing each object key/array items or mixed inside :class=""
I can help with the PR, tests and documentation as needed :)
I like the rule suggestion, thanks for sharing that idea!
I'd say this rule should check duplicates within each class attribute separately so it should not report cases where a class appears once in
class=""and once in:class=""
And also should not report duplicates withing each object key/array items or mixed inside:class=""
Why? Specifying the same class name twice in different places is probably a mistake and should be reported, shouldn't it?
After thinking about it, I agree with you for the simple case where we use both class and :class on the same element as string literals, those duplicates should definitely be reported.
<template>
<div class="foo" :class="'foo'"></div>
</template>My reasoning was mostly about how deep we should check, like what happens within the :class directive. Since :class supports objects, arrays, and conditionals and all of them nested, I initially wanted to only check for duplicates inside each string literal to avoid overly complex cases.
For example:
Here foo can be duplicated if isActive and isAnotherActive are true but we can't know if both expression are going to be true at the same time or the developer makes sure they're not so I believe this case should not be reported.
<template>
<div :class="{ 'foo bar': isActive, 'foo': isAnotherActive }"></div>
</template>The part I’m still unsure about is if we add the class attribute
For Example:
<template>
<div class="foo" :class="{ 'foo bar': isActive, 'foo': isAnotherActive }"></div>
</template>Since foo will be always present maybe we should report it because it is present in the conditions, but this feels much stricter and would add complexity, and I'd be unsure to add a safe fix to those cases.
All of this led me to not report this. Maybe it should report without a fix? but that would make the rule not fixable in some cases
Reporting duplicates in these complex cases would definitely be a nice improvement but unsure how strict it has to be. Maybe for now I should remove some of the "valid" test cases where can be a class duplicate (since they aren't really valid) and leave those cases open for improvement?
I can maybe try to implement the first simple case report
<template> <div class="foo" :class="'foo'"></div> </template>
I've applied some changes and I think the rule is now better, now casses like these when static class name exists in any :class are reported but not auto-fixed
<template>
<!-- ✗ BAD -->
<div class="foo" :class="'foo'"></div>
<div class="foo" :class="`foo`"></div>
<div class="foo" :class="['foo']"></div>
<div class="foo" :class="{ 'foo': isActive }"></div>
<div class="foo" :class="{ 'foo bar': isActive }"></div>
<div class="foo" :class="isActive ? 'foo' : 'bar'"></div>
<div class="foo" :class="'foo ' + 'bar'"></div>
</template>I alread covered a lot of cases but there are still cases that are no reported if we take into consideration the result of the expressions. For example:
<template>
<div :class="['foo', 'foo']"></div>
<div :class="'foo ' + 'foo'"></div>
</template>But I know for sure these should not be reported
<template>
<div :class="isActive ? 'foo' : 'foo'"></div>
<div :class="{ 'foo bar': isActive, 'foo': isAnotherActive }"></div>
</template>I managed to handle the cases without fix:
<template>
<!-- ✗ BAD -->
<div :class="['foo', 'foo']"></div>
<div :class="'foo ' + 'foo'"></div>
<div :class="['foo', { 'foo': isActive }]"></div>
</template>