Skip to content

Commit 2c9e00d

Browse files
NickvanDykeNickvanDyke
committed
Remove avoidInternalEffect
1 parent 1b8db6a commit 2c9e00d

File tree

9 files changed

+16
-164
lines changed

9 files changed

+16
-164
lines changed

‎src/messages.js‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
export const messageIds = {
2-
avoidInternalEffect: "avoidInternalEffect",
32
avoidDerivedState: "avoidDerivedState",
43
avoidInitializingState: "avoidInitializingState",
54
avoidChainingState: "avoidChainingState",
@@ -16,8 +15,6 @@ export const messageIds = {
1615

1716
// TODO: Could include more info in messages, like the relevant node
1817
export const messages = {
19-
[messageIds.avoidInternalEffect]:
20-
"This effect operates entirely on internal React state, with no external dependencies. It is likely unnecessary.",
2118
[messageIds.avoidDerivedState]:
2219
'Avoid storing derived state. Compute "{{state}}" directly during render, optionally with `useMemo` if it\'s expensive.',
2320
[messageIds.avoidInitializingState]:

‎src/rule.js‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -42,27 +42,6 @@ export const rule = {
4242
return;
4343
}
4444

45-
// TODO: Just remove? This is never reported in isolation afaik.
46-
// Could be different in crazy, large real-world effects though.
47-
// And presumably once the user fixes the more specific issue, they'll see the effect is empty and delete it.
48-
const isInternalEffect = effectFnRefs
49-
// Only functions because they actually have effects.
50-
// Notably this also filters out refs that are local parameters, like `items` in `list.filter((item) => ...)`.
51-
.filter((ref) => isFnRef(ref))
52-
.filter((ref) => isDirectCall(getEffectFn(node), ref))
53-
.concat(depsRefs)
54-
.flatMap((ref) => getUpstreamReactVariables(context, ref.identifier))
55-
.notEmptyEvery(
56-
(variable) =>
57-
isState(variable) || (isProp(variable) && !isHOCProp(variable)),
58-
);
59-
60-
if (isInternalEffect) {
61-
context.report({
62-
node,
63-
messageId: messageIds.avoidInternalEffect,
64-
});
65-
}
6645

6746
const propUsedToResetAllState = findPropUsedToResetAllState(
6847
context,

‎test/chaining-state.test.js‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@ new MyRuleTester().run("/chaining-state", {
2828
}
2929
`,
3030
errors: [
31-
{
32-
messageId: messageIds.avoidInternalEffect,
33-
},
3431
{
3532
messageId: messageIds.avoidChainingState,
3633
},
@@ -51,9 +48,6 @@ new MyRuleTester().run("/chaining-state", {
5148
}
5249
`,
5350
errors: [
54-
{
55-
messageId: messageIds.avoidInternalEffect,
56-
},
5751
{
5852
messageId: messageIds.avoidChainingState,
5953
},

‎test/deriving-state.test.js‎

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -289,9 +289,6 @@ new MyRuleTester().run("/deriving-state", {
289289
}
290290
`,
291291
errors: [
292-
{
293-
messageId: messageIds.avoidInternalEffect,
294-
},
295292
{
296293
messageId: messageIds.avoidDerivedState,
297294
data: { state: "fullName" },
@@ -313,9 +310,6 @@ new MyRuleTester().run("/deriving-state", {
313310
}
314311
`,
315312
errors: [
316-
{
317-
messageId: messageIds.avoidInternalEffect,
318-
},
319313
{
320314
messageId: messageIds.avoidDerivedState,
321315
data: { state: "fullName" },
@@ -337,9 +331,6 @@ new MyRuleTester().run("/deriving-state", {
337331
}
338332
`,
339333
errors: [
340-
{
341-
messageId: messageIds.avoidInternalEffect,
342-
},
343334
{
344335
messageId: messageIds.avoidDerivedState,
345336
data: { state: "fullName" },
@@ -358,9 +349,6 @@ new MyRuleTester().run("/deriving-state", {
358349
}
359350
`,
360351
errors: [
361-
{
362-
messageId: messageIds.avoidInternalEffect,
363-
},
364352
{
365353
messageId: messageIds.avoidDerivedState,
366354
data: { state: "fullName" },
@@ -380,9 +368,6 @@ new MyRuleTester().run("/deriving-state", {
380368
}
381369
`,
382370
errors: [
383-
{
384-
messageId: messageIds.avoidInternalEffect,
385-
},
386371
{
387372
messageId: messageIds.avoidDerivedState,
388373
data: { state: "fullName" },
@@ -401,9 +386,6 @@ new MyRuleTester().run("/deriving-state", {
401386
}
402387
`,
403388
errors: [
404-
{
405-
messageId: messageIds.avoidInternalEffect,
406-
},
407389
{
408390
messageId: messageIds.avoidDerivedState,
409391
data: { state: "doubleList" },
@@ -427,9 +409,6 @@ new MyRuleTester().run("/deriving-state", {
427409
}
428410
`,
429411
errors: [
430-
{
431-
messageId: messageIds.avoidInternalEffect,
432-
},
433412
{
434413
messageId: messageIds.avoidDerivedState,
435414
data: { state: "doubleList" },
@@ -454,9 +433,6 @@ new MyRuleTester().run("/deriving-state", {
454433
}
455434
`,
456435
errors: [
457-
{
458-
messageId: messageIds.avoidInternalEffect,
459-
},
460436
{
461437
// NOTE: We consider `doubleList.push` to essentially be a state setter call
462438
messageId: messageIds.avoidDerivedState,
@@ -545,9 +521,6 @@ new MyRuleTester().run("/deriving-state", {
545521
}
546522
`,
547523
errors: [
548-
{
549-
messageId: messageIds.avoidInternalEffect,
550-
},
551524
{
552525
messageId: messageIds.avoidDerivedState,
553526
data: { state: "total" },
@@ -568,9 +541,6 @@ new MyRuleTester().run("/deriving-state", {
568541
}
569542
`,
570543
errors: [
571-
{
572-
messageId: messageIds.avoidInternalEffect,
573-
},
574544
{
575545
messageId: messageIds.avoidDerivedState,
576546
data: { state: "doubleCount" },
@@ -595,9 +565,6 @@ new MyRuleTester().run("/deriving-state", {
595565
}
596566
`,
597567
errors: [
598-
{
599-
messageId: messageIds.avoidInternalEffect,
600-
},
601568
{
602569
messageId: messageIds.avoidDerivedState,
603570
data: { state: "formData" },
@@ -622,9 +589,6 @@ new MyRuleTester().run("/deriving-state", {
622589
}
623590
`,
624591
errors: [
625-
{
626-
messageId: messageIds.avoidInternalEffect,
627-
},
628592
{
629593
messageId: messageIds.avoidDerivedState,
630594
data: { state: "formData" },
@@ -651,9 +615,6 @@ new MyRuleTester().run("/deriving-state", {
651615
}
652616
`,
653617
errors: [
654-
{
655-
messageId: messageIds.avoidInternalEffect,
656-
},
657618
{
658619
messageId: messageIds.avoidDerivedState,
659620
data: { state: "formData" },

‎test/initializing-state.test.js‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@ new MyRuleTester().run("/initializing-state", {
3333
}
3434
`,
3535
errors: [
36-
{
37-
messageId: messageIds.avoidInternalEffect,
38-
},
3936
{
4037
messageId: messageIds.avoidInitializingState,
4138
data: { state: "state" },
@@ -55,9 +52,6 @@ new MyRuleTester().run("/initializing-state", {
5552
}
5653
`,
5754
errors: [
58-
{
59-
messageId: messageIds.avoidInternalEffect,
60-
},
6155
{
6256
messageId: messageIds.avoidInitializingState,
6357
data: { state: "state" },

‎test/parent-child-coupling.test.js‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,6 @@ new MyRuleTester().run("/parent-child-coupling", {
4949
}
5050
`,
5151
errors: [
52-
{
53-
messageId: messageIds.avoidInternalEffect,
54-
},
5552
{
5653
messageId: messageIds.avoidChainingState,
5754
},
@@ -69,9 +66,6 @@ new MyRuleTester().run("/parent-child-coupling", {
6966
}
7067
`,
7168
errors: [
72-
{
73-
messageId: messageIds.avoidInternalEffect,
74-
},
7569
{
7670
messageId: messageIds.avoidParentChildCoupling,
7771
},
@@ -90,9 +84,6 @@ new MyRuleTester().run("/parent-child-coupling", {
9084
}
9185
`,
9286
errors: [
93-
{
94-
messageId: messageIds.avoidInternalEffect,
95-
},
9687
{
9788
messageId: messageIds.avoidParentChildCoupling,
9889
},
@@ -112,9 +103,6 @@ new MyRuleTester().run("/parent-child-coupling", {
112103
}
113104
`,
114105
errors: [
115-
{
116-
messageId: messageIds.avoidInternalEffect,
117-
},
118106
{
119107
messageId: messageIds.avoidParentChildCoupling,
120108
},
@@ -137,9 +125,6 @@ new MyRuleTester().run("/parent-child-coupling", {
137125
}
138126
`,
139127
errors: [
140-
{
141-
messageId: messageIds.avoidInternalEffect,
142-
},
143128
{
144129
messageId: messageIds.avoidParentChildCoupling,
145130
},
@@ -204,9 +189,6 @@ new MyRuleTester().run("/parent-child-coupling", {
204189
}
205190
`,
206191
errors: [
207-
{
208-
messageId: messageIds.avoidInternalEffect,
209-
},
210192
{
211193
// TODO: Ideally we catch using state as an event handler,
212194
// but not sure how to differentiate that
@@ -229,9 +211,6 @@ new MyRuleTester().run("/parent-child-coupling", {
229211
}
230212
`,
231213
errors: [
232-
{
233-
messageId: messageIds.avoidInternalEffect,
234-
},
235214
{
236215
messageId: messageIds.avoidParentChildCoupling,
237216
},
@@ -262,9 +241,6 @@ new MyRuleTester().run("/parent-child-coupling", {
262241
}
263242
`,
264243
errors: [
265-
{
266-
messageId: messageIds.avoidInternalEffect,
267-
},
268244
{
269245
messageId: messageIds.avoidDerivedState,
270246
},

‎test/real-world.test.js‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,6 @@ new MyRuleTester().run("/real-world", {
329329
);
330330
`,
331331
errors: [
332-
{
333-
messageId: messageIds.avoidInternalEffect,
334-
},
335332
{
336333
// TODO: Because the initial state is internal, derived state would be a better flag.
337334
messageId: messageIds.avoidResettingStateFromProps,

‎test/resetting-state-from-props.test.js‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@ new MyRuleTester().run("/resetting-state-from-props", {
1616
}
1717
`,
1818
errors: [
19-
{
20-
messageId: messageIds.avoidInternalEffect,
21-
},
2219
{
2320
messageId: messageIds.avoidDerivedState,
2421
},
@@ -40,9 +37,6 @@ new MyRuleTester().run("/resetting-state-from-props", {
4037
}
4138
`,
4239
errors: [
43-
{
44-
messageId: messageIds.avoidInternalEffect,
45-
},
4640
{
4741
messageId: messageIds.avoidChainingState,
4842
},
@@ -65,9 +59,6 @@ new MyRuleTester().run("/resetting-state-from-props", {
6559
}
6660
`,
6761
errors: [
68-
{
69-
messageId: messageIds.avoidInternalEffect,
70-
},
7162
{
7263
messageId: messageIds.avoidResettingStateFromProps,
7364
data: { prop: "userId" },
@@ -89,9 +80,6 @@ new MyRuleTester().run("/resetting-state-from-props", {
8980
}
9081
`,
9182
errors: [
92-
{
93-
messageId: messageIds.avoidInternalEffect,
94-
},
9583
{
9684
messageId: messageIds.avoidResettingStateFromProps,
9785
data: { prop: "userId" },
@@ -110,9 +98,6 @@ new MyRuleTester().run("/resetting-state-from-props", {
11098
}
11199
`,
112100
errors: [
113-
{
114-
messageId: messageIds.avoidInternalEffect,
115-
},
116101
{
117102
messageId: messageIds.avoidResettingStateFromProps,
118103
// TODO: Ideally would be "user.id"
@@ -132,9 +117,6 @@ new MyRuleTester().run("/resetting-state-from-props", {
132117
}
133118
`,
134119
errors: [
135-
{
136-
messageId: messageIds.avoidInternalEffect,
137-
},
138120
{
139121
messageId: messageIds.avoidResettingStateFromProps,
140122
data: { prop: "userId" },
@@ -154,9 +136,6 @@ new MyRuleTester().run("/resetting-state-from-props", {
154136
}
155137
`,
156138
errors: [
157-
{
158-
messageId: messageIds.avoidInternalEffect,
159-
},
160139
{
161140
messageId: messageIds.avoidResettingStateFromProps,
162141
},
@@ -175,9 +154,6 @@ new MyRuleTester().run("/resetting-state-from-props", {
175154
}
176155
`,
177156
errors: [
178-
{
179-
messageId: messageIds.avoidInternalEffect,
180-
},
181157
{
182158
messageId: messageIds.avoidChainingState,
183159
},

0 commit comments

Comments
 (0)