Skip to content

Commit 1dd0f3e

Browse files
committed
[Compiler] Change ValidateNoDerivedComputationsInEffect logic to track prop and local state derived values variables and add extra tests
Summary: Biggest change of the stack, we track how values prop and local state values are derived throughout the entire component. We are iterating over instructions instead of effects since some mutations can not be caught otherwise. For every derivation we track the type of value its coming from (props or local state) and also the top most relevant sources (These would be the ones that are actually named instead of promoted like t0) We propagate these relevant sources to each derivation. This allows us to catch more complex useEffects though right now we are overcapturing some more complex cases which will be refined further up the stack. This PR also adds a couple tests we will work towards fixing Test Plan: Added: ref-conditional-in-effect-no-error effect-contains-prop-function-call-no-error derived-state-from-ref-and-state-no-error
1 parent 1d9ccd1 commit 1dd0f3e

19 files changed

+716
-154
lines changed

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

Lines changed: 306 additions & 154 deletions
Large diffs are not rendered by default.
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validateNoDerivedComputationsInEffects_exp
6+
import {useEffect, useState} from 'react';
7+
8+
function Component({value, enabled}) {
9+
const [localValue, setLocalValue] = useState('');
10+
11+
useEffect(() => {
12+
if (enabled) {
13+
setLocalValue(value);
14+
} else {
15+
setLocalValue('disabled');
16+
}
17+
}, [value, enabled]);
18+
19+
return <div>{localValue}</div>;
20+
}
21+
22+
export const FIXTURE_ENTRYPOINT = {
23+
fn: Component,
24+
params: [{value: 'test', enabled: true}],
25+
};
26+
27+
```
28+
29+
30+
## Error
31+
32+
```
33+
Found 1 error:
34+
35+
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
36+
37+
error.derived-state-conditionally-in-effect.ts:9:6
38+
7 | useEffect(() => {
39+
8 | if (enabled) {
40+
> 9 | setLocalValue(value);
41+
| ^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
42+
10 | } else {
43+
11 | setLocalValue('disabled');
44+
12 | }
45+
```
46+
47+
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validateNoDerivedComputationsInEffects_exp
6+
import {useEffect, useState} from 'react';
7+
8+
export default function Component({input = 'empty'}) {
9+
const [currInput, setCurrInput] = useState(input);
10+
const localConst = 'local const';
11+
12+
useEffect(() => {
13+
setCurrInput(input + localConst);
14+
}, [input, localConst]);
15+
16+
return <div>{currInput}</div>;
17+
}
18+
19+
export const FIXTURE_ENTRYPOINT = {
20+
fn: Component,
21+
params: [{input: 'test'}],
22+
};
23+
24+
```
25+
26+
27+
## Error
28+
29+
```
30+
Found 1 error:
31+
32+
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
33+
34+
error.derived-state-from-default-props.ts:9:4
35+
7 |
36+
8 | useEffect(() => {
37+
> 9 | setCurrInput(input + localConst);
38+
| ^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
39+
10 | }, [input, localConst]);
40+
11 |
41+
12 | return <div>{currInput}</div>;
42+
```
43+
44+
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validateNoDerivedComputationsInEffects_exp
6+
7+
import {useEffect, useState} from 'react';
8+
9+
function Component({shouldChange}) {
10+
const [count, setCount] = useState(0);
11+
12+
useEffect(() => {
13+
if (shouldChange) {
14+
setCount(count + 1);
15+
}
16+
}, [count]);
17+
18+
return <div>{count}</div>;
19+
}
20+
21+
```
22+
23+
24+
## Error
25+
26+
```
27+
Found 1 error:
28+
29+
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
30+
31+
error.derived-state-from-local-state-in-effect.ts:10:6
32+
8 | useEffect(() => {
33+
9 | if (shouldChange) {
34+
> 10 | setCount(count + 1);
35+
| ^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
36+
11 | }
37+
12 | }, [count]);
38+
13 |
39+
```
40+
41+
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validateNoDerivedComputationsInEffects_exp
6+
import {useEffect, useState} from 'react';
7+
8+
function Component({firstName}) {
9+
const [lastName, setLastName] = useState('Doe');
10+
const [fullName, setFullName] = useState('John');
11+
12+
const middleName = 'D.';
13+
14+
useEffect(() => {
15+
setFullName(firstName + ' ' + middleName + ' ' + lastName);
16+
}, [firstName, middleName, lastName]);
17+
18+
return (
19+
<div>
20+
<input value={lastName} onChange={e => setLastName(e.target.value)} />
21+
<div>{fullName}</div>
22+
</div>
23+
);
24+
}
25+
26+
export const FIXTURE_ENTRYPOINT = {
27+
fn: Component,
28+
params: [{firstName: 'John'}],
29+
};
30+
31+
```
32+
33+
34+
## Error
35+
36+
```
37+
Found 1 error:
38+
39+
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
40+
41+
error.derived-state-from-prop-local-state-and-component-scope.ts:11:4
42+
9 |
43+
10 | useEffect(() => {
44+
> 11 | setFullName(firstName + ' ' + middleName + ' ' + lastName);
45+
| ^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
46+
12 | }, [firstName, middleName, lastName]);
47+
13 |
48+
14 | return (
49+
```
50+
51+
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validateNoDerivedComputationsInEffects_exp
6+
import {useEffect, useState} from 'react';
7+
8+
function Component({value}) {
9+
const [localValue, setLocalValue] = useState('');
10+
11+
useEffect(() => {
12+
setLocalValue(value);
13+
document.title = `Value: ${value}`;
14+
}, [value]);
15+
16+
return <div>{localValue}</div>;
17+
}
18+
19+
export const FIXTURE_ENTRYPOINT = {
20+
fn: Component,
21+
params: [{value: 'test'}],
22+
};
23+
24+
```
25+
26+
27+
## Error
28+
29+
```
30+
Found 1 error:
31+
32+
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
33+
34+
error.derived-state-from-prop-with-side-effect.ts:8:4
35+
6 |
36+
7 | useEffect(() => {
37+
> 8 | setLocalValue(value);
38+
| ^^^^^^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
39+
9 | document.title = `Value: ${value}`;
40+
10 | }, [value]);
41+
11 |
42+
```
43+
44+

0 commit comments

Comments
 (0)