Skip to content

Add event listener lints #110

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 7 commits into from
Jul 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions docs/rules/no-useless-passive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# No Useless Passive

This rule disallows setting `passive: true` for events on which it will have no effect.

Events where `passive: true` has an effect are: `touchstart`, `touchmove`, `wheel`, and `mousewheel`.

```js
// bad (passive has no effect here)
window.addEventListener('scroll', () => { /* ... */ }, { passive: true })

// good
window.addEventListener('scroll', () => { /* ... */ })
```

## Why?

Adding `passive: true` informs the browser that this event will not be calling `preventDefault` and as such is safe to asynchronously dispatch, freeing up the main thread for lag-free operation. However many events are not cancel-able and as such setting `passive: true` will have no effect on the browser.

It is safe to leave the option set, but this may have a negative effect on code authors, as they might believe setting `passive: true` has a positive effect on their operations, leading to a false-confidence in code with `passive: true`. As such, removing the option where it has no effect demonstrates to the code author that this code will need to avoid expensive operations as this might have a detrimental affect on UI performance.

## See Also

https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Improving_scrolling_performance_with_passive_listeners
47 changes: 47 additions & 0 deletions docs/rules/prefer-observers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Prever Observers

Some events, such as `scroll` and `resize` have traditionally caused performance issues on web pages, as they are high frequency events, firing many times per second as the user interacts with the page viewport.

## Scroll vs IntersectionObserver

Typically `scroll` events are used to determine if an element is intersecting a viewport, which can result in expensive operations such as layout calculations, which has a detrimental affect on UI performance. Recent developments in web standards have introduced the `IntersectionObserver`, which is a more performant mechanism for determining if an element is intersecting the viewport.

```js
// bad, expensive, error-prone code to determine if the element is in the viewport;
window.addEventListener('scroll', () => {
const isIntersecting = checkIfElementIntersects(element.getBoundingClientRect(), window.innerHeight, document.clientHeight)
element.classList.toggle('intersecting', isIntersecting)
})

// good - more performant and less error-prone
const observer = new IntersectionObserver(entries => {
for(const {target, isIntersecting} of entries) {
target.classList.toggle(target, isIntersecting)
}
})
observer.observe(element)
```

## Resize vs ResizeObserver

Similarly, `resize` events are typically used to determine if the viewport is smaller or larger than a certain size, similar to CSS media break points. Similar to the `IntersectionObserver` the `ResizeObserver` also exists as a more performant mechanism for observing changes to the viewport size.

```js
// bad, low-performing code to determine if the element is less than 500px large
window.addEventListener('resize', () => {
element.classList.toggle('size-small', element.getBoundingClientRect().width < 500)
})

// good - more performant, only fires when the actual elements change size
const observer = new ResizeObserver(entries => {
for(const {target, contentRect} of entries) {
target.classList.toggle('size-small', contentRect.width < 500)
}
})
observer.observe(element)
```

## See Also

https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API
https://developer.mozilla.org/en-US/docs/Web/API/Resize_Observer_API
21 changes: 21 additions & 0 deletions docs/rules/require-passive-events.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Require Passive Events

This rule enforces adding `passive: true` to high frequency event listeners (`touchstart`, `touchmove`, `wheel`, `mousewheel`).

```js
// bad
window.addEventListener('touchstart', () => { /* ... */ })

// good
window.addEventListener('touchstart', () => { /* ... */ }, { passive: true })
```

## Why?

Adding these events listeners can block the main thread as it waits to find out if the callbacks call `preventDefault`. This can cause large amounts UI lag, which will be noticeable for users.

Adding `passive: true` informs the browser that this event will not be calling `preventDefault` and as such is safe to asynchronously dispatch, freeing up the main thread for lag-free operation.

## See Also

https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Improving_scrolling_performance_with_passive_listeners
5 changes: 4 additions & 1 deletion lib/configs/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ module.exports = {
'github/no-blur': 'error',
'github/no-dataset': 'error',
'github/no-innerText': 'error',
'github/unescaped-html-literal': 'error'
'github/unescaped-html-literal': 'error',
'github/no-useless-passive': 'error',
'github/require-passive-events': 'error',
'github/prefer-observers': 'error'
}
}
46 changes: 46 additions & 0 deletions lib/rules/no-useless-passive.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
const passiveEventListenerNames = new Set(['touchstart', 'touchmove', 'wheel', 'mousewheel'])

const propIsPassiveTrue = prop => prop.key && prop.key.name === 'passive' && prop.value && prop.value.value === true

