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

Add prefer-dataset rule #225

Merged
merged 13 commits into from
Sep 16, 2019
17 changes: 17 additions & 0 deletions docs/rules/prefer-dataset.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Prefer using [`.dataset`](https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/dataset) on DOM elements over `.setAttribute(…)`

This rule is fixable.


## Fail

```js
element.setAttribute('data-unicorn', '🦄');
```


## Pass

```js
element.dataset.unicorn = '🦄';
```
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ module.exports = {
'unicorn/no-zero-fractions': 'error',
'unicorn/number-literal-case': 'error',
'unicorn/prefer-add-event-listener': 'error',
'unicorn/prefer-dataset': 'error',
'unicorn/prefer-event-key': 'error',
'unicorn/prefer-exponentiation-operator': 'error',
'unicorn/prefer-flat-map': 'error',
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ Configure it in `package.json`.
"unicorn/no-zero-fractions": "error",
"unicorn/number-literal-case": "error",
"unicorn/prefer-add-event-listener": "error",
"unicorn/prefer-dataset": "error",
"unicorn/prefer-event-key": "error",
"unicorn/prefer-exponentiation-operator": "error",
"unicorn/prefer-flat-map": "error",
Expand Down Expand Up @@ -101,6 +102,7 @@ Configure it in `package.json`.
- [no-zero-fractions](docs/rules/no-zero-fractions.md) - Disallow number literals with zero fractions or dangling dots. *(fixable)*
- [number-literal-case](docs/rules/number-literal-case.md) - Enforce lowercase identifier and uppercase value for number literals. *(fixable)*
- [prefer-add-event-listener](docs/rules/prefer-add-event-listener.md) - Prefer `.addEventListener()` and `.removeEventListener()` over `on`-functions. *(partly fixable)*
- [prefer-dataset](docs/rules/prefer-dataset.md) - Prefer using `.dataset` on DOM elements over `.setAttribute(…)`. *(fixable)*
- [prefer-event-key](docs/rules/prefer-event-key.md) - Prefer `KeyboardEvent#key` over `KeyboardEvent#keyCode`. *(partly fixable)*
- [prefer-exponentiation-operator](docs/rules/prefer-exponentiation-operator.md) - Prefer the exponentiation operator over `Math.pow()` *(fixable)*
- [prefer-flat-map](docs/rules/prefer-flat-map.md) - Prefer `.flatMap(…)` over `.map(…).flat()`. *(fixable)*
Expand Down
78 changes: 78 additions & 0 deletions rules/prefer-dataset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
'use strict';
const getDocsUrl = require('./utils/get-docs-url');

const getMethodName = memberExpression => memberExpression.property.name;

const getDataAttributeName = arg => {
if (arg.type === 'Literal') {
return (arg.value.match(/^data-(.+)/) || ['', ''])[1];
}

return '';
};

const parseValueArgument = (context, arg) => context.getSourceCode().getText(arg);

const dashToCamelCase = string => string.replace(/-([a-z])/g, s => s[1].toUpperCase());

const getReplacement = (context, node, memberExpression, propertyName) => {
const objectName = memberExpression.object.name;
const value = parseValueArgument(context, node.arguments[1]);

propertyName = dashToCamelCase(propertyName);

if (propertyName.match(/[.:]/)) {
return `${objectName}.dataset['${propertyName}'] = ${value}`;
}

return `${objectName}.dataset.${propertyName} = ${value}`;
};

const isBracketNotation = (context, callee) => {
const bracketOpen = context.getSourceCode().getFirstTokenBetween(callee.object, callee.property, {
filter: token => token.value === '['
});

return bracketOpen !== null && bracketOpen.value === '[';
};

const create = context => {
return {
CallExpression(node) {
const {callee} = node;

if (callee.type !== 'MemberExpression') {
return;
}

if (getMethodName(callee) !== 'setAttribute') {
return;
}

if (isBracketNotation(context, callee)) {
return;
}

const attributeName = getDataAttributeName(node.arguments[0]);
if (attributeName) {
const replacement = getReplacement(context, node, callee, attributeName);
context.report({
node,
message: 'Prefer `.dataset` over `setAttribute(…)`.',
fix: fixer => fixer.replaceText(node, replacement)
});
}
}
};
};

module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
url: getDocsUrl(__filename)
},
fixable: 'code'
}
};
75 changes: 75 additions & 0 deletions test/prefer-dataset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import test from 'ava';
import {outdent} from 'outdent';
import avaRuleTester from 'eslint-ava-rule-tester';
import rule from '../rules/prefer-dataset';

const ruleTester = avaRuleTester(test, {
env: {
es6: true
}
});

const errors = [
{
ruleId: 'prefer-dataset',
message: 'Prefer `.dataset` over `setAttribute(…)`.'
}
];

ruleTester.run('prefer-dataset', rule, {
valid: [
'element.dataset.unicorn = \'🦄\';',
'element.dataset[\'unicorn\'] = \'🦄\';',
'element[setAttribute](\'data-unicorn\', \'🦄\');',
'element.setAttribute(\'foo\', \'bar\');',
'element.setAttribute(foo, bar);',
'element.getAttribute(\'data-unicorn\');'
],
invalid: [
{
code: 'element.setAttribute(\'data-unicorn\', \'🦄\');',
errors,
output: 'element.dataset.unicorn = \'🦄\';'
},
{
code: 'element.setAttribute(\'data-🦄\', \'🦄\');',
errors,
output: 'element.dataset.🦄 = \'🦄\';'
},
{
code: 'element.setAttribute(\'data-foo2\', \'🦄\');',
errors,
output: 'element.dataset.foo2 = \'🦄\';'
},
{
code: 'element.setAttribute(\'data-foo:bar\', \'zaz\');',
errors,
output: 'element.dataset[\'foo:bar\'] = \'zaz\';'
},
{
code: 'element.setAttribute(\'data-foo.bar\', \'zaz\');',
errors,
output: 'element.dataset[\'foo.bar\'] = \'zaz\';'
},
{
code: 'element.setAttribute(\'data-foo-bar\', \'zaz\');',
errors,
output: 'element.dataset.fooBar = \'zaz\';'
},
{
code: 'element.setAttribute(\'data-foo\', /* comment */ \'bar\');',
errors,
output: 'element.dataset.foo = \'bar\';'
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we want to delete comments during fixing but it probably doesn't come up that often. @sindresorhus, what's your take on this?

  1. Removing comments is okay
  2. Comments should be preserved (may be non-trivial)
  3. Do not fix if comments are detected

I've done (3) in the past but I think a bunch of different rules current do (1).

Copy link
Owner

Choose a reason for hiding this comment

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

@MrHen I don't think we should do 1., but we need to come up with a guideline that we can apply to all rules. Maybe we could make an utility that just extract all comments to above a node we give it? The comments can require manual formatting and movement. We just shouldn't remove them. Alternatively, 3. if 2. is too complex, but I think we could make something generic that we could apply to many rules. Would you be able to open a new issue about it? We probably need to review all existing rules for this and update the contributing guidelines.

Copy link
Contributor

Choose a reason for hiding this comment

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

},
{
code: outdent`
element.setAttribute(
\'data-foo\', // comment
\'bar\' // comment
);
`,
errors,
output: 'element.dataset.foo = \'bar\';'
}
amedora marked this conversation as resolved.
Show resolved Hide resolved
]
});