Skip to content

Commit c4ebf40

Browse files
NickvanDykeNickvanDyke
committed
feat(no-pass-data-to-parent)!
#22
1 parent 08edb7c commit c4ebf40

File tree

5 files changed

+248
-3
lines changed

5 files changed

+248
-3
lines changed

‎README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ The plugin will have more information to act upon when you pass the correct depe
6262
| `no-initialize-state` | Disallow initializing state in an effect. | — | 🟡 |
6363
| `no-event-handler` | Disallow using state and an effect as an event handler. | [docs](https://react.dev/learn/you-might-not-need-an-effect#sharing-logic-between-event-handlers) | 🟡 |
6464
| `no-reset-all-state-when-a-prop-changes` | Disallow resetting all state in an effect when a prop changes. | [docs](https://react.dev/learn/you-might-not-need-an-effect#resetting-all-state-when-a-prop-changes) | 🟡 |
65-
| `no-pass-live-state-to-parent` | Disallow passing live state updates to parent components in an effect. | [docs](https://react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes) | 🟡 |
65+
| `no-pass-live-state-to-parent` | Disallow passing live state to parents in an effect. | [docs](https://react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes) | 🟡 |
66+
| `no-pass-data-to-parent` | Disallow passing data to parents in an effect. | [docs](https://react.dev/learn/you-might-not-need-an-effect#passing-data-to-the-parent) | 🟡 |
6667
| `no-manage-parent` | Disallow effects that only use props. | — | 🟡 |
6768
| `no-empty-effect` | Disallow empty effects. | — | 🟡 |
6869

‎src/index.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import * as noPassLiveStateToParent from "./no-pass-live-state-to-parent.js";
55
import * as noInitializeState from "./no-initialize-state.js";
66
import * as noChainStateUpdates from "./no-chain-state-updates.js";
77
import * as noDerivedState from "./no-derived-state.js";
8+
import * as noPassDataToParent from "./no-pass-data-to-parent.js";
89
import * as noManageParent from "./no-manage-parent.js";
910
import globals from "globals";
1011

@@ -19,10 +20,11 @@ const plugin = {
1920
noResetAllStateWhenAPropChanges.rule,
2021
[noEventHandler.name]: noEventHandler.rule,
2122
[noPassLiveStateToParent.name]: noPassLiveStateToParent.rule,
23+
[noPassDataToParent.name]: noPassDataToParent.rule,
24+
[noManageParent.name]: noManageParent.rule,
2225
[noInitializeState.name]: noInitializeState.rule,
2326
[noChainStateUpdates.name]: noChainStateUpdates.rule,
2427
[noDerivedState.name]: noDerivedState.rule,
25-
[noManageParent.name]: noManageParent.rule,
2628
},
2729
};
2830

‎src/no-pass-data-to-parent.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import {
2+
isUseEffect,
3+
getEffectFnRefs,
4+
getDependenciesRefs,
5+
isFnRef,
6+
isDirectCall,
7+
isPropCallback,
8+
isHOCProp,
9+
getUpstreamReactVariables,
10+
isState,
11+
} from "./util/react.js";
12+
import { getCallExpr, getDownstreamRefs } from "./util/ast.js";
13+
14+
export const name = "no-pass-data-to-parent";
15+
export const messages = {
16+
avoidPassingDataToParent: "avoidPassingDataToParent",
17+
};
18+
export const rule = {
19+
meta: {
20+
type: "suggestion",
21+
docs: {
22+
description: "Disallow passing data to parents in an effect.",
23+
url: "https://react.dev/learn/you-might-not-need-an-effect#passing-data-to-the-parent",
24+
},
25+
schema: [],
26+
messages: {
27+
[messages.avoidPassingDataToParent]:
28+
"Avoid passing data to parents in an effect. Instead, let the parent fetch the data itself and pass it down to the child as a prop.",
29+
},
30+
},
31+
create: (context) => ({
32+
CallExpression: (node) => {
33+
if (!isUseEffect(node)) return;
34+
const effectFnRefs = getEffectFnRefs(context, node);
35+
const depsRefs = getDependenciesRefs(context, node);
36+
if (!effectFnRefs || !depsRefs) return;
37+
38+
effectFnRefs
39+
.filter(isFnRef)
40+
.filter((ref) => isDirectCall(ref.identifier))
41+
.filter(
42+
(ref) => isPropCallback(context, ref) && !isHOCProp(ref.resolved),
43+
)
44+
.forEach((ref) => {
45+
const callExpr = getCallExpr(ref);
46+
const argsUpstreamVariables = callExpr.arguments
47+
.flatMap((arg) => getDownstreamRefs(context, arg))
48+
.flatMap((ref) =>
49+
getUpstreamReactVariables(context, ref.identifier),
50+
);
51+
52+
if (
53+
callExpr.arguments.some((arg) => arg.type === "Literal") ||
54+
argsUpstreamVariables.some((variable) => !isState(variable))
55+
) {
56+
context.report({
57+
node: callExpr,
58+
messageId: messages.avoidPassingDataToParent,
59+
});
60+
}
61+
});
62+
},
63+
}),
64+
};

‎test/no-pass-data-to-parent.test.js

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
import { MyRuleTester, js } from "./rule-tester.js";
2+
import { name, messages, rule } from "../src/no-pass-data-to-parent.js";
3+
4+
new MyRuleTester().run(name, rule, {
5+
valid: [
6+
{
7+
name: "Pass live internal state",
8+
code: js`
9+
const Child = ({ onTextChanged }) => {
10+
const [text, setText] = useState();
11+
12+
useEffect(() => {
13+
onTextChanged(text);
14+
}, [onTextChanged, text]);
15+
16+
return (
17+
<input
18+
type="text"
19+
onChange={(e) => setText(e.target.value)}
20+
/>
21+
);
22+
}
23+
`,
24+
},
25+
{
26+
name: "No-arg prop callback",
27+
code: js`
28+
function Form({ onClose }) {
29+
const [name, setName] = useState();
30+
const [isOpen, setIsOpen] = useState(true);
31+
32+
useEffect(() => {
33+
if (!isOpen) {
34+
onClose();
35+
}
36+
}, [isOpen]);
37+
38+
return (
39+
<button onClick={() => setIsOpen(false)}>Close</button>
40+
)
41+
}
42+
`,
43+
errors: [
44+
{
45+
messageId: messages.avoidPassingDataToParent,
46+
},
47+
],
48+
},
49+
{
50+
// This might be an anti-pattern in the first place...
51+
name: "Prop getter",
52+
code: js`
53+
function Child({ getData }) {
54+
useEffect(() => {
55+
console.log(getData());
56+
}, [getData]);
57+
}
58+
`,
59+
},
60+
{
61+
name: "Pass internal state to HOC prop",
62+
code: js`
63+
import { withRouter } from 'react-router-dom';
64+
65+
const MyComponent = withRouter(({ history }) => {
66+
const [option, setOption] = useState();
67+
68+
useEffect(() => {
69+
history.push(option);
70+
}, [option]);
71+
});
72+
`,
73+
},
74+
{
75+
name: "Pass external state to HOC prop",
76+
code: js`
77+
import { withRouter } from 'react-router-dom';
78+
79+
const MyComponent = withRouter(({ history }) => {
80+
const data = useSomeAPI();
81+
82+
useEffect(() => {
83+
if (data.error) {
84+
history.push(data.error);
85+
}
86+
}, [data]);
87+
});
88+
`,
89+
},
90+
],
91+
invalid: [
92+
{
93+
name: "Pass literal value to prop",
94+
code: js`
95+
const Child = ({ onTextChanged }) => {
96+
useEffect(() => {
97+
onTextChanged("Hello World");
98+
}, [onTextChanged]);
99+
}
100+
`,
101+
errors: [
102+
{
103+
messageId: messages.avoidPassingDataToParent,
104+
},
105+
],
106+
},
107+
{
108+
name: "Pass external state",
109+
code: js`
110+
const Child = ({ onFetched }) => {
111+
const data = useSomeAPI();
112+
113+
useEffect(() => {
114+
onFetched(data);
115+
}, [onFetched, data]);
116+
}
117+
`,
118+
errors: [
119+
{
120+
messageId: messages.avoidPassingDataToParent,
121+
},
122+
],
123+
},
124+
{
125+
name: "Pass derived external state",
126+
code: js`
127+
const Child = ({ onFetched }) => {
128+
const data = useSomeAPI();
129+
const firstElement = data[0];
130+
131+
useEffect(() => {
132+
onFetched(firstElement);
133+
}, [onFetched, firstElement]);
134+
}
135+
`,
136+
errors: [
137+
{
138+
messageId: messages.avoidPassingDataToParent,
139+
},
140+
],
141+
},
142+
{
143+
name: "Pass external state that's retrieved in effect",
144+
todo: true, // TODO: We ignore the `data` variable because it's a Parameter :/
145+
code: js`
146+
const Child = ({ onFetched }) => {
147+
useEffect(() => {
148+
fetchData()
149+
.then((data) => onFetched(data));
150+
}, []);
151+
}
152+
`,
153+
errors: [
154+
{
155+
messageId: messages.avoidPassingDataToParent,
156+
},
157+
],
158+
},
159+
{
160+
name: "From props via member function",
161+
code: js`
162+
function DoubleList({ list }) {
163+
const [doubleList, setDoubleList] = useState([]);
164+
165+
useEffect(() => {
166+
setDoubleList(list.concat(list));
167+
}, [list]);
168+
}
169+
`,
170+
errors: [
171+
{
172+
// We consider `list.concat` to essentially be a prop callback
173+
messageId: messages.avoidPassingDataToParent,
174+
},
175+
],
176+
},
177+
],
178+
});

‎test/no-pass-live-state-to-parent.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ new MyRuleTester().run(name, rule, {
5151
}, [isOpen]);
5252
5353
return (
54-
<button onClick={() => setIsOpen(false)}>Submit</button>
54+
<button onClick={() => setIsOpen(false)}>Close</button>
5555
)
5656
}
5757
`,

0 commit comments

Comments
 (0)