Skip to content

Commit 27645f3

Browse files
committed
Add avoidEmptyEffect
1 parent 2c9e00d commit 27645f3

File tree

4 files changed

+44
-10
lines changed

4 files changed

+44
-10
lines changed

src/messages.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
export const messageIds = {
2+
avoidEmptyEffect: "avoidEmptyEffect",
23
avoidDerivedState: "avoidDerivedState",
34
avoidInitializingState: "avoidInitializingState",
45
avoidChainingState: "avoidChainingState",
@@ -15,6 +16,8 @@ export const messageIds = {
1516

1617
// TODO: Could include more info in messages, like the relevant node
1718
export const messages = {
19+
[messageIds.avoidEmptyEffect]:
20+
'This effect is empty and could be removed.',
1821
[messageIds.avoidDerivedState]:
1922
'Avoid storing derived state. Compute "{{state}}" directly during render, optionally with `useMemo` if it\'s expensive.',
2023
[messageIds.avoidInitializingState]:

src/rule.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,18 @@ export const rule = {
3838
const effectFnRefs = getEffectFnRefs(context, node);
3939
const depsRefs = getDependenciesRefs(context, node);
4040

41-
if (!effectFnRefs || effectFnRefs.length === 0 || !depsRefs) {
41+
if (!effectFnRefs || !depsRefs) {
42+
return;
43+
} else if (effectFnRefs.length === 0) {
44+
// Hopefully it's obvious the effect can be removed.
45+
// More a follow-up for once they fix/remove other issues.
46+
context.report({
47+
node,
48+
messageId: messageIds.avoidEmptyEffect,
49+
});
4250
return;
4351
}
4452

45-
4653
const propUsedToResetAllState = findPropUsedToResetAllState(
4754
context,
4855
effectFnRefs,

test/empty-effect.test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { MyRuleTester, js } from "./rule-tester.js";
2+
import { messageIds } from "../src/messages.js";
3+
4+
new MyRuleTester().run("/empty-effect", {
5+
valid: [
6+
{
7+
name: "Valid effect",
8+
code: js`
9+
function Component() {
10+
useEffect(() => {
11+
console.log("Meow");
12+
}, []);
13+
}
14+
`,
15+
}
16+
],
17+
invalid: [
18+
{
19+
name: "Empty effect",
20+
code: js`
21+
function Component() {
22+
useEffect(() => {}, []);
23+
}
24+
`,
25+
errors: [
26+
{
27+
messageId: messageIds.avoidEmptyEffect,
28+
},
29+
],
30+
},
31+
],
32+
});

test/syntax.test.js

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,6 @@ const code = ({
2727
// Could be overkill; they shouldn't affect each other (supposedly, but I guess that's the point of tests!)
2828
new MyRuleTester().run("/syntax", {
2929
valid: [
30-
{
31-
name: "Empty effect",
32-
code: js`
33-
function Component() {
34-
useEffect(() => {}, []);
35-
}
36-
`,
37-
},
3830
{
3931
name: "Two components with overlapping names",
4032
// Not a super realistic example

0 commit comments

Comments
 (0)