Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
prefer-modern-dom-apis: Only fix when expression is not used (#503)
  • Loading branch information
brettz9 committed Feb 1, 2020
1 parent fb0268b commit 096fead
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 62 deletions.
53 changes: 21 additions & 32 deletions rules/prefer-modern-dom-apis.js
@@ -1,5 +1,6 @@
'use strict';
const getDocumentationUrl = require('./utils/get-documentation-url');
const isValueNotUsable = require('./utils/is-value-not-usable');

const getArgumentNameForReplaceChildOrInsertBefore = nodeArguments => {
if (nodeArguments.type === 'Identifier') {
Expand All @@ -12,14 +13,6 @@ const forbiddenIdentifierNames = new Map([
['insertBefore', 'before']
]);

const isPartOfVariableAssignment = nodeParentType => {
if (nodeParentType === 'VariableDeclarator' || nodeParentType === 'AssignmentExpression') {
return true;
}

return false;
};

const checkForReplaceChildOrInsertBefore = (context, node) => {
const identifierName = node.callee.property.name;

Expand All @@ -42,24 +35,22 @@ const checkForReplaceChildOrInsertBefore = (context, node) => {
}

const parentNode = node.callee.object.name;
// This check makes sure that only the first method of chained methods with same identifier name e.g: parentNode.insertBefore(alfa, beta).insertBefore(charlie, delta); gets transformed
// This check makes sure that only the first method of chained methods with same identifier name e.g: parentNode.insertBefore(alfa, beta).insertBefore(charlie, delta); gets reported
if (!parentNode) {
return;
}

const preferredSelector = forbiddenIdentifierNames.get(identifierName);

let fix = fixer => fixer.replaceText(
node,
`${oldChildNodeArgument}.${preferredSelector}(${newChildNodeArgument})`
);

// Report error when the method is part of a variable assignment
// but don't offer to autofix `.replaceWith()` and `.before()`
// which don't have a return value.
if (isPartOfVariableAssignment(node.parent.type)) {
fix = undefined;
}
const fix = isValueNotUsable(node) ?
// Report error when the method is part of a variable assignment
// but don't offer to autofix `.replaceWith()` and `.before()`
// which don't have a return value.
fixer => fixer.replaceText(
node,
`${oldChildNodeArgument}.${preferredSelector}(${newChildNodeArgument})`
) :
undefined;

return context.report({
node,
Expand Down Expand Up @@ -112,18 +103,16 @@ const checkForInsertAdjacentTextOrInsertAdjacentElement = (context, node) => {
nodeArguments[1]
);

let fix = fixer =>
fixer.replaceText(
node,
`${referenceNode}.${preferredSelector}(${insertedTextArgument})`
);

// Report error when the method is part of a variable assignment
// but don't offer to autofix `.insertAdjacentElement()`
// which don't have a return value.
if (identifierName === 'insertAdjacentElement' && isPartOfVariableAssignment(node.parent.type)) {
fix = undefined;
}
const fix = identifierName === 'insertAdjacentElement' && !isValueNotUsable(node) ?
// Report error when the method is part of a variable assignment
// but don't offer to autofix `.insertAdjacentElement()`
// which doesn't have a return value.
undefined :
fixer =>
fixer.replaceText(
node,
`${referenceNode}.${preferredSelector}(${insertedTextArgument})`
);

return context.report({
node,
Expand Down
4 changes: 2 additions & 2 deletions rules/prefer-node-append.js
@@ -1,6 +1,6 @@
'use strict';
const getDocumentationUrl = require('./utils/get-documentation-url');
const isValueUsed = require('./utils/is-value-used');
const isValueNotUsable = require('./utils/is-value-not-usable');

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

Expand All @@ -10,7 +10,7 @@ const create = context => {
const {callee} = node;

if (callee.type === 'MemberExpression' && getMethodName(callee) === 'appendChild') {
const fix = isValueUsed(node) ? undefined : fixer => fixer.replaceText(callee.property, 'append');
const fix = isValueNotUsable(node) ? fixer => fixer.replaceText(callee.property, 'append') : undefined;

context.report({
node,
Expand Down
4 changes: 2 additions & 2 deletions rules/prefer-node-remove.js
@@ -1,6 +1,6 @@
'use strict';
const getDocumentationUrl = require('./utils/get-documentation-url');
const isValueUsed = require('./utils/is-value-used');
const isValueNotUsable = require('./utils/is-value-not-usable');

const getMethodName = callee => {
const {property} = callee;
Expand Down Expand Up @@ -66,7 +66,7 @@ const create = context => {
const argumentName = getArgumentName(node.arguments);

if (argumentName) {
const fix = isValueUsed(node) ? undefined : fixer => fixer.replaceText(node, `${argumentName}.remove()`);
const fix = isValueNotUsable(node) ? fixer => fixer.replaceText(node, `${argumentName}.remove()`) : undefined;

context.report({
node,
Expand Down
3 changes: 3 additions & 0 deletions rules/utils/is-value-not-usable.js
@@ -0,0 +1,3 @@
'use strict';

module.exports = ({parent}) => !parent || parent.type === 'ExpressionStatement';
25 changes: 0 additions & 25 deletions rules/utils/is-value-used.js

This file was deleted.

74 changes: 73 additions & 1 deletion test/prefer-modern-dom-apis.js
Expand Up @@ -102,7 +102,7 @@ ruleTester.run('prefer-modern-dom-apis', rule, {
'Prefer `beta.before(alfa)` over `parentNode.insertBefore(alfa, beta)`.'
}
],
output: 'beta.before(alfa).insertBefore(charlie, delta);'
output: 'parentNode.insertBefore(alfa, beta).insertBefore(charlie, delta);'
},
{
code: 'const foo = parentNode.insertBefore(alfa, beta);',
Expand All @@ -124,6 +124,28 @@ ruleTester.run('prefer-modern-dom-apis', rule, {
],
output: 'foo = parentNode.insertBefore(alfa, beta);'
},
{
code: 'new Dom(parentNode.insertBefore(alfa, beta))',
errors: [
{
message:
'Prefer `beta.before(alfa)` over `parentNode.insertBefore(alfa, beta)`.'
}
],
output: 'new Dom(parentNode.insertBefore(alfa, beta))'
},
{
/* eslint-disable no-template-curly-in-string */
code: '`${parentNode.insertBefore(alfa, beta)}`',
errors: [
{
message:
'Prefer `beta.before(alfa)` over `parentNode.insertBefore(alfa, beta)`.'
}
],
output: '`${parentNode.insertBefore(alfa, beta)}`'
/* eslint-enable no-template-curly-in-string */
},
// Tests for .insertAdjacentText()
{
code: 'referenceNode.insertAdjacentText("beforebegin", "text");',
Expand Down Expand Up @@ -275,6 +297,56 @@ ruleTester.run('prefer-modern-dom-apis', rule, {
}
],
output: 'foo = referenceNode.insertAdjacentElement("beforebegin", newNode);'
},
{
code: 'const foo = [referenceNode.insertAdjacentElement("beforebegin", newNode)]',
errors: [
{
message:
'Prefer `referenceNode.before(newNode)` over `referenceNode.insertAdjacentElement("beforebegin", newNode)`.'
}
],
output: 'const foo = [referenceNode.insertAdjacentElement("beforebegin", newNode)]'
},
{
code: 'foo(bar = referenceNode.insertAdjacentElement("beforebegin", newNode))',
errors: [
{
message:
'Prefer `referenceNode.before(newNode)` over `referenceNode.insertAdjacentElement("beforebegin", newNode)`.'
}
],
output: 'foo(bar = referenceNode.insertAdjacentElement("beforebegin", newNode))'
},
{
code: 'const foo = () => { return referenceNode.insertAdjacentElement("beforebegin", newNode); }',
errors: [
{
message:
'Prefer `referenceNode.before(newNode)` over `referenceNode.insertAdjacentElement("beforebegin", newNode)`.'
}
],
output: 'const foo = () => { return referenceNode.insertAdjacentElement("beforebegin", newNode); }'
},
{
code: 'if (referenceNode.insertAdjacentElement("beforebegin", newNode)) {}',
errors: [
{
message:
'Prefer `referenceNode.before(newNode)` over `referenceNode.insertAdjacentElement("beforebegin", newNode)`.'
}
],
output: 'if (referenceNode.insertAdjacentElement("beforebegin", newNode)) {}'
},
{
code: 'const foo = { bar: referenceNode.insertAdjacentElement("beforebegin", newNode) }',
errors: [
{
message:
'Prefer `referenceNode.before(newNode)` over `referenceNode.insertAdjacentElement("beforebegin", newNode)`.'
}
],
output: 'const foo = { bar: referenceNode.insertAdjacentElement("beforebegin", newNode) }'
}
]
});

0 comments on commit 096fead

Please sign in to comment.