-
-
Notifications
You must be signed in to change notification settings - Fork 48
feat: add svelte/require-each-key
rule
#473
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: b098c06 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 |
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 thought I would like to check if the key was a variable used in a #{each}
tag.
e.g.
<script>
let foo;
let bar;
</script>
<!-- This is NG -->
{#each foo as name (bar)}...{/each}
As you said, I think it would be useful for a rule to check if a key contains an iteration variable 👍 |
docs: { | ||
description: "require keyed `{#each}` block", | ||
category: "Best Practices", | ||
recommended: false, |
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 can toggle to true in major version up?
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.
Hmm. I personally like to enforce the key, but vue/require-v-for-key
is unpopular even in eslint-plugin-vue, so I think it's better not to include it in the preset. (However, the Vue style guide recommends using keys.)
I think it will be easier to introduce eslint if the presets are not too strict.
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.
Okay!
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 wrote one comment but LGTM!
close #459