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.

Sign up for GitHub

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

Jump to bottom

[RFC] Refactor shared logic into withMethod and withProperty helpers #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[RFC] Refactor shared logic into withMethod and withProperty helpers #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 11 additions & 16 deletions rules/no-ajax.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,19 @@
'use strict'

const utils = require('./utils.js')

module.exports = function(context) {
return {
CallExpression: function(node) {
if (node.callee.type !== 'MemberExpression') return
if (node.callee.object.name !== '$') return

const name = node.callee.property.name
switch (name) {
case 'ajax':
case 'get':
case 'getJSON':
case 'getScript':
case 'post':
context.report({
node: node,
message: 'Prefer fetch to $.' + name
})
CallExpression: utils.withProperty(
['ajax', 'get', 'getJSON', 'getScript', 'post'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can also pass an array of property or method names into these util functions if you want to check multiple property/method names.

function(node) {
const name = node.callee.property.name
context.report({
node: node,
message: 'Prefer fetch to $.' + name
})
}
}
)
}
}

Expand Down
17 changes: 6 additions & 11 deletions rules/no-css.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,12 @@ const utils = require('./utils.js')

module.exports = function(context) {
return {
CallExpression: function(node) {
if (node.callee.type !== 'MemberExpression') return
if (node.callee.property.name !== 'css') return

if (utils.isjQuery(node)) {
context.report({
node: node,
message: 'Prefer getComputedStyle to $.css'
})
}
}
CallExpression: utils.withMethod('css', function(node) {
context.report({
node: node,
message: 'Prefer getComputedStyle to $.css'
})
})
}
}

Expand Down
10 changes: 4 additions & 6 deletions rules/no-deferred.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
'use strict'

module.exports = function(context) {
function enforce(node) {
if (node.callee.type !== 'MemberExpression') return
if (node.callee.object.name !== '$') return
if (node.callee.property.name !== 'Deferred') return
const utils = require('./utils.js')

module.exports = function(context) {
const enforce = utils.withProperty('Deferred', function(node) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since these helper functions return a function, you can assign it to a variable and use it as a visitor callback for multiple node types (like the CallExpression and NewExpression in this example).

context.report({
node: node,
message: 'Prefer Promise to $.Deferred'
})
}
})

return {
CallExpression: enforce,
Expand Down
10 changes: 4 additions & 6 deletions rules/no-global-eval.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
'use strict'

const utils = require('./utils.js')

module.exports = function(context) {
return {
CallExpression: function(node) {
if (node.callee.type !== 'MemberExpression') return
if (node.callee.object.name !== '$') return
if (node.callee.property.name !== 'globalEval') return

CallExpression: utils.withProperty('globalEval', function(node) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The callback function is called when a match is found, which allows the caller to call context.report() and/or access the node itself.

context.report({
node: node,
message: '$.globalEval is not allowed'
})
}
})
}
}

Expand Down
36 changes: 35 additions & 1 deletion rules/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,41 @@ function isjQuery(node) {
return id && id.name.startsWith('$')
}

function withProperty(property, callback) {
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 helper function encapsulates the visitor logic when checking a property on the jQuery global, for example:

$.get()

I'm not convinced on the name but this with the best thing I could come up with at the time. 😂

if (typeof callback !== 'function') {
throw new TypeError('Must provide callback function')
}

const properties = new Set([].concat(property))

return function(node) {
if (node.callee.type !== 'MemberExpression') return
if (node.callee.object.name !== '$') return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In regards to #18, if we pass the ESLint context into the withProperty() or withMethod() helpers, we can refactor this line to support jQuery global aliases:

// pseudocode
const aliases = getjQueryAliases(context)
if (!aliases.has(node.callee.object.name)) return

if (!properties.has(node.callee.property.name)) return

callback(node)
}
}

function withMethod(method, callback) {
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 helper function encapsulates the visitor logic when checking a method call on a jQuery object, for example:

// the .each()
$('li').each()

Again, open to clearer naming if you have suggestions.

if (typeof callback !== 'function') {
throw new TypeError('Must provide callback function')
}

const methods = new Set([].concat(method))

return function(node) {
if (node.callee.type !== 'MemberExpression') return
if (!methods.has(node.callee.property.name)) return
if (isjQuery(node)) {
callback(node)
}
}
}

module.exports = {
traverse: traverse,
isjQuery: isjQuery
isjQuery: isjQuery,
withMethod: withMethod,
withProperty: withProperty
}