Skip to content

Commit bea625b

Browse files
committed
[compiler] Add enableVerboseNoSetStateInEffect to suggest options to user/agent
The current `validateNoSetStateInEffects` error has potential false positives because we cannot fully statically detect patterns where calling setState in an effect is actually valid. This flag `enableVerboseNoSetStateInEffect` adds a verbose error mode that presents multiple possible use-cases, allowing an agent to reason about which fix is appropriate before acting: 1. Non-local derived data - suggests restructuring state ownership 2. Derived event pattern - suggests requesting an event callback from parent 3. Force update / external sync - suggests using `useSyncExternalStore` This gives agents the context needed to make informed decisions rather than blindly applying a fix that may not be correct for the specific situation.
1 parent 2cb08e6 commit bea625b

9 files changed

+385
-21
lines changed

compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,9 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule {
854854
severity: ErrorSeverity.Error,
855855
name: 'set-state-in-effect',
856856
description:
857-
'Validates against calling setState synchronously in an effect, which can lead to re-renders that degrade performance',
857+
'Validates against calling setState synchronously in an effect. ' +
858+
'This can indicate non-local derived data, a derived event pattern, or ' +
859+
'improper external data synchronization.',
858860
preset: LintRulePreset.Recommended,
859861
};
860862
}

compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,16 @@ export const EnvironmentConfigSchema = z.object({
695695
*/
696696
enableAllowSetStateFromRefsInEffects: z.boolean().default(true),
697697

698+
/**
699+
* When enabled, provides verbose error messages for setState calls within effects,
700+
* presenting multiple possible fixes to the user/agent since we cannot statically
701+
* determine which specific use-case applies:
702+
* 1. Non-local derived data - requires restructuring state ownership
703+
* 2. Derived event pattern - detecting when a prop changes
704+
* 3. Force update / external sync - should use useSyncExternalStore
705+
*/
706+
enableVerboseNoSetStateInEffect: z.boolean().default(false),
707+
698708
/**
699709
* Enables inference of event handler types for JSX props on built-in DOM elements.
700710
* When enabled, functions passed to event handler props (props starting with "on")

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

Lines changed: 52 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -121,26 +121,58 @@ export function validateNoSetStateInEffects(
121121
if (arg !== undefined && arg.kind === 'Identifier') {
122122
const setState = setStateFunctions.get(arg.identifier.id);
123123
if (setState !== undefined) {
124-
errors.pushDiagnostic(
125-
CompilerDiagnostic.create({
126-
category: ErrorCategory.EffectSetState,
127-
reason:
128-
'Calling setState synchronously within an effect can trigger cascading renders',
129-
description:
130-
'Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. ' +
131-
'In general, the body of an effect should do one or both of the following:\n' +
132-
'* Update external systems with the latest state from React.\n' +
133-
'* Subscribe for updates from some external system, calling setState in a callback function when external state changes.\n\n' +
134-
'Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. ' +
135-
'(https://react.dev/learn/you-might-not-need-an-effect)',
136-
suggestions: null,
137-
}).withDetails({
138-
kind: 'error',
139-
loc: setState.loc,
140-
message:
141-
'Avoid calling setState() directly within an effect',
142-
}),
143-
);
124+
const enableVerbose =
125+
env.config.enableVerboseNoSetStateInEffect;
126+
if (enableVerbose) {
127+
errors.pushDiagnostic(
128+
CompilerDiagnostic.create({
129+
category: ErrorCategory.EffectSetState,
130+
reason:
131+
'Calling setState synchronously within an effect can trigger cascading renders',
132+
description:
133+
'Effects are intended to synchronize state between React and external systems. ' +
134+
'Calling setState synchronously causes cascading renders that hurt performance.\n\n' +
135+
'This pattern may indicate one of several issues:\n\n' +
136+
'**1. Non-local derived data**: If the value being set could be computed from props/state ' +
137+
'but requires data from a parent component, consider restructuring state ownership so the ' +
138+
'derivation can happen during render in the component that owns the relevant state.\n\n' +
139+
"**2. Derived event pattern**: If you're detecting when a prop changes (e.g., `isPlaying` " +
140+
'transitioning from false to true), this often indicates the parent should provide an event ' +
141+
'callback (like `onPlay`) instead of just the current state. Request access to the original event.\n\n' +
142+
"**3. Force update / external sync**: If you're forcing a re-render to sync with an external " +
143+
'data source (mutable values outside React), use `useSyncExternalStore` to properly subscribe ' +
144+
'to external state changes.\n\n' +
145+
'See: https://react.dev/learn/you-might-not-need-an-effect',
146+
suggestions: null,
147+
}).withDetails({
148+
kind: 'error',
149+
loc: setState.loc,
150+
message:
151+
'Avoid calling setState() directly within an effect',
152+
}),
153+
);
154+
} else {
155+
errors.pushDiagnostic(
156+
CompilerDiagnostic.create({
157+
category: ErrorCategory.EffectSetState,
158+
reason:
159+
'Calling setState synchronously within an effect can trigger cascading renders',
160+
description:
161+
'Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. ' +
162+
'In general, the body of an effect should do one or both of the following:\n' +
163+
'* Update external systems with the latest state from React.\n' +
164+
'* Subscribe for updates from some external system, calling setState in a callback function when external state changes.\n\n' +
165+
'Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. ' +
166+
'(https://react.dev/learn/you-might-not-need-an-effect)',
167+
suggestions: null,
168+
}).withDetails({
169+
kind: 'error',
170+
loc: setState.loc,
171+
message:
172+
'Avoid calling setState() directly within an effect',
173+
}),
174+
);
175+
}
144176
}
145177
}
146178
}
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @loggerTestOnly @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
6+
import {useState, useEffect} from 'react';
7+
8+
function VideoPlayer({isPlaying}) {
9+
const [wasPlaying, setWasPlaying] = useState(isPlaying);
10+
useEffect(() => {
11+
if (isPlaying !== wasPlaying) {
12+
setWasPlaying(isPlaying);
13+
console.log('Play state changed!');
14+
}
15+
}, [isPlaying, wasPlaying]);
16+
return <video />;
17+
}
18+
19+
export const FIXTURE_ENTRYPOINT = {
20+
fn: VideoPlayer,
21+
params: [{isPlaying: true}],
22+
};
23+
24+
```
25+
26+
## Code
27+
28+
```javascript
29+
import { c as _c } from "react/compiler-runtime"; // @loggerTestOnly @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
30+
import { useState, useEffect } from "react";
31+
32+
function VideoPlayer(t0) {
33+
const $ = _c(5);
34+
const { isPlaying } = t0;
35+
const [wasPlaying, setWasPlaying] = useState(isPlaying);
36+
let t1;
37+
let t2;
38+
if ($[0] !== isPlaying || $[1] !== wasPlaying) {
39+
t1 = () => {
40+
if (isPlaying !== wasPlaying) {
41+
setWasPlaying(isPlaying);
42+
console.log("Play state changed!");
43+
}
44+
};
45+
46+
t2 = [isPlaying, wasPlaying];
47+
$[0] = isPlaying;
48+
$[1] = wasPlaying;
49+
$[2] = t1;
50+
$[3] = t2;
51+
} else {
52+
t1 = $[2];
53+
t2 = $[3];
54+
}
55+
useEffect(t1, t2);
56+
let t3;
57+
if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
58+
t3 = <video />;
59+
$[4] = t3;
60+
} else {
61+
t3 = $[4];
62+
}
63+
return t3;
64+
}
65+
66+
export const FIXTURE_ENTRYPOINT = {
67+
fn: VideoPlayer,
68+
params: [{ isPlaying: true }],
69+
};
70+
71+
```
72+
73+
## Logs
74+
75+
```
76+
{"kind":"CompileError","detail":{"options":{"category":"EffectSetState","reason":"Calling setState synchronously within an effect can trigger cascading renders","description":"Effects are intended to synchronize state between React and external systems. Calling setState synchronously causes cascading renders that hurt performance.\n\nThis pattern may indicate one of several issues:\n\n**1. Non-local derived data**: If the value being set could be computed from props/state but requires data from a parent component, consider restructuring state ownership so the derivation can happen during render in the component that owns the relevant state.\n\n**2. Derived event pattern**: If you're detecting when a prop changes (e.g., `isPlaying` transitioning from false to true), this often indicates the parent should provide an event callback (like `onPlay`) instead of just the current state. Request access to the original event.\n\n**3. Force update / external sync**: If you're forcing a re-render to sync with an external data source (mutable values outside React), use `useSyncExternalStore` to properly subscribe to external state changes.\n\nSee: https://react.dev/learn/you-might-not-need-an-effect","suggestions":null,"details":[{"kind":"error","loc":{"start":{"line":8,"column":6,"index":282},"end":{"line":8,"column":19,"index":295},"filename":"invalid-set-state-in-effect-verbose-derived-event.ts","identifierName":"setWasPlaying"},"message":"Avoid calling setState() directly within an effect"}]}},"fnLoc":null}
77+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":125},"end":{"line":13,"column":1,"index":408},"filename":"invalid-set-state-in-effect-verbose-derived-event.ts"},"fnName":"VideoPlayer","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
78+
```
79+
80+
### Eval output
81+
(kind: ok) <video></video>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// @loggerTestOnly @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
2+
import {useState, useEffect} from 'react';
3+
4+
function VideoPlayer({isPlaying}) {
5+
const [wasPlaying, setWasPlaying] = useState(isPlaying);
6+
useEffect(() => {
7+
if (isPlaying !== wasPlaying) {
8+
setWasPlaying(isPlaying);
9+
console.log('Play state changed!');
10+
}
11+
}, [isPlaying, wasPlaying]);
12+
return <video />;
13+
}
14+
15+
export const FIXTURE_ENTRYPOINT = {
16+
fn: VideoPlayer,
17+
params: [{isPlaying: true}],
18+
};
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @loggerTestOnly @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
6+
import {useState, useEffect} from 'react';
7+
8+
const externalStore = {
9+
value: 0,
10+
subscribe(callback) {
11+
return () => {};
12+
},
13+
getValue() {
14+
return this.value;
15+
},
16+
};
17+
18+
function ExternalDataComponent() {
19+
const [, forceUpdate] = useState({});
20+
useEffect(() => {
21+
const unsubscribe = externalStore.subscribe(() => {
22+
forceUpdate({});
23+
});
24+
return unsubscribe;
25+
}, []);
26+
return <div>{externalStore.getValue()}</div>;
27+
}
28+
29+
export const FIXTURE_ENTRYPOINT = {
30+
fn: ExternalDataComponent,
31+
params: [],
32+
};
33+
34+
```
35+
36+
## Code
37+
38+
```javascript
39+
import { c as _c } from "react/compiler-runtime"; // @loggerTestOnly @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
40+
import { useState, useEffect } from "react";
41+
42+
const externalStore = {
43+
value: 0,
44+
subscribe(callback) {
45+
return () => {};
46+
},
47+
getValue() {
48+
return this.value;
49+
},
50+
};
51+
52+
function ExternalDataComponent() {
53+
const $ = _c(4);
54+
let t0;
55+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
56+
t0 = {};
57+
$[0] = t0;
58+
} else {
59+
t0 = $[0];
60+
}
61+
const [, forceUpdate] = useState(t0);
62+
let t1;
63+
let t2;
64+
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
65+
t1 = () => {
66+
const unsubscribe = externalStore.subscribe(() => {
67+
forceUpdate({});
68+
});
69+
return unsubscribe;
70+
};
71+
t2 = [];
72+
$[1] = t1;
73+
$[2] = t2;
74+
} else {
75+
t1 = $[1];
76+
t2 = $[2];
77+
}
78+
useEffect(t1, t2);
79+
let t3;
80+
if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
81+
t3 = <div>{externalStore.getValue()}</div>;
82+
$[3] = t3;
83+
} else {
84+
t3 = $[3];
85+
}
86+
return t3;
87+
}
88+
89+
export const FIXTURE_ENTRYPOINT = {
90+
fn: ExternalDataComponent,
91+
params: [],
92+
};
93+
94+
```
95+
96+
## Logs
97+
98+
```
99+
{"kind":"CompileSuccess","fnLoc":{"start":{"line":14,"column":0,"index":258},"end":{"line":23,"column":1,"index":523},"filename":"invalid-set-state-in-effect-verbose-force-update.ts"},"fnName":"ExternalDataComponent","memoSlots":4,"memoBlocks":3,"memoValues":4,"prunedMemoBlocks":0,"prunedMemoValues":0}
100+
```
101+
102+
### Eval output
103+
(kind: ok) <div>0</div>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// @loggerTestOnly @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
2+
import {useState, useEffect} from 'react';
3+
4+
const externalStore = {
5+
value: 0,
6+
subscribe(callback) {
7+
return () => {};
8+
},
9+
getValue() {
10+
return this.value;
11+
},
12+
};
13+
14+
function ExternalDataComponent() {
15+
const [, forceUpdate] = useState({});
16+
useEffect(() => {
17+
const unsubscribe = externalStore.subscribe(() => {
18+
forceUpdate({});
19+
});
20+
return unsubscribe;
21+
}, []);
22+
return <div>{externalStore.getValue()}</div>;
23+
}
24+
25+
export const FIXTURE_ENTRYPOINT = {
26+
fn: ExternalDataComponent,
27+
params: [],
28+
};

0 commit comments

Comments
 (0)