Skip to content

Commit 7c278ae

Browse files
committed
[Compiler] Avoid capturing global setStates for no-derived-computations lint
Summary: This only matters when enableTreatSetIdentifiersAsStateSetters=true This pattern is still bad. But Right now the validation can only recommend to move stuff to "calculate in render" A global setState should not be moved to render, not even conditionally and you can't remove state without crossing Component boundaries, which makes this a different kind of fix. So while we are only suggesting "calculate in render" as a fix we should disallow the lint from throwing in this case IMO Test Plan: Added a fixture
1 parent cd1d0dc commit 7c278ae

File tree

4 files changed

+80
-1
lines changed

4 files changed

+80
-1
lines changed

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,16 @@ function validateEffect(
690690
instr.value.args.length === 1 &&
691691
instr.value.args[0].kind === 'Identifier'
692692
) {
693+
const calleeMetadata = context.derivationCache.cache.get(
694+
instr.value.callee.identifier.id,
695+
);
696+
697+
// If the setState comes from a source other than local state skip
698+
// since the fix is not to calculate in render
699+
if (calleeMetadata?.typeOfValue != 'fromState') {
700+
continue;
701+
}
702+
693703
const argMetadata = context.derivationCache.cache.get(
694704
instr.value.args[0].identifier.id,
695705
);
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validateNoDerivedComputationsInEffects_exp @enableTreatSetIdentifiersAsStateSetters
6+
7+
function Component({ setParentState, prop }) {
8+
9+
useEffect(() => {
10+
setParentState(prop)
11+
}, [prop]);
12+
13+
return (<div>{prop}</div>)
14+
}
15+
16+
```
17+
18+
## Code
19+
20+
```javascript
21+
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @enableTreatSetIdentifiersAsStateSetters
22+
23+
function Component(t0) {
24+
const $ = _c(7);
25+
const { setParentState, prop } = t0;
26+
let t1;
27+
if ($[0] !== prop || $[1] !== setParentState) {
28+
t1 = () => {
29+
setParentState(prop);
30+
};
31+
$[0] = prop;
32+
$[1] = setParentState;
33+
$[2] = t1;
34+
} else {
35+
t1 = $[2];
36+
}
37+
let t2;
38+
if ($[3] !== prop) {
39+
t2 = [prop];
40+
$[3] = prop;
41+
$[4] = t2;
42+
} else {
43+
t2 = $[4];
44+
}
45+
useEffect(t1, t2);
46+
let t3;
47+
if ($[5] !== prop) {
48+
t3 = <div>{prop}</div>;
49+
$[5] = prop;
50+
$[6] = t3;
51+
} else {
52+
t3 = $[6];
53+
}
54+
return t3;
55+
}
56+
57+
```
58+
59+
### Eval output
60+
(kind: exception) Fixture not implemented
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// @validateNoDerivedComputationsInEffects_exp @enableTreatSetIdentifiersAsStateSetters @loggerTestOnly
2+
3+
function Component({ setParentState, prop }) {
4+
5+
useEffect(() => {
6+
setParentState(prop)
7+
}, [prop]);
8+
9+
return (<div>{prop}</div>)
10+
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/usestate-derived-from-prop-no-show-in-data-flow-tree.expect.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ function Component(t0) {
6464
## Logs
6565

6666
```
67-
{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nState: [second]\n\nData Flow Tree:\n└── second (State)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":14,"column":4,"index":443},"end":{"line":14,"column":8,"index":447},"filename":"usestate-derived-from-prop-no-show-in-data-flow-tree.ts","identifierName":"setS"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
6867
{"kind":"CompileSuccess","fnLoc":{"start":{"line":3,"column":0,"index":64},"end":{"line":18,"column":1,"index":500},"filename":"usestate-derived-from-prop-no-show-in-data-flow-tree.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
6968
```
7069

0 commit comments

Comments
 (0)