Skip to content

Commit f0ff04d

Browse files
bschlenkfiskersindresorhus
authored
Add no-invalid-remove-event-listener rule (#1216)
Co-authored-by: fisker Cheung <[email protected]> Co-authored-by: Sindre Sorhus <[email protected]>
1 parent 68786b8 commit f0ff04d

File tree

7 files changed

+271
-0
lines changed

7 files changed

+271
-0
lines changed

‎docs/rules/no-invalid-remove-event-listener.md‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# Prevent calling `EventTarget#removeEventListener()` with the result of an expression
2+
3+
The [`removeEventListener`](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener) function must be called with a reference to the same function that was passed to [`addEventListener`](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener). Calling `removeEventListener` with an inline function or the result of an inline `.bind()` call is indicative of an error, and won't actually remove the listener.
4+
5+
## Fail
6+
7+
```js
8+
window.removeEventListener('click', fn.bind(window));
9+
```
10+
11+
```js
12+
window.removeEventListener('click', () => {});
13+
```
14+
15+
```js
16+
window.removeEventListener('click', function () {});
17+
```
18+
19+
```js
20+
class MyElement extends HTMLElement {
21+
handler() {}
22+
23+
disconnectedCallback() {
24+
this.removeEventListener('click', this.handler.bind(this));
25+
}
26+
}
27+
```
28+
29+
## Pass
30+
31+
```js
32+
window.removeEventListener('click', listener);
33+
```
34+
35+
```js
36+
window.removeEventListener('click', getListener());
37+
```
38+
39+
```js
40+
class MyElement extends HTMLElement {
41+
constructor() {
42+
super();
43+
this.handler = this.handler.bind(this);
44+
}
45+
46+
handler() {}
47+
48+
disconnectedCallback() {
49+
this.removeEventListener('click', this.handler);
50+
}
51+
}
52+
```

‎index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ module.exports = {
6363
'unicorn/no-for-loop': 'error',
6464
'unicorn/no-hex-escape': 'error',
6565
'unicorn/no-instanceof-array': 'error',
66+
'unicorn/no-invalid-remove-event-listener': 'error',
6667
'unicorn/no-keyword-prefix': 'off',
6768
'unicorn/no-lonely-if': 'error',
6869
'no-nested-ternary': 'off',

‎readme.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ Configure it in `package.json`.
6060
"unicorn/no-for-loop": "error",
6161
"unicorn/no-hex-escape": "error",
6262
"unicorn/no-instanceof-array": "error",
63+
"unicorn/no-invalid-remove-event-listener": "error",
6364
"unicorn/no-keyword-prefix": "off",
6465
"unicorn/no-lonely-if": "error",
6566
"no-nested-ternary": "off",
@@ -179,6 +180,7 @@ Each rule has emojis denoting:
179180
| [no-for-loop](docs/rules/no-for-loop.md) | Do not use a `for` loop that can be replaced with a `for-of` loop. | ✅ | 🔧 | |
180181
| [no-hex-escape](docs/rules/no-hex-escape.md) | Enforce the use of Unicode escapes instead of hexadecimal escapes. | ✅ | 🔧 | |
181182
| [no-instanceof-array](docs/rules/no-instanceof-array.md) | Require `Array.isArray()` instead of `instanceof Array`. | ✅ | 🔧 | |
183+
| [no-invalid-remove-event-listener](docs/rules/no-invalid-remove-event-listener.md) | Prevent calling `EventTarget#removeEventListener()` with the result of an expression. | ✅ | | |
182184
| [no-keyword-prefix](docs/rules/no-keyword-prefix.md) | Disallow identifiers starting with `new` or `class`. | | | |
183185
| [no-lonely-if](docs/rules/no-lonely-if.md) | Disallow `if` statements as the only statement in `if` blocks without `else`. | ✅ | 🔧 | |
184186
| [no-nested-ternary](docs/rules/no-nested-ternary.md) | Disallow nested ternary expressions. | ✅ | 🔧 | |

‎rules/no-invalid-remove-event-listener.js‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
'use strict';
2+
const {getFunctionHeadLocation} = require('eslint-utils');
3+
const getDocumentationUrl = require('./utils/get-documentation-url.js');
4+
const {methodCallSelector, matches} = require('./selectors/index.js');
5+
6+
const MESSAGE_ID = 'no-invalid-remove-event-listener';
7+
const messages = {
8+
[MESSAGE_ID]: 'The listener argument should be a function reference.',
9+
};
10+
11+
const removeEventListenerSelector = [
12+
methodCallSelector({
13+
method: 'removeEventListener',
14+
minimumArguments: 2,
15+
}),
16+
'[arguments.0.type!="SpreadElement"]',
17+
matches([
18+
'[arguments.1.type="FunctionExpression"]',
19+
'[arguments.1.type="ArrowFunctionExpression"]',
20+
methodCallSelector({method: 'bind', path: 'arguments.1'}),
21+
]),
22+
].join('');
23+
24+
/** @param {import('eslint').Rule.RuleContext} context */
25+
const create = context => {
26+
return {
27+
[removeEventListenerSelector]: node => {
28+
const listener = node.arguments[1];
29+
if (['ArrowFunctionExpression', 'FunctionExpression'].includes(listener.type)) {
30+
return {
31+
node: listener,
32+
loc: getFunctionHeadLocation(listener, context.getSourceCode()),
33+
messageId: MESSAGE_ID,
34+
};
35+
}
36+
37+
return {
38+
node: listener.callee.property,
39+
messageId: MESSAGE_ID,
40+
};
41+
},
42+
};
43+
};
44+
45+
module.exports = {
46+
create,
47+
meta: {
48+
type: 'problem',
49+
docs: {
50+
description: 'Prevent calling `EventTarget#removeEventListener()` with the result of an expression.',
51+
url: getDocumentationUrl(__filename),
52+
},
53+
schema: [],
54+
messages,
55+
},
56+
};

‎test/no-invalid-remove-event-listener.mjs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import outdent from 'outdent';
2+
import {getTester} from './utils/test.mjs';
3+
4+
const {test} = getTester(import.meta);
5+
6+
test.snapshot({
7+
valid: [
8+
// CallExpression
9+
'new el.removeEventListener("click", () => {})',
10+
'el?.removeEventListener("click", () => {})',
11+
'el.removeEventListener?.("click", () => {})',
12+
'el.notRemoveEventListener("click", () => {})',
13+
'el[removeEventListener]("click", () => {})',
14+
15+
// Arguments
16+
'el.removeEventListener("click")',
17+
'el.removeEventListener()',
18+
'el.removeEventListener(() => {})',
19+
'el.removeEventListener(...["click", () => {}], () => {})',
20+
'el.removeEventListener(() => {}, "click")',
21+
'window.removeEventListener("click", bind())',
22+
'window.removeEventListener("click", handler.notBind())',
23+
'window.removeEventListener("click", handler[bind]())',
24+
'window.removeEventListener("click", handler.bind?.())',
25+
'window.removeEventListener("click", handler?.bind())',
26+
27+
'window.removeEventListener(handler)',
28+
outdent`
29+
class MyComponent {
30+
handler() {}
31+
disconnectedCallback() {
32+
this.removeEventListener('click', this.handler);
33+
}
34+
}
35+
`,
36+
'this.removeEventListener("click", getListener())',
37+
'el.removeEventListener("scroll", handler)',
38+
'el.removeEventListener("keydown", obj.listener)',
39+
'removeEventListener("keyup", () => {})',
40+
'removeEventListener("keydown", function () {})',
41+
42+
],
43+
invalid: [
44+
'window.removeEventListener("scroll", handler.bind(abc))',
45+
'window.removeEventListener("scroll", this.handler.bind(abc))',
46+
'window.removeEventListener("click", () => {})',
47+
'window.removeEventListener("keydown", function () {})',
48+
'el.removeEventListener("click", (e) => { e.preventDefault(); })',
49+
'el.removeEventListener("mouseover", fn.bind(abc))',
50+
'el.removeEventListener("mouseout", function (e) {})',
51+
'el.removeEventListener("mouseout", function (e) {}, true)',
52+
'el.removeEventListener("click", function (e) {}, ...moreArguments)',
53+
'el.removeEventListener(() => {}, () => {}, () => {})',
54+
],
55+
});

‎test/snapshots/no-invalid-remove-event-listener.mjs.md‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
# Snapshot report for `test/no-invalid-remove-event-listener.mjs`
2+
3+
The actual snapshot is saved in `no-invalid-remove-event-listener.mjs.snap`.
4+
5+
Generated by [AVA](https://avajs.dev).
6+
7+
## Invalid #1
8+
1 | window.removeEventListener("scroll", handler.bind(abc))
9+
10+
> Error 1/1
11+
12+
`␊
13+
> 1 | window.removeEventListener("scroll", handler.bind(abc))␊
14+
| ^^^^ The listener argument should be a function reference.␊
15+
`
16+
17+
## Invalid #2
18+
1 | window.removeEventListener("scroll", this.handler.bind(abc))
19+
20+
> Error 1/1
21+
22+
`␊
23+
> 1 | window.removeEventListener("scroll", this.handler.bind(abc))␊
24+
| ^^^^ The listener argument should be a function reference.␊
25+
`
26+
27+
## Invalid #3
28+
1 | window.removeEventListener("click", () => {})
29+
30+
> Error 1/1
31+
32+
`␊
33+
> 1 | window.removeEventListener("click", () => {})␊
34+
| ^^ The listener argument should be a function reference.␊
35+
`
36+
37+
## Invalid #4
38+
1 | window.removeEventListener("keydown", function () {})
39+
40+
> Error 1/1
41+
42+
`␊
43+
> 1 | window.removeEventListener("keydown", function () {})␊
44+
| ^^^^^^^^^ The listener argument should be a function reference.␊
45+
`
46+
47+
## Invalid #5
48+
1 | el.removeEventListener("click", (e) => { e.preventDefault(); })
49+
50+
> Error 1/1
51+
52+
`␊
53+
> 1 | el.removeEventListener("click", (e) => { e.preventDefault(); })␊
54+
| ^^ The listener argument should be a function reference.␊
55+
`
56+
57+
## Invalid #6
58+
1 | el.removeEventListener("mouseover", fn.bind(abc))
59+
60+
> Error 1/1
61+
62+
`␊
63+
> 1 | el.removeEventListener("mouseover", fn.bind(abc))␊
64+
| ^^^^ The listener argument should be a function reference.␊
65+
`
66+
67+
## Invalid #7
68+
1 | el.removeEventListener("mouseout", function (e) {})
69+
70+
> Error 1/1
71+
72+
`␊
73+
> 1 | el.removeEventListener("mouseout", function (e) {})␊
74+
| ^^^^^^^^^ The listener argument should be a function reference.␊
75+
`
76+
77+
## Invalid #8
78+
1 | el.removeEventListener("mouseout", function (e) {}, true)
79+
80+
> Error 1/1
81+
82+
`␊
83+
> 1 | el.removeEventListener("mouseout", function (e) {}, true)␊
84+
| ^^^^^^^^^ The listener argument should be a function reference.␊
85+
`
86+
87+
## Invalid #9
88+
1 | el.removeEventListener("click", function (e) {}, ...moreArguments)
89+
90+
> Error 1/1
91+
92+
`␊
93+
> 1 | el.removeEventListener("click", function (e) {}, ...moreArguments)␊
94+
| ^^^^^^^^^ The listener argument should be a function reference.␊
95+
`
96+
97+
## Invalid #10
98+
1 | el.removeEventListener(() => {}, () => {}, () => {})
99+
100+
> Error 1/1
101+
102+
`␊
103+
> 1 | el.removeEventListener(() => {}, () => {}, () => {})␊
104+
| ^^ The listener argument should be a function reference.␊
105+
`

‎test/snapshots/no-invalid-remove-event-listener.mjs.snap‎

615 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)