Skip to content

Commit 672e839

Browse files
NickvanDykeNickvanDyke
committed
feat(no-pass-data-to-parent)!: add rule
#22
1 parent 565460e commit 672e839

File tree

5 files changed

+269
-3
lines changed

5 files changed

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

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

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
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+
// TODO: Definitely an anti-pattern, but may fit better elsewhere
92+
// because refs are not quite state, e.g. don't cause re-renders.
93+
// However they *are* local to the component...
94+
// At the least, could use a different message when flagged.
95+
name: "Pass ref to parent",
96+
code: js`
97+
const Child = ({ onRef }) => {
98+
const ref = useRef();
99+
100+
useEffect(() => {
101+
onRef(ref);
102+
}, [onRef, ref]);
103+
104+
return <div ref={ref}>Child</div>;
105+
}
106+
`,
107+
},
108+
],
109+
invalid: [
110+
{
111+
name: "Pass literal value to prop",
112+
code: js`
113+
const Child = ({ onTextChanged }) => {
114+
useEffect(() => {
115+
onTextChanged("Hello World");
116+
}, [onTextChanged]);
117+
}
118+
`,
119+
errors: [
120+
{
121+
messageId: messages.avoidPassingDataToParent,
122+
},
123+
],
124+
},
125+
{
126+
name: "Pass external state",
127+
code: js`
128+
const Child = ({ onFetched }) => {
129+
const data = useSomeAPI();
130+
131+
useEffect(() => {
132+
onFetched(data);
133+
}, [onFetched, data]);
134+
}
135+
`,
136+
errors: [
137+
{
138+
messageId: messages.avoidPassingDataToParent,
139+
},
140+
],
141+
},
142+
{
143+
name: "Pass derived external state",
144+
code: js`
145+
const Child = ({ onFetched }) => {
146+
const data = useSomeAPI();
147+
const firstElement = data[0];
148+
149+
useEffect(() => {
150+
onFetched(firstElement);
151+
}, [onFetched, firstElement]);
152+
}
153+
`,
154+
errors: [
155+
{
156+
messageId: messages.avoidPassingDataToParent,
157+
},
158+
],
159+
},
160+
{
161+
name: "Pass external state that's retrieved in effect",
162+
todo: true, // TODO: We ignore the `data` variable because it's a Parameter :/
163+
code: js`
164+
const Child = ({ onFetched }) => {
165+
useEffect(() => {
166+
fetchData()
167+
.then((data) => onFetched(data));
168+
}, []);
169+
}
170+
`,
171+
errors: [
172+
{
173+
messageId: messages.avoidPassingDataToParent,
174+
},
175+
],
176+
},
177+
{
178+
name: "From props via member function",
179+
code: js`
180+
function DoubleList({ list }) {
181+
const [doubleList, setDoubleList] = useState([]);
182+
183+
useEffect(() => {
184+
setDoubleList(list.concat(list));
185+
}, [list]);
186+
}
187+
`,
188+
errors: [
189+
{
190+
// We consider `list.concat` to essentially be a prop callback
191+
messageId: messages.avoidPassingDataToParent,
192+
},
193+
],
194+
},
195+
],
196+
});

‎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)