Skip to content

Commit 6004f03

Browse files
committed
fix(no-manage-parent): don't flag when non-prop in deps
#21
1 parent b89efcd commit 6004f03

File tree

3 files changed

+28
-5
lines changed

3 files changed

+28
-5
lines changed

src/no-event-handler.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ export const rule = {
3030
const depsRefs = getDependenciesRefs(context, node);
3131
if (!effectFnRefs || !depsRefs) return;
3232

33+
// TODO: Can we also flag this when the deps are internal, and the body calls internal stuff?
34+
3335
findDownstreamNodes(context, node, "IfStatement")
3436
.filter((ifNode) => !ifNode.alternate)
3537
.filter((ifNode) =>

src/no-manage-parent.js

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { isUseEffect, getEffectFnRefs } from "./util/react.js";
1+
import {
2+
isUseEffect,
3+
getEffectFnRefs,
4+
getDependenciesRefs,
5+
} from "./util/react.js";
26
import { isProp, isHOCProp } from "./util/react.js";
37

48
export const name = "no-manage-parent";
@@ -20,13 +24,16 @@ export const rule = {
2024
create: (context) => ({
2125
CallExpression: (node) => {
2226
if (!isUseEffect(node)) return;
23-
const effectFnRefs = getEffectFnRefs(context, node) || [];
27+
const effectFnRefs = getEffectFnRefs(context, node);
28+
const depsRefs = getDependenciesRefs(context, node);
29+
if (!effectFnRefs || !depsRefs) return;
30+
2431
if (effectFnRefs.length === 0) return;
2532

2633
if (
27-
effectFnRefs.every(
28-
(ref) => isProp(ref.resolved) && !isHOCProp(ref.resolved),
29-
)
34+
effectFnRefs
35+
.concat(depsRefs)
36+
.every((ref) => isProp(ref.resolved) && !isHOCProp(ref.resolved))
3037
) {
3138
context.report({
3239
node,

test/no-manage-parent.test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,20 @@ new MyRuleTester().run(name, rule, {
5050
}
5151
`,
5252
},
53+
{
54+
// TODO: I'm pretty sure this is an anti-pattern, but I'm not totally sure how...
55+
// Should be `no-event-handler`? Because we can call `foo` wherever we call `setBar`.
56+
// `foo` is internal, so this isn't syncing to an external system.
57+
name: "Effect uses prop but deps has state",
58+
code: js`
59+
function Component({ foo }) {
60+
const [bar, setBar] = useState(0);
61+
useEffect(() => {
62+
foo;
63+
}, [bar]);
64+
}
65+
`,
66+
},
5367
],
5468
invalid: [
5569
{

0 commit comments

Comments
 (0)