diff --git a/docs/rules/no-useless-passive.md b/docs/rules/no-useless-passive.md new file mode 100644 index 00000000..1f8413f5 --- /dev/null +++ b/docs/rules/no-useless-passive.md @@ -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 diff --git a/docs/rules/prefer-observers.md b/docs/rules/prefer-observers.md new file mode 100644 index 00000000..7597156a --- /dev/null +++ b/docs/rules/prefer-observers.md @@ -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 diff --git a/docs/rules/require-passive-events.md b/docs/rules/require-passive-events.md new file mode 100644 index 00000000..b860e8ef --- /dev/null +++ b/docs/rules/require-passive-events.md @@ -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 diff --git a/lib/configs/browser.js b/lib/configs/browser.js index 3cf65f17..c2e5c446 100644 --- a/lib/configs/browser.js +++ b/lib/configs/browser.js @@ -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' } } diff --git a/lib/rules/no-useless-passive.js b/lib/rules/no-useless-passive.js new file mode 100644 index 00000000..f9eef57d --- /dev/null +++ b/lib/rules/no-useless-passive.js @@ -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)) + } + }) + } + } + } + } +} diff --git a/lib/rules/prefer-observers.js b/lib/rules/prefer-observers.js new file mode 100644 index 00000000..0e9824a6 --- /dev/null +++ b/lib/rules/prefer-observers.js @@ -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` + }) + } + } + } +} diff --git a/lib/rules/require-passive-events.js b/lib/rules/require-passive-events.js new file mode 100644 index 00000000..72b8a682 --- /dev/null +++ b/lib/rules/require-passive-events.js @@ -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\``) + } + } + } +} diff --git a/tests/no-useless-passive.js b/tests/no-useless-passive.js new file mode 100644 index 00000000..4ecabd3f --- /dev/null +++ b/tests/no-useless-passive.js @@ -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' + } + ] + } + ] +}) diff --git a/tests/prefer-observers.js b/tests/prefer-observers.js new file mode 100644 index 00000000..8cdba73a --- /dev/null +++ b/tests/prefer-observers.js @@ -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' + } + ] + } + ] +}) diff --git a/tests/require-passive-events.js b/tests/require-passive-events.js new file mode 100644 index 00000000..e5f3f22f --- /dev/null +++ b/tests/require-passive-events.js @@ -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' + } + ] + } + ] +})