-
-
Notifications
You must be signed in to change notification settings - Fork 696
fix(vue/valid-v-for): allow using an empty alias in value #2991
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
Conversation
🦋 Changeset detectedLatest commit: 3ea9630 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@FloEdelmann
FloEdelmann
left a comment
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.
Thank you for looking into it. Looks good to me 🙂
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.
Pull request overview
This PR fixes the vue/valid-v-for rule to correctly allow empty value aliases in v-for directives, such as v-for="(, key) in obj", which is valid Vue.js syntax when you only need the key/index from an iteration.
Key Changes:
- Updated validation logic to only report an error when both value and key are empty
- Added test cases for the newly allowed syntax patterns
- Removed incorrectly failing test case
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/rules/valid-v-for.js | Modified the empty alias validation to allow empty value when a key is present |
| tests/lib/rules/valid-v-for.js | Added valid test cases for (, key) and (, key, index) patterns; removed false-positive invalid test |
| .changeset/silly-falcons-explain.md | Added changeset documenting the patch-level fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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 is also a valid syntax, we consider it as invalid since no one is actually using the object's index, right?
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.
If using the index makes sense, I wonder if invalidEmptyAlias was intentionally added to avoid potential errors. Maybe this rule recommends explicitly enumerating all variables before use (and pairing it with other rules to handle unused variables).
If that goes beyond this rule, I think cases like (value, , index), (value,,)or (, , index)—while valid syntax—should also be allowed. (or invalidEmptyAlias could just focus on cases like (value,,) where the alias is empty at the end)
Let me know what you guys 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.
If using the index makes sense, I wonder if invalidEmptyAlias was intentionally added to avoid potential errors.
To be honest, I’m not sure what the original background or motivation for invalidEmptyAlias.
invalidEmptyAliascould just focus on cases like(value,,)where the alias is empty at the end.
I actually like this idea, it feels more reasonable to me. What do you think?
Under this definition, the behavior would be:
// GOOD (value, , index) in list (, key) in list (, key, index) in list (,, index) in list // BAD (,) in list (,,) in list (,,,) in list (value,,) in list (value, key,,) in list
resolves #2733
This is indeed valid syntax.
See Vue Playground