diff --git a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts index c7c8d6a161e..7bc0d6a551a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts @@ -854,7 +854,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, }; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 17dd53adf56..750e650dec1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -695,6 +695,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") diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInEffects.ts index 86070e2973e..91de8f20671 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInEffects.ts @@ -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', + }), + ); + } } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-set-state-in-effect-verbose-derived-event.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-set-state-in-effect-verbose-derived-event.expect.md new file mode 100644 index 00000000000..60479b8c230 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-set-state-in-effect-verbose-derived-event.expect.md @@ -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 ; +} + +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 = ; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: VideoPlayer, + params: [{ isPlaying: true }], +}; + +``` + +### Eval output +(kind: ok) \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-set-state-in-effect-verbose-derived-event.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-set-state-in-effect-verbose-derived-event.js new file mode 100644 index 00000000000..4928dbe6078 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-set-state-in-effect-verbose-derived-event.js @@ -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 ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: VideoPlayer, + params: [{isPlaying: true}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-set-state-in-effect-verbose-force-update.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-set-state-in-effect-verbose-force-update.expect.md new file mode 100644 index 00000000000..131eb99127b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-set-state-in-effect-verbose-force-update.expect.md @@ -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