Merge pull request #110 from github/addEventListener-lints

Add event listener lints

Author: keithamus

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

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

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

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'
   }
 }

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))
+            }
+          })
+        }
+      }
+    }
+  }
+}

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`
+        })
+      }
+    }
+  }
+}

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\``)
+      }
+    }
+  }
+}

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'
+        }
+      ]
+    }
+  ]
+})

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'
+        }
+      ]
+    }
+  ]
+})

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'
+        }
+      ]
+    }
+  ]
+})