Skip to content

Commit a1e60dc

Browse files
AndreaPontrandolfoAndreaPontrandolfo
committed
feat: added rule ensure-matching-remove-event-listener
1 parent b94cf05 commit a1e60dc

File tree

3 files changed

+310
-0
lines changed

3 files changed

+310
-0
lines changed

‎docs/rules/ensure-matching-remove-event-listener.md

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# Enforces that every addEventListener should have a matching removeEventListener in the return statement of the same useEffect block (ensure-matching-remove-event-listener)
2+
3+
When adding "eventListeners" at the mounting phase of a component, sometimes developers forget to remove the "eventListener" in the cleanup function of the useEffect block. This can cause memory leaks in the react application:
4+
5+
> **Console error**: Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
6+
7+
## Rule Details
8+
9+
This rule aims at reminding developers to add the corrisponding removeEventListener in the useEffect block of React components.
10+
11+
Examples of **incorrect** code for this rule:
12+
13+
```js
14+
15+
// Missing a matching removeEventListener.
16+
17+
useEffect(() => {
18+
window.addEventListener("keydown", handleUserKeyPress);
19+
return () => {
20+
doThis();
21+
};
22+
}, [])
23+
24+
```
25+
26+
```js
27+
28+
// Missing a cleanup function for the addEventListener.
29+
30+
useEffect(() => {
31+
window.addEventListener("keydown", handleUserKeyPress);
32+
}, [])
33+
34+
```
35+
36+
Examples of **correct** code for this rule:
37+
38+
```js
39+
40+
useEffect(() => {
41+
window.addEventListener("keydown", handleUserKeyPress);
42+
return () => {
43+
window.removeEventListener("keydown", handleUserKeyPress);
44+
};
45+
}, []);
46+
47+
```
48+
49+
## When Not To Use It
50+
51+
You don't need this rules if you want to allow developers to not remove eventListeners added in the DOM.
52+

‎lib/rules/ensure-matching-remove-event-listener.js

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/**
2+
* @fileoverview every addEventListener should have a matching removeEventListener in the return statement of the same useEffect block
3+
* @author AndreaPontrandolfo
4+
*/
5+
6+
'use strict';
7+
8+
const Components = require('../util/Components');
9+
const docsUrl = require('../util/docsUrl');
10+
const report = require('../util/report');
11+
12+
//------------------------------------------------------------------------------
13+
// Rule Definition
14+
//------------------------------------------------------------------------------
15+
16+
const messages = {
17+
requiredCleanup: 'Missing a cleanup function for the addEventListener.',
18+
requiredRemoveEventListener: 'Missing a matching removeEventListener.',
19+
};
20+
21+
module.exports = {
22+
meta: {
23+
type: 'problem',
24+
docs: {
25+
description:
26+
'Enforces that every addEventListener has a matching removeEventListener in the return statement of the same useEffect block',
27+
category: 'Possible Errors',
28+
recommended: false,
29+
url: docsUrl('ensure-matching-remove-event-listener'),
30+
},
31+
messages,
32+
fixable: null, // Or `code` or `whitespace`
33+
schema: [], // Add a schema if the rule has options
34+
},
35+
36+
create: Components.detect((context) => ({
37+
ExpressionStatement(node) {
38+
let hasAddEventListener = null;
39+
let hasReturnStatement = null;
40+
let hasRemoveEventListener = null;
41+
const expression = node && node.expression;
42+
const calleeName = expression && expression.callee && expression.callee.name;
43+
if (calleeName === 'useEffect') {
44+
const internalExpressions = expression
45+
&& expression.arguments
46+
&& expression.arguments.length > 0
47+
&& expression.arguments[0].body
48+
&& expression.arguments[0].body.body;
49+
if (internalExpressions && internalExpressions.length > 0) {
50+
internalExpressions.every((element) => {
51+
if (hasRemoveEventListener) {
52+
return false;
53+
}
54+
const elementType = element.type;
55+
const internalExpression = element.expression;
56+
const internalExpressionCallee = internalExpression && internalExpression.callee;
57+
const internalExpressionCalleeProperty = internalExpressionCallee
58+
&& internalExpressionCallee.property
59+
&& internalExpressionCallee.property.name;
60+
if (internalExpressionCalleeProperty === 'addEventListener') {
61+
hasAddEventListener = true;
62+
return true;
63+
}
64+
if (hasAddEventListener) {
65+
if (elementType === 'ReturnStatement') {
66+
hasReturnStatement = true;
67+
const returnBlockBody = element.argument
68+
&& element.argument.body
69+
&& element.argument.body.body;
70+
if (returnBlockBody && returnBlockBody.length > 0) {
71+
returnBlockBody.every((returnElement) => {
72+
if (hasRemoveEventListener) {
73+
return false;
74+
}
75+
const returnElementCallee = returnElement.expression
76+
&& returnElement.expression.callee;
77+
const returnElementCalleeObject = returnElementCallee
78+
&& returnElementCallee.object
79+
&& returnElementCallee.object.name;
80+
const returnElementCalleeProperty = returnElementCallee
81+
&& returnElementCallee.property
82+
&& returnElementCallee.property.name;
83+
if (
84+
returnElementCalleeObject === 'window'
85+
&& returnElementCalleeProperty === 'removeEventListener'
86+
) {
87+
hasRemoveEventListener = true;
88+
}
89+
return true;
90+
});
91+
}
92+
}
93+
}
94+
return true;
95+
});
96+
}
97+
if (hasAddEventListener) {
98+
if (!hasRemoveEventListener) {
99+
if (!hasReturnStatement) {
100+
const messageId = 'requiredCleanup';
101+
102+
report(context, messages, messageId, {
103+
node,
104+
});
105+
} else {
106+
const messageId = 'requiredRemoveEventListener';
107+
108+
report(context, messages, messageId, {
109+
node,
110+
});
111+
}
112+
}
113+
}
114+
}
115+
},
116+
})),
117+
};

