Skip to content

Update attribute-hyphenation to allow a custom ignore list #461

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

Merged
merged 5 commits into from
Apr 22, 2018

Conversation

MechJosh0
Copy link
Contributor

Changes:

  • By default it'll still ignore data- and aria-, added to the default ignore list is slot-scope as this is a Vue attribute that cannot be un-hyphenated.
  • Users can now do "vue/attribute-hyphenation": ["error", "never", {"ignore": ["data-", "aria-", "my-prop"]}] to overwrite the existing ignore list.
  • Added /.idea folder created by Phpstorm to the /.gitignore list.

@@ -1,17 +1,17 @@
# enforce attribute naming style in template (vue/attribute-hyphenation)
# enforce attribute naming style on custom components in template (vue/attribute-hyphenation)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is automatically generated based on the rule's metadata informations. If you want to update it please - do so in rule file instead. Otherwise it'll be overwritten during next release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it just this one line coming from meta.docs.description and needs updating? If so I've done it, let me know if there is anything else.

if (optionsPayload && optionsPayload.ignore) {
ignoredAttributes = optionsPayload.ignore
} else {
ignoredAttributes = ['data-', 'aria-', 'slot-scope']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can set ignoredAttributes above and remove the whole else clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense, I have done this change.

@@ -48,9 +74,14 @@ module.exports = {
}

function isIgnoredAttribute (value) {
if (value.indexOf('data-') !== -1 || value.indexOf('aria-') !== -1) {
const isIgnored = !ignoredAttributes.every(function (attr) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you can use .some and remove the negation?

ignoredAttributes.some(function (attr) {
  return value.indexOf(attr) !== -1;
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does make more sense, thanks.

@@ -38,3 +38,17 @@ Default casing is set to `always`
```html
<MyComponent my-prop="prop"/>
```

### `[2, "never", { 'ignore': ['data-', 'aria-', 'slot-scope', 'custom-prop'] }]` - Don't use hyphenated name but allow custom attributes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether it wouldn't be easier for users to just merge their settings with default ones. I can't imagine a case where you'd want to remote any of the default settings - they are sort of required for this rule to work properly. So I'd just assume these setting are always there and whatever user adds here - will be merged to the default array.
What do you think about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was providing more flexibility for long term use if things change, but I do agree that it's unlikely people will want to overwrite these defaults. I have updated it so it will not overwrite the default array but instead push to it.

Copy link
Member

@michalsnik michalsnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work 👍

{
filename: 'test.vue',
code: '<template><div data-id="foo" aria-test="bar" slot-scope="{ data }" custom-hypen="foo"><a onClick="" my-prop="prop"></a></div></template>',
options: ['never', { 'ignore': ['data-', 'aria-', 'slot-scope', 'custom-hypen'] }]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, one thing you missed. We don't need to pass 'data-', 'aria-', 'slot-scope' anymore :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants