mirror of
https://github.com/facebook/react.git
synced 2026-02-21 19:31:52 +00:00
[compiler] Add enableVerboseNoSetStateInEffect to suggest options to user/agent (#35306)
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.
This commit is contained in:
@@ -869,7 +869,9 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule {
|
||||
severity: ErrorSeverity.Error,
|
||||
name: 'set-state-in-effect',
|
||||
description:
|
||||
'Validates against calling setState synchronously in an effect, which can lead to re-renders that degrade performance',
|
||||
'Validates against calling setState synchronously in an effect. ' +
|
||||
'This can indicate non-local derived data, a derived event pattern, or ' +
|
||||
'improper external data synchronization.',
|
||||
preset: LintRulePreset.Recommended,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -700,6 +700,16 @@ export const EnvironmentConfigSchema = z.object({
|
||||
*/
|
||||
enableAllowSetStateFromRefsInEffects: z.boolean().default(true),
|
||||
|
||||
/**
|
||||
* When enabled, provides verbose error messages for setState calls within effects,
|
||||
* presenting multiple possible fixes to the user/agent since we cannot statically
|
||||
* determine which specific use-case applies:
|
||||
* 1. Non-local derived data - requires restructuring state ownership
|
||||
* 2. Derived event pattern - detecting when a prop changes
|
||||
* 3. Force update / external sync - should use useSyncExternalStore
|
||||
*/
|
||||
enableVerboseNoSetStateInEffect: z.boolean().default(false),
|
||||
|
||||
/**
|
||||
* Enables inference of event handler types for JSX props on built-in DOM elements.
|
||||
* When enabled, functions passed to event handler props (props starting with "on")
|
||||
|
||||
@@ -121,26 +121,58 @@ export function validateNoSetStateInEffects(
|
||||
if (arg !== undefined && arg.kind === 'Identifier') {
|
||||
const setState = setStateFunctions.get(arg.identifier.id);
|
||||
if (setState !== undefined) {
|
||||
errors.pushDiagnostic(
|
||||
CompilerDiagnostic.create({
|
||||
category: ErrorCategory.EffectSetState,
|
||||
reason:
|
||||
'Calling setState synchronously within an effect can trigger cascading renders',
|
||||
description:
|
||||
'Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. ' +
|
||||
'In general, the body of an effect should do one or both of the following:\n' +
|
||||
'* Update external systems with the latest state from React.\n' +
|
||||
'* Subscribe for updates from some external system, calling setState in a callback function when external state changes.\n\n' +
|
||||
'Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. ' +
|
||||
'(https://react.dev/learn/you-might-not-need-an-effect)',
|
||||
suggestions: null,
|
||||
}).withDetails({
|
||||
kind: 'error',
|
||||
loc: setState.loc,
|
||||
message:
|
||||
'Avoid calling setState() directly within an effect',
|
||||
}),
|
||||
);
|
||||
const enableVerbose =
|
||||
env.config.enableVerboseNoSetStateInEffect;
|
||||
if (enableVerbose) {
|
||||
errors.pushDiagnostic(
|
||||
CompilerDiagnostic.create({
|
||||
category: ErrorCategory.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\n' +
|
||||
'This 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\n' +
|
||||
'See: https://react.dev/learn/you-might-not-need-an-effect',
|
||||
suggestions: null,
|
||||
}).withDetails({
|
||||
kind: 'error',
|
||||
loc: setState.loc,
|
||||
message:
|
||||
'Avoid calling setState() directly within an effect',
|
||||
}),
|
||||
);
|
||||
} else {
|
||||
errors.pushDiagnostic(
|
||||
CompilerDiagnostic.create({
|
||||
category: ErrorCategory.EffectSetState,
|
||||
reason:
|
||||
'Calling setState synchronously within an effect can trigger cascading renders',
|
||||
description:
|
||||
'Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. ' +
|
||||
'In general, the body of an effect should do one or both of the following:\n' +
|
||||
'* Update external systems with the latest state from React.\n' +
|
||||
'* Subscribe for updates from some external system, calling setState in a callback function when external state changes.\n\n' +
|
||||
'Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. ' +
|
||||
'(https://react.dev/learn/you-might-not-need-an-effect)',
|
||||
suggestions: null,
|
||||
}).withDetails({
|
||||
kind: 'error',
|
||||
loc: setState.loc,
|
||||
message:
|
||||
'Avoid calling setState() directly within an effect',
|
||||
}),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
|
||||
import {useState, useEffect} from 'react';
|
||||
|
||||
function VideoPlayer({isPlaying}) {
|
||||
const [wasPlaying, setWasPlaying] = useState(isPlaying);
|
||||
useEffect(() => {
|
||||
if (isPlaying !== wasPlaying) {
|
||||
setWasPlaying(isPlaying);
|
||||
console.log('Play state changed!');
|
||||
}
|
||||
}, [isPlaying, wasPlaying]);
|
||||
return <video />;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: VideoPlayer,
|
||||
params: [{isPlaying: true}],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
|
||||
import { useState, useEffect } from "react";
|
||||
|
||||
function VideoPlayer(t0) {
|
||||
const $ = _c(5);
|
||||
const { isPlaying } = t0;
|
||||
const [wasPlaying, setWasPlaying] = useState(isPlaying);
|
||||
let t1;
|
||||
let t2;
|
||||
if ($[0] !== isPlaying || $[1] !== wasPlaying) {
|
||||
t1 = () => {
|
||||
if (isPlaying !== wasPlaying) {
|
||||
setWasPlaying(isPlaying);
|
||||
console.log("Play state changed!");
|
||||
}
|
||||
};
|
||||
|
||||
t2 = [isPlaying, wasPlaying];
|
||||
$[0] = isPlaying;
|
||||
$[1] = wasPlaying;
|
||||
$[2] = t1;
|
||||
$[3] = t2;
|
||||
} else {
|
||||
t1 = $[2];
|
||||
t2 = $[3];
|
||||
}
|
||||
useEffect(t1, t2);
|
||||
let t3;
|
||||
if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
t3 = <video />;
|
||||
$[4] = t3;
|
||||
} else {
|
||||
t3 = $[4];
|
||||
}
|
||||
return t3;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: VideoPlayer,
|
||||
params: [{ isPlaying: true }],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: ok) <video></video>
|
||||
@@ -0,0 +1,18 @@
|
||||
// @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
|
||||
import {useState, useEffect} from 'react';
|
||||
|
||||
function VideoPlayer({isPlaying}) {
|
||||
const [wasPlaying, setWasPlaying] = useState(isPlaying);
|
||||
useEffect(() => {
|
||||
if (isPlaying !== wasPlaying) {
|
||||
setWasPlaying(isPlaying);
|
||||
console.log('Play state changed!');
|
||||
}
|
||||
}, [isPlaying, wasPlaying]);
|
||||
return <video />;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: VideoPlayer,
|
||||
params: [{isPlaying: true}],
|
||||
};
|
||||
@@ -0,0 +1,97 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
|
||||
import {useState, useEffect} from 'react';
|
||||
|
||||
const externalStore = {
|
||||
value: 0,
|
||||
subscribe(callback) {
|
||||
return () => {};
|
||||
},
|
||||
getValue() {
|
||||
return this.value;
|
||||
},
|
||||
};
|
||||
|
||||
function ExternalDataComponent() {
|
||||
const [, forceUpdate] = useState({});
|
||||
useEffect(() => {
|
||||
const unsubscribe = externalStore.subscribe(() => {
|
||||
forceUpdate({});
|
||||
});
|
||||
return unsubscribe;
|
||||
}, []);
|
||||
return <div>{externalStore.getValue()}</div>;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: ExternalDataComponent,
|
||||
params: [],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
|
||||
import { useState, useEffect } from "react";
|
||||
|
||||
const externalStore = {
|
||||
value: 0,
|
||||
subscribe(callback) {
|
||||
return () => {};
|
||||
},
|
||||
getValue() {
|
||||
return this.value;
|
||||
},
|
||||
};
|
||||
|
||||
function ExternalDataComponent() {
|
||||
const $ = _c(4);
|
||||
let t0;
|
||||
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
t0 = {};
|
||||
$[0] = t0;
|
||||
} else {
|
||||
t0 = $[0];
|
||||
}
|
||||
const [, forceUpdate] = useState(t0);
|
||||
let t1;
|
||||
let t2;
|
||||
if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
t1 = () => {
|
||||
const unsubscribe = externalStore.subscribe(() => {
|
||||
forceUpdate({});
|
||||
});
|
||||
return unsubscribe;
|
||||
};
|
||||
t2 = [];
|
||||
$[1] = t1;
|
||||
$[2] = t2;
|
||||
} else {
|
||||
t1 = $[1];
|
||||
t2 = $[2];
|
||||
}
|
||||
useEffect(t1, t2);
|
||||
let t3;
|
||||
if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
t3 = <div>{externalStore.getValue()}</div>;
|
||||
$[3] = t3;
|
||||
} else {
|
||||
t3 = $[3];
|
||||
}
|
||||
return t3;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: ExternalDataComponent,
|
||||
params: [],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: ok) <div>0</div>
|
||||
@@ -0,0 +1,28 @@
|
||||
// @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
|
||||
import {useState, useEffect} from 'react';
|
||||
|
||||
const externalStore = {
|
||||
value: 0,
|
||||
subscribe(callback) {
|
||||
return () => {};
|
||||
},
|
||||
getValue() {
|
||||
return this.value;
|
||||
},
|
||||
};
|
||||
|
||||
function ExternalDataComponent() {
|
||||
const [, forceUpdate] = useState({});
|
||||
useEffect(() => {
|
||||
const unsubscribe = externalStore.subscribe(() => {
|
||||
forceUpdate({});
|
||||
});
|
||||
return unsubscribe;
|
||||
}, []);
|
||||
return <div>{externalStore.getValue()}</div>;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: ExternalDataComponent,
|
||||
params: [],
|
||||
};
|
||||
@@ -0,0 +1,68 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
|
||||
import {useState, useEffect} from 'react';
|
||||
|
||||
function Child({firstName, lastName}) {
|
||||
const [fullName, setFullName] = useState('');
|
||||
useEffect(() => {
|
||||
setFullName(firstName + ' ' + lastName);
|
||||
}, [firstName, lastName]);
|
||||
return <div>{fullName}</div>;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: Child,
|
||||
params: [{firstName: 'John', lastName: 'Doe'}],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
|
||||
import { useState, useEffect } from "react";
|
||||
|
||||
function Child(t0) {
|
||||
const $ = _c(6);
|
||||
const { firstName, lastName } = t0;
|
||||
const [fullName, setFullName] = useState("");
|
||||
let t1;
|
||||
let t2;
|
||||
if ($[0] !== firstName || $[1] !== lastName) {
|
||||
t1 = () => {
|
||||
setFullName(firstName + " " + lastName);
|
||||
};
|
||||
t2 = [firstName, lastName];
|
||||
$[0] = firstName;
|
||||
$[1] = lastName;
|
||||
$[2] = t1;
|
||||
$[3] = t2;
|
||||
} else {
|
||||
t1 = $[2];
|
||||
t2 = $[3];
|
||||
}
|
||||
useEffect(t1, t2);
|
||||
let t3;
|
||||
if ($[4] !== fullName) {
|
||||
t3 = <div>{fullName}</div>;
|
||||
$[4] = fullName;
|
||||
$[5] = t3;
|
||||
} else {
|
||||
t3 = $[5];
|
||||
}
|
||||
return t3;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: Child,
|
||||
params: [{ firstName: "John", lastName: "Doe" }],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: ok) <div>John Doe</div>
|
||||
@@ -0,0 +1,15 @@
|
||||
// @validateNoSetStateInEffects @enableVerboseNoSetStateInEffect
|
||||
import {useState, useEffect} from 'react';
|
||||
|
||||
function Child({firstName, lastName}) {
|
||||
const [fullName, setFullName] = useState('');
|
||||
useEffect(() => {
|
||||
setFullName(firstName + ' ' + lastName);
|
||||
}, [firstName, lastName]);
|
||||
return <div>{fullName}</div>;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: Child,
|
||||
params: [{firstName: 'John', lastName: 'Doe'}],
|
||||
};
|
||||
Reference in New Issue
Block a user