module.exports = {
meta: {
docs: {},
fixable: 'code'
},

create(context) {
return {
['CallExpression[callee.property.name="addEventListener"]']: function(node) {
const [name, listener, options] = node.arguments
if (name.type !== 'Literal') return
if (passiveEventListenerNames.has(name.value)) return
if (options && options.type === 'ObjectExpression') {
const i = options.properties.findIndex(propIsPassiveTrue)
if (i === -1) return
const passiveProp = options.properties[i]
const l = options.properties.length
const source = context.getSourceCode()
context.report({
node: passiveProp,
message: `"${name.value}" event listener is not cancellable and so \`passive: true\` does nothing.`,
fix(fixer) {
const removals = []
if (l === 1) {
removals.push(options)
removals.push(...source.getTokensBetween(listener, options))
} else {
removals.push(passiveProp)
if (i > 0) {
removals.push(...source.getTokensBetween(options.properties[i - 1], passiveProp))
} else {
removals.push(...source.getTokensBetween(passiveProp, options.properties[i + 1]))
}
}
return removals.map(t => fixer.remove(t))
}
})
}
}
}
}
}
24 changes: 24 additions & 0 deletions lib/rules/prefer-observers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
const observerMap = {
scroll: 'IntersectionObserver',
resize: 'ResizeObserver'
}
module.exports = {
meta: {
docs: {},
fixable: 'code'
},

create(context) {
return {
['CallExpression[callee.property.name="addEventListener"]']: function(node) {
const [name] = node.arguments
if (name.type !== 'Literal') return
if (!(name.value in observerMap)) return
context.report({
node,
message: `Avoid using "${name.value}" event listener. Consider using ${observerMap[name.value]} instead`
})
}
}
}
}
22 changes: 22 additions & 0 deletions lib/rules/require-passive-events.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const passiveEventListenerNames = new Set(['touchstart', 'touchmove', 'wheel', 'mousewheel'])

const propIsPassiveTrue = prop => prop.key && prop.key.name === 'passive' && prop.value && prop.value.value === true

module.exports = {
meta: {
docs: {}
},

create(context) {
return {
['CallExpression[callee.property.name="addEventListener"]']: function(node) {
const [name, listener, options] = node.arguments
if (!listener) return
if (name.type !== 'Literal') return
if (!passiveEventListenerNames.has(name.value)) return
if (options && options.type === 'ObjectExpression' && options.properties.some(propIsPassiveTrue)) return
context.report(node, `High Frequency Events like "${name.value}" should be \`passive: true\``)
}
}
}
}
40 changes: 40 additions & 0 deletions tests/no-useless-passive.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
const rule = require('../lib/rules/no-useless-passive')
const RuleTester = require('eslint').RuleTester

const ruleTester = new RuleTester()

ruleTester.run('no-useless-passive', rule, {
valid: [
{
code: 'document.addEventListener("scroll", function(event) {})'
},
{
code: 'document.addEventListener("resize", function(event) {})'
},
{
code: 'document.addEventListener("resize", function(event) {}, { passive: false })'
}
],
invalid: [
{
code: 'document.addEventListener("scroll", function(event) {}, { passive: true })',
output: 'document.addEventListener("scroll", function(event) {} )',
errors: [
{
message: '"scroll" event listener is not cancellable and so `passive: true` does nothing.',
type: 'Property'
}
]
},
{
code: 'document.addEventListener("scroll", function(event) {}, { passive: true, foo: 1 })',
output: 'document.addEventListener("scroll", function(event) {}, { foo: 1 })',
errors: [
{
message: '"scroll" event listener is not cancellable and so `passive: true` does nothing.',
type: 'Property'
}
]
}
]
})
32 changes: 32 additions & 0 deletions tests/prefer-observers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
const rule = require('../lib/rules/prefer-observers')
const RuleTester = require('eslint').RuleTester

const ruleTester = new RuleTester()

ruleTester.run('prefer-observers', rule, {
valid: [
{
code: 'document.addEventListener("touchstart", function(event) {})'
}
],
invalid: [
{
code: 'document.addEventListener("scroll", function(event) {})',
errors: [
{
message: 'Avoid using "scroll" event listener. Consider using IntersectionObserver instead',
type: 'CallExpression'
}
]
},
{
code: 'document.addEventListener("resize", function(event) {})',
errors: [
{
message: 'Avoid using "resize" event listener. Consider using ResizeObserver instead',
type: 'CallExpression'
}
]
}
]
})
69 changes: 69 additions & 0 deletions tests/require-passive-events.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
const rule = require('../lib/rules/require-passive-events')
const RuleTester = require('eslint').RuleTester

const ruleTester = new RuleTester()

ruleTester.run('require-passive-events', rule, {
valid: [
{
code: 'document.addEventListener("load", function(event) {})'
},
{
code: 'document.addEventListener("click", function(event) {})'
},
{
code: 'document.addEventListener("touchstart", function(event) {}, { passive: true })'
},
{
code: 'el.addEventListener("touchstart", function(event) {}, { passive: true })'
}
],
invalid: [
{
code: 'document.addEventListener("touchstart", function(event) {})',
errors: [
{
message: 'High Frequency Events like "touchstart" should be `passive: true`',
type: 'CallExpression'
}
]
},
{
code: 'el.addEventListener("wheel", function(event) {})',
errors: [
{
message: 'High Frequency Events like "wheel" should be `passive: true`',
type: 'CallExpression'
}
]
},
{
code: 'document.addEventListener("wheel", function(event) {})',
errors: [
{
message: 'High Frequency Events like "wheel" should be `passive: true`',
type: 'CallExpression'
}
]
},
{
// Intentionally mispelled!
code: 'document.addEventListener("wheel", function(event) {}, { pssive: true })',
errors: [
{
message: 'High Frequency Events like "wheel" should be `passive: true`',
type: 'CallExpression'
}
]
},
{
code: 'document.addEventListener("wheel", function(event) {}, { passive: false })',
errors: [
{
message: 'High Frequency Events like "wheel" should be `passive: true`',
type: 'CallExpression'
}
]
}
]
})