Skip to content

Commit 361c22a

Browse files
committed
[compiler] Remove single line constraint and improve overall capturing logic
1 parent 3fd58cf commit 361c22a

21 files changed

+601
-560
lines changed

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

Lines changed: 296 additions & 309 deletions
Large diffs are not rendered by default.

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-conditional-no-error.expect.md

Lines changed: 0 additions & 79 deletions
This file was deleted.

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/derived-state-with-side-effects-no-error.expect.md

Lines changed: 0 additions & 74 deletions
This file was deleted.

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useEffect/error.bug-derived-state-from-mixed-deps.expect.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@ export const FIXTURE_ENTRYPOINT = {
3434
```
3535
Found 1 error:
3636
37-
Error: You may not need this effect. Values derived from 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)
37+
Error: Derive values in render, not effects.
3838
39-
This effect updates state based on other state values. Consider calculating this value directly during render.
39+
This setState() appears to derive a value from both props and local state [prefix, name]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
4040
4141
error.bug-derived-state-from-mixed-deps.ts:9:4
4242
7 |
4343
8 | useEffect(() => {
4444
> 9 | setDisplayName(prefix + name);
45-
| ^^^^^^^^^^^^^^ You may not need this effect. Values derived from 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)
45+
| ^^^^^^^^^^^^^^ This should be computed during render, not in an effect
4646
10 | }, [prefix, name]);
4747
11 |
4848
12 | return (
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validateNoDerivedComputationsInEffects
6+
import {useState, useEffect} from 'react';
7+
8+
function Component({props, number}) {
9+
const nothing = 0;
10+
const missDirection = number;
11+
const [displayValue, setDisplayValue] = useState(props.prefix + missDirection + nothing);
12+
13+
useEffect(() => {
14+
setDisplayValue(props.prefix + missDirection + nothing);
15+
}, [props.prefix, missDirection, nothing]);
16+
17+
return (
18+
<div
19+
onClick={() => {
20+
setDisplayValue('clicked');
21+
}}>
22+
{displayValue}
23+
</div>
24+
);
25+
}
26+
27+
```
28+
29+
30+
## Error
31+
32+
```
33+
Found 1 error:
34+
35+
Error: Local state shadows parent state.
36+
37+
This setState() appears to derive a value from props [props, number]. This state value shadows a value passed as a prop. Instead of shadowing the prop with local state, hoist the state to the parent component and update it there.
38+
39+
error.derived-state-from-shadowed-props.ts:10:4
40+
8 |
41+
9 | useEffect(() => {
42+
> 10 | setDisplayValue(props.prefix + missDirection + nothing);
43+
| ^^^^^^^^^^^^^^^ this setState synchronizes the state
44+
11 | }, [props.prefix, missDirection, nothing]);
45+
12 |
46+
13 | return (
47+
48+
error.derived-state-from-shadowed-props.ts:16:8
49+
14 | <div
50+
15 | onClick={() => {
51+
> 16 | setDisplayValue('clicked');
52+
| ^^^^^^^^^^^^^^^ this setState updates the shadowed state, but should call an onChange event from the parent
53+
17 | }}>
54+
18 | {displayValue}
55+
19 | </div>
56+
```
57+
58+
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// @validateNoDerivedComputationsInEffects
2+
import {useState, useEffect} from 'react';
3+
4+
function Component({props, number}) {
5+
const nothing = 0;
6+
const missDirection = number;
7+
const [displayValue, setDisplayValue] = useState(props.prefix + missDirection + nothing);
8+
9+
useEffect(() => {
10+
setDisplayValue(props.prefix + missDirection + nothing);
11+
}, [props.prefix, missDirection, nothing]);
12+
13+
return (
14+
<div
15+
onClick={() => {
16+
setDisplayValue('clicked');
17+
}}>
18+
{displayValue}
19+
</div>
20+
);
21+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @validateNoDerivedComputationsInEffects
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: Derive values in render, not effects.
36+
37+
This setState() appears to derive a value from props [value]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
38+
39+
error.derived-state-with-conditional.ts:9:6
40+
7 | useEffect(() => {
41+
8 | if (enabled) {
42+
> 9 | setLocalValue(value);
43+
| ^^^^^^^^^^^^^ This should be computed during render, not in an effect
44+
10 | } else {
45+
11 | setLocalValue('disabled');
46+
12 | }
47+
```
48+
49+
File renamed without changes.
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
6+
import {useEffect, useState} from 'react';
7+
8+
function Component({value}) {
9+
const [localValue, setLocalValue] = useState('');
10+
11+
useEffect(() => {
12+
console.log('Value changed:', value);
13+
setLocalValue(value);
14+
document.title = `Value: ${value}`;
15+
}, [value]);
16+
17+
return <div>{localValue}</div>;
18+
}
19+
20+
export const FIXTURE_ENTRYPOINT = {
21+
fn: Component,
22+
params: [{value: 'test'}],
23+
};
24+
25+
```
26+
27+
28+
## Error
29+
30+
```
31+
Found 1 error:
32+
33+
Error: Derive values in render, not effects.
34+
35+
This setState() appears to derive a value from props [value]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
36+
37+
error.derived-state-with-side-effects.ts:9:4
38+
7 | useEffect(() => {
39+
8 | console.log('Value changed:', value);
40+
> 9 | setLocalValue(value);
41+
| ^^^^^^^^^^^^^ This should be computed during render, not in an effect
42+
10 | document.title = `Value: ${value}`;
43+
11 | }, [value]);
44+
12 |
45+
```
46+
47+
File renamed without changes.

0 commit comments

Comments
 (0)