‎tests/lib/rules/ensure-matching-remove-event-listener.js

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/**
2+
* @fileoverview every addEventListener should have a matching removeEventListener in the return statement of the same useEffect block
3+
* @author AndreaPontrandolfo
4+
*/
5+
6+
'use strict';
7+
8+
//------------------------------------------------------------------------------
9+
// Requirements
10+
//------------------------------------------------------------------------------
11+
12+
const RuleTester = require('eslint').RuleTester;
13+
const rule = require('../../../lib/rules/ensure-matching-remove-event-listener');
14+
15+
const parserOptions = {
16+
ecmaVersion: 6,
17+
sourceType: 'module',
18+
ecmaFeatures: {
19+
jsx: true,
20+
},
21+
};
22+
23+
//------------------------------------------------------------------------------
24+
// Tests
25+
//------------------------------------------------------------------------------
26+
27+
const ruleTester = new RuleTester({ parserOptions });
28+
ruleTester.run('ensure-matching-remove-event-listener', rule, {
29+
valid: [
30+
`useEffect(() => {
31+
doThis();
32+
doMoreOfThis();
33+
window.addEventListener("keydown", handleUserKeyPress);
34+
doOtherStuff();
35+
doSomeOtherStuff();
36+
return () => {
37+
doThatBefore();
38+
doMoreOfThatBefore();
39+
window.removeEventListener("keydown", handleUserKeyPress);
40+
doThatAfter();
41+
doMoreOfThatAfter();
42+
};
43+
}, [])`,
44+
`useEffect(() => {
45+
refcurrent = value;
46+
}, [value]);`,
47+
],
48+
49+
invalid: [
50+
{
51+
code: `useEffect(() => {
52+
doThis();
53+
doMoreOfThis();
54+
window.addEventListener("keydown", handleUserKeyPress);
55+
doOtherStuff();
56+
doSomeOtherStuff();
57+
}, [])`,
58+
errors: [
59+
{
60+
messageId: 'required-cleanup',
61+
type: 'ExpressionStatement',
62+
},
63+
],
64+
},
65+
{
66+
code: `useEffect(() => {
67+
doThis();
68+
doMoreOfThis();
69+
const content = window;
70+
content.addEventListener("keydown", handleUserKeyPress);
71+
doOtherStuff();
72+
doSomeOtherStuff();
73+
}, [])`,
74+
errors: [
75+
{
76+
messageId: 'required-cleanup',
77+
type: 'ExpressionStatement',
78+
},
79+
],
80+
},
81+
{
82+
code: `useEffect(() => {
83+
doThis();
84+
doMoreOfThis();
85+
window.addEventListener("keydown", handleUserKeyPress);
86+
doOtherStuff();
87+
doSomeOtherStuff();
88+
return () => {
89+
doThat();
90+
doMoreOfThat();
91+
};
92+
}, [])`,
93+
errors: [
94+
{
95+
messageId: 'required-remove-eventListener',
96+
type: 'ExpressionStatement',
97+
},
98+
],
99+
},
100+
{
101+
code: `useEffect(() => {
102+
doThis();
103+
doMoreOfThis();
104+
const content = window;
105+
content.addEventListener("keydown", handleUserKeyPress);
106+
doOtherStuff();
107+
doSomeOtherStuff();
108+
return () => {
109+
doThat();
110+
doMoreOfThat();
111+
};
112+
}, [])`,
113+
errors: [
114+
{
115+
messageId: 'required-remove-eventListener',
116+
type: 'ExpressionStatement',
117+
},
118+
],
119+
},
120+
{
121+
code: `useEffect(() => {
122+
doThis();
123+
doMoreOfThis();
124+
window.addEventListener("keydown", handleUserKeyPress);
125+
doOtherStuff();
126+
doSomeOtherStuff();
127+
return () => {
128+
doThat();
129+
window.addEventListener("keydown", handleUserKeyPress);
130+
doMoreOfThat();
131+
};
132+
}, [])`,
133+
errors: [
134+
{
135+
messageId: 'required-remove-eventListener',
136+
type: 'ExpressionStatement',
137+
},
138+
],
139+
},
140+
],
141+
});

0 commit comments

Comments
 (0)