feat(rule): add rule scope-dom-lookups

closes ##19
This commit is contained in:
Dominik Ferber
2016-03-16 15:03:54 +01:00
parent 529156ea4a
commit 99e5c52060
5 changed files with 218 additions and 0 deletions

View File

@@ -64,6 +64,7 @@ An article with detailed setup instructions can be found [here](https://medium.c
* [no-template-lifecycle-assignments](docs/rules/no-template-lifecycle-assignments.md): Prevent deprecated template lifecycle callback assignments
* [eventmap-params](docs/rules/eventmap-params.md): Force consistent event handler parameter names in event maps
* [prefix-eventmap-selectors](docs/rules/prefix-eventmap-selectors.md): Convention for eventmap selectors
* [scope-dom-lookups](docs/rules/scope-dom-lookups.md): Scope DOM lookups to the template instance
## Core API
* *currently no rules implemented*

View File

@@ -0,0 +1,71 @@
# Scope DOM lookups to the template instance (scope-dom-lookups)
> Its a bad idea to look up things directly in the DOM with jQuerys global `$()`. Its easy to select some element on the page that has nothing to do with the current component. Also, it limits your options on rendering outside of the main document. - [source](http://guide.meteor.com/blaze.html#scope-dom-lookups-to-instance)
## Rule Details
This rule aims to ensure DOM lookups are scoped to the template instance to improve performance and to reduce accidental side-effects.
The following patterns are considered warnings:
```js
Template.foo.onRendered(function () {
$('.bar').focus()
})
Template.foo.onRendered(function () {
const $bar = $('.bar')
// ..
})
Template.foo.events({
'click .bar': function (event, instance) {
$('.baz').focus()
}
})
Template.foo.helpers({
'bar': function () {
$('.baz').focus()
}
})
Template.foo.onDestroyed(function () {
$('.bar').focus()
})
Template.foo.onRendered(function () {
jQuery('.bar').focus()
})
```
The following patterns are not warnings:
```js
Template.foo.onRendered(function () {
this.$('.bar').focus()
})
Template.foo.onRendered(function () {
Template.instance().$('.bar').focus()
})
Template.foo.events({
'click .bar': function (event, instance) {
instance.$('.baz').focus()
}
})
```
## When Not To Use It
Disable this rule for specific lines if something outside of the template needs to be looked up and there is no way around it.
## Further Reading
- http://guide.meteor.com/blaze.html#scope-dom-lookups-to-instance

View File

@@ -7,6 +7,7 @@ import eventmapParams from './rules/eventmap-params'
import prefixEventmapSelectors from './rules/prefix-eventmap-selectors'
import preferSessionEquals from './rules/prefer-session-equals'
import templateNamingConvention from './rules/template-names'
import scopeDomLookups from './rules/scope-dom-lookups'
export const rules = {
@@ -18,6 +19,7 @@ export const rules = {
'prefix-eventmap-selectors': prefixEventmapSelectors,
'prefer-session-equals': preferSessionEquals,
'template-names': templateNamingConvention,
'scope-dom-lookups': scopeDomLookups,
}
export const configs = {
@@ -31,6 +33,7 @@ export const configs = {
'meteor/prefix-eventmap-selectors': 0,
'meteor/prefer-session-equals': 0,
'meteor/template-names': 2,
'meteor/scope-dom-lookups': 0,
},
},
guide: {

View File

@@ -0,0 +1,41 @@
/**
* @fileoverview Scope DOM lookups to the template instance
* @author Dominik Ferber
* @copyright 2016 Dominik Ferber. All rights reserved.
* See LICENSE file in root directory for full license.
*/
import getPropertyName from '../util/ast/getPropertyName'
const jQueryNames = new Set(['$', 'jQuery'])
const relevantTemplatePropertyNames = new Set([
'onRendered',
'onDestroyed',
'events',
'helpers',
])
const isJQueryIdentifier = node =>
node.type === 'Identifier' && jQueryNames.has(node.name)
const isRelevantTemplateCallExpression = node => (
node.type === 'CallExpression' &&
node.callee.type === 'MemberExpression' &&
node.callee.object.type === 'MemberExpression' &&
node.callee.object.object.type === 'Identifier' &&
node.callee.object.object.name === 'Template' &&
relevantTemplatePropertyNames.has(getPropertyName(node.callee.property))
)
const isInRelevantTemplateScope = ancestors => ancestors.some(isRelevantTemplateCallExpression)
export default context => ({
CallExpression: node => {
if (!isJQueryIdentifier(node.callee)) return
if (!isInRelevantTemplateScope(context.getAncestors())) return
context.report(node, 'Use scoped DOM lookup instead')
},
})
export const schema = []

View File

@@ -0,0 +1,102 @@
/**
* @fileoverview Scope DOM lookups to the template instance
* @author Dominik Ferber
* @copyright 2016 Dominik Ferber. All rights reserved.
* See LICENSE file in root directory for full license.
*/
import rule from '../../../lib/rules/scope-dom-lookups'
import { RuleTester } from 'eslint'
const ruleTester = new RuleTester()
ruleTester.run('scope-dom-lookups', rule, {
valid: [
'$(".foo")',
'Template.foo.xyz(function () { $(".foo"); })',
`
Template.foo.onRendered(function () {
this.$('.bar').addClass('baz')
})
`,
`
Template.foo.onRendered(function () {
Template.instance().$('.bar').addClass('.baz')
})
`,
`
Template.foo.events({
'click .js-bar': function (event, instance) {
instance.$('.baz').focus()
}
})
`,
],
invalid: [
{
code: `
Template.foo.onRendered(function () {
$('.bar').addClass('baz')
})
`,
errors: [
{ message: 'Use scoped DOM lookup instead', type: 'CallExpression' },
],
},
{
code: `
Template.foo.events({
'click .js-bar': function (event, instance) {
$('.baz').focus()
}
})
`,
errors: [
{ message: 'Use scoped DOM lookup instead', type: 'CallExpression' },
],
},
{
code: `
Template.foo.onRendered(function () {
var $bar = $('.bar')
$bar.addClass('baz')
})
`,
errors: [
{ message: 'Use scoped DOM lookup instead', type: 'CallExpression' },
],
},
{
code: `
Template.foo.helpers({
'bar': function () {
$('.baz').focus()
}
})
`,
errors: [
{ message: 'Use scoped DOM lookup instead', type: 'CallExpression' },
],
},
{
code: `
Template.foo.onDestroyed(function () {
$('.bar').addClass('baz')
})
`,
errors: [
{ message: 'Use scoped DOM lookup instead', type: 'CallExpression' },
],
},
{
code: `
Template.foo.onRendered(function () {
jQuery('.bar').addClass('baz')
})
`,
errors: [
{ message: 'Use scoped DOM lookup instead', type: 'CallExpression' },
],
},
],
})