From a6e489bba34143e1241eaf9f10e3edd228f7a9f4 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 4 Dec 2025 00:12:34 -0800 Subject: [PATCH 1/6] [compiler][poc] Reuse ValidateExhaustiveDeps for effect dep validation Alternative approach to #35282 for validating effect deps in the compiler that builds on the machinery in ValidateExhaustiveDependencies. Key changes to that pass: * Refactor to track the dependencies of array expressions as temporaries so we can look them up later if they appear as effect deps. * Instead of not storing temporaries for LoadLocals of locally created variables, we store the temporary but also propagate the local-ness through. This allows us to record deps at the top level, necessary for effect deps. Previously the pass was only ever concerned with tracking deps within function expressions. * Refactor the bulk of the dependency-checking logic from `onFinishMemoize()` into a standalone helper to use it for the new `onEffect()` helper as well. A few important remaining todos: * Add a new ErrorCategory for effect deps, use it for errors on effects * Put the effect dep validation behind a feature flag * Adjust the error reason for effect errors Testing it locally on fixtures this looks ballpark right but i didn't do an exhaustive test. --- .../ValidateExhaustiveDependencies.ts | 590 +++++++++++------- 1 file changed, 348 insertions(+), 242 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index 2e6217bb51a..0208380d823 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -10,6 +10,7 @@ import { CompilerDiagnostic, CompilerError, CompilerSuggestionOperation, + Effect, SourceLocation, } from '..'; import {CompilerSuggestion, ErrorCategory} from '../CompilerError'; @@ -18,6 +19,7 @@ import { BlockId, DependencyPath, FinishMemoize, + GeneratedSource, HIRFunction, Identifier, IdentifierId, @@ -40,6 +42,7 @@ import { } from '../HIR/visitors'; import {Result} from '../Utils/Result'; import {retainWhere} from '../Utils/utils'; +import {isEffectHook} from './ValidateMemoizedEffectDependencies'; const DEBUG = false; @@ -128,268 +131,327 @@ export function validateExhaustiveDependencies( ); visitCandidateDependency(value.decl, temporaries, dependencies, locals); const inferred: Array = Array.from(dependencies); - // Sort dependencies by name and path, with shorter/non-optional paths first - inferred.sort((a, b) => { - if (a.kind === 'Global' && b.kind == 'Global') { - return a.binding.name.localeCompare(b.binding.name); - } else if (a.kind == 'Local' && b.kind == 'Local') { - CompilerError.simpleInvariant( - a.identifier.name != null && - a.identifier.name.kind === 'named' && - b.identifier.name != null && - b.identifier.name.kind === 'named', - { - reason: 'Expected dependencies to be named variables', - loc: a.loc, - }, - ); - if (a.identifier.id !== b.identifier.id) { - return a.identifier.name.value.localeCompare(b.identifier.name.value); - } - if (a.path.length !== b.path.length) { - // if a's path is shorter this returns a negative, sorting a first - return a.path.length - b.path.length; + + const diagnostic = validateDependencies( + inferred, + startMemo.deps ?? [], + reactive, + startMemo.depsLoc, + ); + if (diagnostic != null) { + error.pushDiagnostic(diagnostic); + } + + dependencies.clear(); + locals.clear(); + startMemo = null; + } + + collectDependencies( + fn, + temporaries, + { + onStartMemoize, + onFinishMemoize, + onEffect: (inferred, manual) => { + if (DEBUG) { + console.log(Array.from(inferred, printInferredDependency)); + console.log(Array.from(manual, printInferredDependency)); } - for (let i = 0; i < a.path.length; i++) { - const aProperty = a.path[i]; - const bProperty = b.path[i]; - const aOptional = aProperty.optional ? 0 : 1; - const bOptional = bProperty.optional ? 0 : 1; - if (aOptional !== bOptional) { - // sort non-optionals first - return aOptional - bOptional; - } else if (aProperty.property !== bProperty.property) { - return String(aProperty.property).localeCompare( - String(bProperty.property), - ); + const manualDeps: Array = []; + for (const dep of manual) { + if (dep.kind === 'Local') { + manualDeps.push({ + root: { + kind: 'NamedLocal', + constant: false, + value: { + effect: Effect.Read, + identifier: dep.identifier, + kind: 'Identifier', + loc: dep.loc, + reactive: reactive.has(dep.identifier.id), + }, + }, + path: dep.path, + loc: dep.loc, + }); + } else { + manualDeps.push({ + root: { + kind: 'Global', + identifierName: dep.binding.name, + }, + path: [], + loc: GeneratedSource, + }); } } - return 0; - } else { - const aName = - a.kind === 'Global' ? a.binding.name : a.identifier.name?.value; - const bName = - b.kind === 'Global' ? b.binding.name : b.identifier.name?.value; - if (aName != null && bName != null) { - return aName.localeCompare(bName); - } - return 0; - } - }); - // remove redundant inferred dependencies - retainWhere(inferred, (dep, ix) => { - const match = inferred.findIndex(prevDep => { - return ( - isEqualTemporary(prevDep, dep) || - (prevDep.kind === 'Local' && - dep.kind === 'Local' && - prevDep.identifier.id === dep.identifier.id && - isSubPath(prevDep.path, dep.path)) + const diagnostic = validateDependencies( + Array.from(inferred), + manualDeps, + reactive, + null, ); - }); - // only retain entries that don't have a prior match - return match === -1 || match >= ix; - }); - // Validate that all manual dependencies belong there - if (DEBUG) { - console.log('manual'); - console.log( - (startMemo.deps ?? []) - .map(x => ' ' + printManualMemoDependency(x)) - .join('\n'), - ); - console.log('inferred'); - console.log( - inferred.map(x => ' ' + printInferredDependency(x)).join('\n'), + if (diagnostic != null) { + error.pushDiagnostic(diagnostic); + } + }, + }, + false, // isFunctionExpression + ); + return error.asResult(); +} + +function validateDependencies( + inferred: Array, + manualDependencies: Array, + reactive: Set, + manualMemoLoc: SourceLocation | null, +): CompilerDiagnostic | null { + // Sort dependencies by name and path, with shorter/non-optional paths first + inferred.sort((a, b) => { + if (a.kind === 'Global' && b.kind == 'Global') { + return a.binding.name.localeCompare(b.binding.name); + } else if (a.kind == 'Local' && b.kind == 'Local') { + CompilerError.simpleInvariant( + a.identifier.name != null && + a.identifier.name.kind === 'named' && + b.identifier.name != null && + b.identifier.name.kind === 'named', + { + reason: 'Expected dependencies to be named variables', + loc: a.loc, + }, ); - } - const manualDependencies = startMemo.deps ?? []; - const matched: Set = new Set(); - const missing: Array> = []; - const extra: Array = []; - for (const inferredDependency of inferred) { - if (inferredDependency.kind === 'Global') { - for (const manualDependency of manualDependencies) { - if ( - manualDependency.root.kind === 'Global' && - manualDependency.root.identifierName === - inferredDependency.binding.name - ) { - matched.add(manualDependency); - extra.push(manualDependency); - } + if (a.identifier.id !== b.identifier.id) { + return a.identifier.name.value.localeCompare(b.identifier.name.value); + } + if (a.path.length !== b.path.length) { + // if a's path is shorter this returns a negative, sorting a first + return a.path.length - b.path.length; + } + for (let i = 0; i < a.path.length; i++) { + const aProperty = a.path[i]; + const bProperty = b.path[i]; + const aOptional = aProperty.optional ? 0 : 1; + const bOptional = bProperty.optional ? 0 : 1; + if (aOptional !== bOptional) { + // sort non-optionals first + return aOptional - bOptional; + } else if (aProperty.property !== bProperty.property) { + return String(aProperty.property).localeCompare( + String(bProperty.property), + ); } - continue; } - CompilerError.simpleInvariant(inferredDependency.kind === 'Local', { - reason: 'Unexpected function dependency', - loc: value.loc, - }); - let hasMatchingManualDependency = false; + return 0; + } else { + const aName = + a.kind === 'Global' ? a.binding.name : a.identifier.name?.value; + const bName = + b.kind === 'Global' ? b.binding.name : b.identifier.name?.value; + if (aName != null && bName != null) { + return aName.localeCompare(bName); + } + return 0; + } + }); + // remove redundant inferred dependencies + retainWhere(inferred, (dep, ix) => { + const match = inferred.findIndex(prevDep => { + return ( + isEqualTemporary(prevDep, dep) || + (prevDep.kind === 'Local' && + dep.kind === 'Local' && + prevDep.identifier.id === dep.identifier.id && + isSubPath(prevDep.path, dep.path)) + ); + }); + // only retain entries that don't have a prior match + return match === -1 || match >= ix; + }); + // Validate that all manual dependencies belong there + if (DEBUG) { + console.log('manual'); + console.log( + manualDependencies + .map(x => ' ' + printManualMemoDependency(x)) + .join('\n'), + ); + console.log('inferred'); + console.log( + inferred.map(x => ' ' + printInferredDependency(x)).join('\n'), + ); + } + const matched: Set = new Set(); + const missing: Array> = []; + const extra: Array = []; + for (const inferredDependency of inferred) { + if (inferredDependency.kind === 'Global') { for (const manualDependency of manualDependencies) { if ( - manualDependency.root.kind === 'NamedLocal' && - manualDependency.root.value.identifier.id === - inferredDependency.identifier.id && - (areEqualPaths(manualDependency.path, inferredDependency.path) || - isSubPathIgnoringOptionals( - manualDependency.path, - inferredDependency.path, - )) + manualDependency.root.kind === 'Global' && + manualDependency.root.identifierName === + inferredDependency.binding.name ) { - hasMatchingManualDependency = true; matched.add(manualDependency); + extra.push(manualDependency); } } + continue; + } + CompilerError.simpleInvariant(inferredDependency.kind === 'Local', { + reason: 'Unexpected function dependency', + loc: inferredDependency.loc, + }); + let hasMatchingManualDependency = false; + for (const manualDependency of manualDependencies) { if ( - hasMatchingManualDependency || - isOptionalDependency(inferredDependency, reactive) + manualDependency.root.kind === 'NamedLocal' && + manualDependency.root.value.identifier.id === + inferredDependency.identifier.id && + (areEqualPaths(manualDependency.path, inferredDependency.path) || + isSubPathIgnoringOptionals( + manualDependency.path, + inferredDependency.path, + )) ) { - continue; + hasMatchingManualDependency = true; + matched.add(manualDependency); } - missing.push(inferredDependency); } + if ( + hasMatchingManualDependency || + isOptionalDependency(inferredDependency, reactive) + ) { + continue; + } + missing.push(inferredDependency); + } - for (const dep of startMemo.deps ?? []) { - if (matched.has(dep)) { - continue; - } - if (dep.root.kind === 'NamedLocal' && dep.root.constant) { - CompilerError.simpleInvariant( - !dep.root.value.reactive && - isPrimitiveType(dep.root.value.identifier), - { - reason: 'Expected constant-folded dependency to be non-reactive', - loc: dep.root.value.loc, - }, - ); - /* - * Constant primitives can get constant-folded, which means we won't - * see a LoadLocal for the value within the memo function. - */ - continue; - } - extra.push(dep); + for (const dep of manualDependencies) { + if (matched.has(dep)) { + continue; } + if (dep.root.kind === 'NamedLocal' && dep.root.constant) { + CompilerError.simpleInvariant( + !dep.root.value.reactive && isPrimitiveType(dep.root.value.identifier), + { + reason: 'Expected constant-folded dependency to be non-reactive', + loc: dep.root.value.loc, + }, + ); + /* + * Constant primitives can get constant-folded, which means we won't + * see a LoadLocal for the value within the memo function. + */ + continue; + } + extra.push(dep); + } - if (missing.length !== 0 || extra.length !== 0) { - let suggestion: CompilerSuggestion | null = null; - if (startMemo.depsLoc != null && typeof startMemo.depsLoc !== 'symbol') { - suggestion = { - description: 'Update dependencies', - range: [startMemo.depsLoc.start.index, startMemo.depsLoc.end.index], - op: CompilerSuggestionOperation.Replace, - text: `[${inferred - .filter( - dep => - dep.kind === 'Local' && !isOptionalDependency(dep, reactive), - ) - .map(printInferredDependency) - .join(', ')}]`, - }; + if (missing.length !== 0 || extra.length !== 0) { + let suggestion: CompilerSuggestion | null = null; + if (manualMemoLoc != null && typeof manualMemoLoc !== 'symbol') { + suggestion = { + description: 'Update dependencies', + range: [manualMemoLoc.start.index, manualMemoLoc.end.index], + op: CompilerSuggestionOperation.Replace, + text: `[${inferred + .filter( + dep => dep.kind === 'Local' && !isOptionalDependency(dep, reactive), + ) + .map(printInferredDependency) + .join(', ')}]`, + }; + } + const diagnostic = CompilerDiagnostic.create({ + category: ErrorCategory.MemoDependencies, + reason: 'Found missing/extra memoization dependencies', + description: [ + missing.length !== 0 + ? 'Missing dependencies can cause a value to update less often than it should, ' + + 'resulting in stale UI' + : null, + extra.length !== 0 + ? 'Extra dependencies can cause a value to update more often than it should, ' + + 'resulting in performance problems such as excessive renders or effects firing too often' + : null, + ] + .filter(Boolean) + .join('. '), + suggestions: suggestion != null ? [suggestion] : null, + }); + for (const dep of missing) { + let reactiveStableValueHint = ''; + if (isStableType(dep.identifier)) { + reactiveStableValueHint = + '. Refs, setState functions, and other "stable" values generally do not need to be added ' + + 'as dependencies, but this variable may change over time to point to different values'; } - const diagnostic = CompilerDiagnostic.create({ - category: ErrorCategory.MemoDependencies, - reason: 'Found missing/extra memoization dependencies', - description: [ - missing.length !== 0 - ? 'Missing dependencies can cause a value to update less often than it should, ' + - 'resulting in stale UI' - : null, - extra.length !== 0 - ? 'Extra dependencies can cause a value to update more often than it should, ' + - 'resulting in performance problems such as excessive renders or effects firing too often' - : null, - ] - .filter(Boolean) - .join('. '), - suggestions: suggestion != null ? [suggestion] : null, + diagnostic.withDetails({ + kind: 'error', + message: `Missing dependency \`${printInferredDependency(dep)}\`${reactiveStableValueHint}`, + loc: dep.loc, }); - for (const dep of missing) { - let reactiveStableValueHint = ''; - if (isStableType(dep.identifier)) { - reactiveStableValueHint = - '. Refs, setState functions, and other "stable" values generally do not need to be added ' + - 'as dependencies, but this variable may change over time to point to different values'; - } + } + for (const dep of extra) { + if (dep.root.kind === 'Global') { diagnostic.withDetails({ kind: 'error', - message: `Missing dependency \`${printInferredDependency(dep)}\`${reactiveStableValueHint}`, - loc: dep.loc, + message: + `Unnecessary dependency \`${printManualMemoDependency(dep)}\`. ` + + 'Values declared outside of a component/hook should not be listed as ' + + 'dependencies as the component will not re-render if they change', + loc: dep.loc ?? manualMemoLoc, }); - } - for (const dep of extra) { - if (dep.root.kind === 'Global') { + } else { + const root = dep.root.value; + const matchingInferred = inferred.find( + ( + inferredDep, + ): inferredDep is Extract => { + return ( + inferredDep.kind === 'Local' && + inferredDep.identifier.id === root.identifier.id && + isSubPathIgnoringOptionals(inferredDep.path, dep.path) + ); + }, + ); + if ( + matchingInferred != null && + !isOptionalDependency(matchingInferred, reactive) + ) { diagnostic.withDetails({ kind: 'error', message: - `Unnecessary dependency \`${printManualMemoDependency(dep)}\`. ` + - 'Values declared outside of a component/hook should not be listed as ' + - 'dependencies as the component will not re-render if they change', - loc: dep.loc ?? startMemo.depsLoc ?? value.loc, + `Overly precise dependency \`${printManualMemoDependency(dep)}\`, ` + + `use \`${printInferredDependency(matchingInferred)}\` instead`, + loc: dep.loc ?? manualMemoLoc, }); - error.pushDiagnostic(diagnostic); } else { - const root = dep.root.value; - const matchingInferred = inferred.find( - ( - inferredDep, - ): inferredDep is Extract => { - return ( - inferredDep.kind === 'Local' && - inferredDep.identifier.id === root.identifier.id && - isSubPathIgnoringOptionals(inferredDep.path, dep.path) - ); - }, - ); - if ( - matchingInferred != null && - !isOptionalDependency(matchingInferred, reactive) - ) { - diagnostic.withDetails({ - kind: 'error', - message: - `Overly precise dependency \`${printManualMemoDependency(dep)}\`, ` + - `use \`${printInferredDependency(matchingInferred)}\` instead`, - loc: dep.loc ?? startMemo.depsLoc ?? value.loc, - }); - } else { - /** - * Else this dependency doesn't correspond to anything referenced in the memo function, - * or is an optional dependency so we don't want to suggest adding it - */ - diagnostic.withDetails({ - kind: 'error', - message: `Unnecessary dependency \`${printManualMemoDependency(dep)}\``, - loc: dep.loc ?? startMemo.depsLoc ?? value.loc, - }); - } + /** + * Else this dependency doesn't correspond to anything referenced in the memo function, + * or is an optional dependency so we don't want to suggest adding it + */ + diagnostic.withDetails({ + kind: 'error', + message: `Unnecessary dependency \`${printManualMemoDependency(dep)}\``, + loc: dep.loc ?? manualMemoLoc, + }); } } - if (suggestion != null) { - diagnostic.withDetails({ - kind: 'hint', - message: `Inferred dependencies: \`${suggestion.text}\``, - }); - } - error.pushDiagnostic(diagnostic); } - - dependencies.clear(); - locals.clear(); - startMemo = null; + if (suggestion != null) { + diagnostic.withDetails({ + kind: 'hint', + message: `Inferred dependencies: \`${suggestion.text}\``, + }); + } + return diagnostic; } - - collectDependencies( - fn, - temporaries, - { - onStartMemoize, - onFinishMemoize, - }, - false, // isFunctionExpression - ); - return error.asResult(); + return null; } function addDependency( @@ -397,7 +459,7 @@ function addDependency( dependencies: Set, locals: Set, ): void { - if (dep.kind === 'Function') { + if (dep.kind === 'Aggregate') { for (const x of dep.dependencies) { addDependency(x, dependencies, locals); } @@ -480,9 +542,13 @@ function collectDependencies( dependencies: Set, locals: Set, ) => void; + onEffect: ( + inferred: Set, + manual: Set, + ) => void; } | null, isFunctionExpression: boolean, -): Extract { +): Extract { const optionals = findOptionalPlaces(fn); if (DEBUG) { console.log(prettyFormat(optionals)); @@ -501,25 +567,25 @@ function collectDependencies( } for (const block of fn.body.blocks.values()) { for (const phi of block.phis) { - let deps: Array | null = null; + const deps: Array = []; for (const operand of phi.operands.values()) { const dep = temporaries.get(operand.identifier.id); if (dep == null) { continue; } - if (deps == null) { - deps = [dep]; + if (dep.kind === 'Aggregate') { + deps.push(...dep.dependencies); } else { deps.push(dep); } } - if (deps == null) { + if (deps.length === 0) { continue; } else if (deps.length === 1) { temporaries.set(phi.place.identifier.id, deps[0]!); } else { temporaries.set(phi.place.identifier.id, { - kind: 'Function', + kind: 'Aggregate', dependencies: new Set(deps), }); } @@ -537,9 +603,6 @@ function collectDependencies( } case 'LoadContext': case 'LoadLocal': { - if (locals.has(value.place.identifier.id)) { - break; - } const temp = temporaries.get(value.place.identifier.id); if (temp != null) { if (temp.kind === 'Local') { @@ -548,6 +611,9 @@ function collectDependencies( } else { temporaries.set(lvalue.identifier.id, temp); } + if (locals.has(value.place.identifier.id)) { + locals.add(lvalue.identifier.id); + } } break; } @@ -683,10 +749,50 @@ function collectDependencies( } break; } + case 'ArrayExpression': { + const arrayDeps: Set = new Set(); + for (const item of value.elements) { + if (item.kind === 'Hole') { + continue; + } + const place = item.kind === 'Identifier' ? item : item.place; + // Visit with alternative deps/locals to record manual dependencies + visitCandidateDependency(place, temporaries, arrayDeps, new Set()); + // Visit normally to propagate inferred dependencies upward + visit(place); + } + temporaries.set(lvalue.identifier.id, { + kind: 'Aggregate', + dependencies: arrayDeps, + }); + break; + } + case 'CallExpression': case 'MethodCall': { + const receiver = + value.kind === 'CallExpression' ? value.callee : value.property; + + const onEffect = callbacks?.onEffect; + if (onEffect != null && isEffectHook(receiver.identifier)) { + const [fn, deps] = value.args; + if (fn?.kind === 'Identifier' && deps?.kind === 'Identifier') { + const fnDeps = temporaries.get(fn.identifier.id); + const manualDeps = temporaries.get(deps.identifier.id); + if ( + fnDeps?.kind === 'Aggregate' && + manualDeps?.kind === 'Aggregate' + ) { + onEffect(fnDeps.dependencies, manualDeps.dependencies); + } + } + } + // Ignore the method itself for (const operand of eachInstructionValueOperand(value)) { - if (operand.identifier.id === value.property.identifier.id) { + if ( + value.kind === 'MethodCall' && + operand.identifier.id === value.property.identifier.id + ) { continue; } visit(operand); @@ -710,7 +816,7 @@ function collectDependencies( visit(operand); } } - return {kind: 'Function', dependencies}; + return {kind: 'Aggregate', dependencies}; } function printInferredDependency(dep: InferredDependency): string { @@ -748,7 +854,7 @@ function printManualMemoDependency(dep: ManualMemoDependency): string { function isEqualTemporary(a: Temporary, b: Temporary): boolean { switch (a.kind) { - case 'Function': { + case 'Aggregate': { return false; } case 'Global': { @@ -773,7 +879,7 @@ type Temporary = context: boolean; loc: SourceLocation; } - | {kind: 'Function'; dependencies: Set}; + | {kind: 'Aggregate'; dependencies: Set}; type InferredDependency = Extract; function collectReactiveIdentifiersHIR(fn: HIRFunction): Set { From 5c7e374d7399a61d9fc336bc01eae585307729f7 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Thu, 4 Dec 2025 14:36:16 -0500 Subject: [PATCH 2/6] Add config option validateExhaustiveEffectDependencies --- .../src/Entrypoint/Pipeline.ts | 7 ++- .../src/HIR/Environment.ts | 5 ++ .../ValidateExhaustiveDependencies.ts | 27 ++++++---- .../error.invalid-exhaustive-deps.expect.md | 44 +-------------- ...r.invalid-exhaustive-effect-deps.expect.md | 54 +++++++++++++++++++ .../error.invalid-exhaustive-effect-deps.js | 14 +++++ 6 files changed, 96 insertions(+), 55 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 1b76dfb43e1..3d2b5f6e743 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -304,9 +304,12 @@ function runWithEnvironment( log({kind: 'hir', name: 'InferReactivePlaces', value: hir}); if (env.enableValidations) { - if (env.config.validateExhaustiveMemoizationDependencies) { + if ( + env.config.validateExhaustiveMemoizationDependencies || + env.config.validateExhaustiveEffectDependencies + ) { // NOTE: this relies on reactivity inference running first - validateExhaustiveDependencies(hir).unwrap(); + validateExhaustiveDependencies(hir, env).unwrap(); } } 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 fc5ba403817..c9ef6d3e426 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -223,6 +223,11 @@ export const EnvironmentConfigSchema = z.object({ */ validateExhaustiveMemoizationDependencies: z.boolean().default(true), + /** + * Validate that dependencies supplied to effect hooks are exhaustive. + */ + validateExhaustiveEffectDependencies: z.boolean().default(false), + /** * When this is true, rather than pruning existing manual memoization but ensuring or validating * that the memoized values remain memoized, the compiler will simply not prune existing calls to diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index 0208380d823..f6152cc3ae9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -18,6 +18,7 @@ import { areEqualPaths, BlockId, DependencyPath, + Environment, FinishMemoize, GeneratedSource, HIRFunction, @@ -87,6 +88,7 @@ const DEBUG = false; */ export function validateExhaustiveDependencies( fn: HIRFunction, + env: Environment, ): Result { const reactive = collectReactiveIdentifiersHIR(fn); @@ -129,17 +131,19 @@ export function validateExhaustiveDependencies( loc: value.loc, }, ); - visitCandidateDependency(value.decl, temporaries, dependencies, locals); - const inferred: Array = Array.from(dependencies); + if (env.config.validateExhaustiveMemoizationDependencies) { + visitCandidateDependency(value.decl, temporaries, dependencies, locals); + const inferred: Array = Array.from(dependencies); - const diagnostic = validateDependencies( - inferred, - startMemo.deps ?? [], - reactive, - startMemo.depsLoc, - ); - if (diagnostic != null) { - error.pushDiagnostic(diagnostic); + const diagnostic = validateDependencies( + inferred, + startMemo.deps ?? [], + reactive, + startMemo.depsLoc, + ); + if (diagnostic != null) { + error.pushDiagnostic(diagnostic); + } } dependencies.clear(); @@ -154,6 +158,9 @@ export function validateExhaustiveDependencies( onStartMemoize, onFinishMemoize, onEffect: (inferred, manual) => { + if (env.config.validateExhaustiveEffectDependencies === false) { + return; + } if (DEBUG) { console.log(Array.from(inferred, printInferredDependency)); console.log(Array.from(manual, printInferredDependency)); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md index 7f94c039039..557551050b4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md @@ -51,7 +51,7 @@ function Component({x, y, z}) { ## Error ``` -Found 5 errors: +Found 4 errors: Error: Found missing/extra memoization dependencies @@ -145,48 +145,6 @@ Inferred dependencies: `[]` Error: Found missing/extra memoization dependencies -Extra dependencies can cause a value to update more often than it should, resulting in performance problems such as excessive renders or effects firing too often. - -error.invalid-exhaustive-deps.ts:31:6 - 29 | return []; - 30 | // error: unnecessary -> 31 | }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]); - | ^ Unnecessary dependency `x` - 32 | const ref1 = useRef(null); - 33 | const ref2 = useRef(null); - 34 | const ref = z ? ref1 : ref2; - -error.invalid-exhaustive-deps.ts:31:9 - 29 | return []; - 30 | // error: unnecessary -> 31 | }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]); - | ^^^ Unnecessary dependency `y.z` - 32 | const ref1 = useRef(null); - 33 | const ref2 = useRef(null); - 34 | const ref = z ? ref1 : ref2; - -error.invalid-exhaustive-deps.ts:31:14 - 29 | return []; - 30 | // error: unnecessary -> 31 | }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]); - | ^^^^^^^ Unnecessary dependency `z?.y?.a` - 32 | const ref1 = useRef(null); - 33 | const ref2 = useRef(null); - 34 | const ref = z ? ref1 : ref2; - -error.invalid-exhaustive-deps.ts:31:23 - 29 | return []; - 30 | // error: unnecessary -> 31 | }, [x, y.z, z?.y?.a, UNUSED_GLOBAL]); - | ^^^^^^^^^^^^^ Unnecessary dependency `UNUSED_GLOBAL`. Values declared outside of a component/hook should not be listed as dependencies as the component will not re-render if they change - 32 | const ref1 = useRef(null); - 33 | const ref2 = useRef(null); - 34 | const ref = z ? ref1 : ref2; - -Inferred dependencies: `[]` - -Error: Found missing/extra memoization dependencies - Missing dependencies can cause a value to update less often than it should, resulting in stale UI. error.invalid-exhaustive-deps.ts:37:13 diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md new file mode 100644 index 00000000000..b69607503a4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md @@ -0,0 +1,54 @@ + +## Input + +```javascript +// @validateExhaustiveEffectDependencies +import {useEffect} from 'react'; + +function Component({x, y, z}) { + // error: missing dep - x + useEffect(() => { + console.log(x); + }, []); + + // error: extra dep - y + useEffect(() => { + console.log(x); + }, [x, y]); +} + +``` + + +## Error + +``` +Found 2 errors: + +Error: Found missing/extra memoization dependencies + +Missing dependencies can cause a value to update less often than it should, resulting in stale UI. + +error.invalid-exhaustive-effect-deps.ts:7:16 + 5 | // error: missing dep - x + 6 | useEffect(() => { +> 7 | console.log(x); + | ^ Missing dependency `x` + 8 | }, []); + 9 | + 10 | // error: extra dep - y + +Error: Found missing/extra memoization dependencies + +Extra dependencies can cause a value to update more often than it should, resulting in performance problems such as excessive renders or effects firing too often. + +error.invalid-exhaustive-effect-deps.ts:13:9 + 11 | useEffect(() => { + 12 | console.log(x); +> 13 | }, [x, y]); + | ^ Unnecessary dependency `y` + 14 | } + 15 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.js new file mode 100644 index 00000000000..7cc279abaf0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.js @@ -0,0 +1,14 @@ +// @validateExhaustiveEffectDependencies +import {useEffect} from 'react'; + +function Component({x, y, z}) { + // error: missing dep - x + useEffect(() => { + console.log(x); + }, []); + + // error: extra dep - y + useEffect(() => { + console.log(x); + }, [x, y]); +} From 0a93bf88e0ba116f5a2664dcd838b3338cce0daa Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Thu, 4 Dec 2025 15:29:28 -0500 Subject: [PATCH 3/6] Add new ErrorCategory.EffectExhaustiveDependencies and update reason/description for effect dep errors --- .../src/CompilerError.ts | 17 +++- .../ValidateExhaustiveDependencies.ts | 84 +++++++++++++++---- ...invalid-dep-on-ref-current-value.expect.md | 2 +- ...eps-disallow-unused-stable-types.expect.md | 2 +- .../error.invalid-exhaustive-deps.expect.md | 4 +- ...r.invalid-exhaustive-effect-deps.expect.md | 80 +++++++++++++++--- .../error.invalid-exhaustive-effect-deps.js | 14 +++- ...g-nonreactive-dep-inner-function.expect.md | 2 +- ...ssing-nonreactive-dep-unmemoized.expect.md | 2 +- ....invalid-missing-nonreactive-dep.expect.md | 2 +- ...ror.sketchy-code-exhaustive-deps.expect.md | 2 +- ...o-unrelated-mutation-in-depslist.expect.md | 2 +- 12 files changed, 171 insertions(+), 42 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts index c7c8d6a161e..a8869fdb8f0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts @@ -601,7 +601,8 @@ function printErrorSummary(category: ErrorCategory, message: string): string { case ErrorCategory.Syntax: case ErrorCategory.UseMemo: case ErrorCategory.VoidUseMemo: - case ErrorCategory.MemoDependencies: { + case ErrorCategory.MemoDependencies: + case ErrorCategory.EffectExhaustiveDependencies: { heading = 'Error'; break; } @@ -683,6 +684,10 @@ export enum ErrorCategory { * Checks for memoized effect deps */ EffectDependencies = 'EffectDependencies', + /** + * Checks for exhaustive and extraneous effect dependencies + */ + EffectExhaustiveDependencies = 'EffectExhaustiveDependencies', /** * Checks for no setState in effect bodies */ @@ -838,6 +843,16 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule { preset: LintRulePreset.Off, }; } + case ErrorCategory.EffectExhaustiveDependencies: { + return { + category, + severity: ErrorSeverity.Error, + name: 'exhaustive-effect-dependencies', + description: + 'Validates that effect dependencies are exhaustive and without extraneous values', + preset: LintRulePreset.Off, + }; + } case ErrorCategory.EffectDerivationsOfState: { return { category, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index f6152cc3ae9..e22e63ead50 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -140,6 +140,7 @@ export function validateExhaustiveDependencies( startMemo.deps ?? [], reactive, startMemo.depsLoc, + ErrorCategory.MemoDependencies, ); if (diagnostic != null) { error.pushDiagnostic(diagnostic); @@ -199,6 +200,7 @@ export function validateExhaustiveDependencies( manualDeps, reactive, null, + ErrorCategory.EffectExhaustiveDependencies, ); if (diagnostic != null) { error.pushDiagnostic(diagnostic); @@ -215,6 +217,9 @@ function validateDependencies( manualDependencies: Array, reactive: Set, manualMemoLoc: SourceLocation | null, + category: + | ErrorCategory.MemoDependencies + | ErrorCategory.EffectExhaustiveDependencies, ): CompilerDiagnostic | null { // Sort dependencies by name and path, with shorter/non-optional paths first inferred.sort((a, b) => { @@ -373,23 +378,7 @@ function validateDependencies( .join(', ')}]`, }; } - const diagnostic = CompilerDiagnostic.create({ - category: ErrorCategory.MemoDependencies, - reason: 'Found missing/extra memoization dependencies', - description: [ - missing.length !== 0 - ? 'Missing dependencies can cause a value to update less often than it should, ' + - 'resulting in stale UI' - : null, - extra.length !== 0 - ? 'Extra dependencies can cause a value to update more often than it should, ' + - 'resulting in performance problems such as excessive renders or effects firing too often' - : null, - ] - .filter(Boolean) - .join('. '), - suggestions: suggestion != null ? [suggestion] : null, - }); + const diagnostic = createDiagnostic(category, missing, extra, suggestion); for (const dep of missing) { let reactiveStableValueHint = ''; if (isStableType(dep.identifier)) { @@ -1001,3 +990,64 @@ function isOptionalDependency( isPrimitiveType(inferredDependency.identifier)) ); } + +function createDiagnostic( + category: + | ErrorCategory.MemoDependencies + | ErrorCategory.EffectExhaustiveDependencies, + missing: Array, + extra: Array, + suggestion: CompilerSuggestion | null, +): CompilerDiagnostic { + let reason: string; + let description: string; + + function joinMissingExtraDetail( + missingString: string, + extraString: string, + joinStr: string, + ): string { + return [ + missing.length !== 0 ? missingString : null, + extra.length !== 0 ? extraString : null, + ] + .filter(Boolean) + .join(joinStr); + } + + switch (category) { + case ErrorCategory.MemoDependencies: { + reason = `Found ${joinMissingExtraDetail('missing', 'extra', '/')} memoization dependencies`; + description = joinMissingExtraDetail( + 'Missing dependencies can cause a value to update less often than it should, resulting in stale UI', + 'Extra dependencies can cause a value to update more often than it should, resulting in performance' + + ' problems such as excessive renders or effects firing too often', + '. ', + ); + break; + } + case ErrorCategory.EffectExhaustiveDependencies: { + reason = `Found ${joinMissingExtraDetail('missing', 'extra', '/')} effect dependencies`; + description = joinMissingExtraDetail( + 'Missing dependencies can cause an effect to fire less often than it should', + 'Extra dependencies can cause an effect to fire more often than it should, resulting' + + ' in performance problems such as excessive renders and side effects', + '. ', + ); + break; + } + default: { + CompilerError.simpleInvariant(false, { + reason: `Unexpected error category: ${category}`, + loc: GeneratedSource, + }); + } + } + + return CompilerDiagnostic.create({ + category, + reason, + description, + suggestions: suggestion != null ? [suggestion] : null, + }); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-dep-on-ref-current-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-dep-on-ref-current-value.expect.md index ab9b4080e44..cba2cb4b4ae 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-dep-on-ref-current-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-dep-on-ref-current-value.expect.md @@ -21,7 +21,7 @@ function Component() { ``` Found 1 error: -Error: Found missing/extra memoization dependencies +Error: Found extra memoization dependencies Extra dependencies can cause a value to update more often than it should, resulting in performance problems such as excessive renders or effects firing too often. diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps-disallow-unused-stable-types.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps-disallow-unused-stable-types.expect.md index 30ab558918d..6c1bddae85f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps-disallow-unused-stable-types.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps-disallow-unused-stable-types.expect.md @@ -25,7 +25,7 @@ function Component() { ``` Found 1 error: -Error: Found missing/extra memoization dependencies +Error: Found extra memoization dependencies Extra dependencies can cause a value to update more often than it should, resulting in performance problems such as excessive renders or effects firing too often. diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md index 557551050b4..2c864f56aff 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md @@ -101,7 +101,7 @@ error.invalid-exhaustive-deps.ts:17:6 Inferred dependencies: `[x?.y.z.a?.b]` -Error: Found missing/extra memoization dependencies +Error: Found extra memoization dependencies Extra dependencies can cause a value to update more often than it should, resulting in performance problems such as excessive renders or effects firing too often. @@ -143,7 +143,7 @@ error.invalid-exhaustive-deps.ts:31:23 Inferred dependencies: `[]` -Error: Found missing/extra memoization dependencies +Error: Found missing memoization dependencies Missing dependencies can cause a value to update less often than it should, resulting in stale UI. diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md index b69607503a4..bb067e7a956 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md @@ -8,13 +8,23 @@ import {useEffect} from 'react'; function Component({x, y, z}) { // error: missing dep - x useEffect(() => { - console.log(x); + log(x); }, []); // error: extra dep - y useEffect(() => { - console.log(x); + log(x); }, [x, y]); + + // error: missing dep - z; extra dep - y + useEffect(() => { + log(x, z); + }, [x, y]); + + // error: missing dep x + useEffect(() => { + log(x); + }, [x.y]); } ``` @@ -23,32 +33,76 @@ function Component({x, y, z}) { ## Error ``` -Found 2 errors: +Found 4 errors: -Error: Found missing/extra memoization dependencies +Error: Found missing effect dependencies -Missing dependencies can cause a value to update less often than it should, resulting in stale UI. +Missing dependencies can cause an effect to fire less often than it should. -error.invalid-exhaustive-effect-deps.ts:7:16 +error.invalid-exhaustive-effect-deps.ts:7:8 5 | // error: missing dep - x 6 | useEffect(() => { -> 7 | console.log(x); - | ^ Missing dependency `x` +> 7 | log(x); + | ^ Missing dependency `x` 8 | }, []); 9 | 10 | // error: extra dep - y -Error: Found missing/extra memoization dependencies +Error: Found extra effect dependencies -Extra dependencies can cause a value to update more often than it should, resulting in performance problems such as excessive renders or effects firing too often. +Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects. error.invalid-exhaustive-effect-deps.ts:13:9 11 | useEffect(() => { - 12 | console.log(x); + 12 | log(x); > 13 | }, [x, y]); | ^ Unnecessary dependency `y` - 14 | } - 15 | + 14 | + 15 | // error: missing dep - z; extra dep - y + 16 | useEffect(() => { + +Error: Found missing/extra effect dependencies + +Missing dependencies can cause an effect to fire less often than it should. Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects. + +error.invalid-exhaustive-effect-deps.ts:17:11 + 15 | // error: missing dep - z; extra dep - y + 16 | useEffect(() => { +> 17 | log(x, z); + | ^ Missing dependency `z` + 18 | }, [x, y]); + 19 | + 20 | // error: missing dep x + +error.invalid-exhaustive-effect-deps.ts:18:9 + 16 | useEffect(() => { + 17 | log(x, z); +> 18 | }, [x, y]); + | ^ Unnecessary dependency `y` + 19 | + 20 | // error: missing dep x + 21 | useEffect(() => { + +Error: Found missing/extra effect dependencies + +Missing dependencies can cause an effect to fire less often than it should. Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects. + +error.invalid-exhaustive-effect-deps.ts:22:8 + 20 | // error: missing dep x + 21 | useEffect(() => { +> 22 | log(x); + | ^ Missing dependency `x` + 23 | }, [x.y]); + 24 | } + 25 | + +error.invalid-exhaustive-effect-deps.ts:23:6 + 21 | useEffect(() => { + 22 | log(x); +> 23 | }, [x.y]); + | ^^^ Overly precise dependency `x.y`, use `x` instead + 24 | } + 25 | ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.js index 7cc279abaf0..b508117acb0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.js @@ -4,11 +4,21 @@ import {useEffect} from 'react'; function Component({x, y, z}) { // error: missing dep - x useEffect(() => { - console.log(x); + log(x); }, []); // error: extra dep - y useEffect(() => { - console.log(x); + log(x); }, [x, y]); + + // error: missing dep - z; extra dep - y + useEffect(() => { + log(x, z); + }, [x, y]); + + // error: missing dep x + useEffect(() => { + log(x); + }, [x.y]); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-inner-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-inner-function.expect.md index 6d03fb42351..2693d3bd05a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-inner-function.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-inner-function.expect.md @@ -26,7 +26,7 @@ function useHook() { ``` Found 1 error: -Error: Found missing/extra memoization dependencies +Error: Found missing memoization dependencies Missing dependencies can cause a value to update less often than it should, resulting in stale UI. diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-unmemoized.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-unmemoized.expect.md index a6868ef325c..bb991d17dad 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-unmemoized.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-unmemoized.expect.md @@ -24,7 +24,7 @@ function useHook() { ``` Found 1 error: -Error: Found missing/extra memoization dependencies +Error: Found missing memoization dependencies Missing dependencies can cause a value to update less often than it should, resulting in stale UI. diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep.expect.md index f3423495097..1bd8e1ba56f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep.expect.md @@ -21,7 +21,7 @@ function useHook() { ``` Found 1 error: -Error: Found missing/extra memoization dependencies +Error: Found missing memoization dependencies Missing dependencies can cause a value to update less often than it should, resulting in stale UI. diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.sketchy-code-exhaustive-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.sketchy-code-exhaustive-deps.expect.md index 0422a07cd87..70be9d35d78 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.sketchy-code-exhaustive-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.sketchy-code-exhaustive-deps.expect.md @@ -25,7 +25,7 @@ function Component() { ``` Found 1 error: -Error: Found missing/extra memoization dependencies +Error: Found missing memoization dependencies Missing dependencies can cause a value to update less often than it should, resulting in stale UI. diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-unrelated-mutation-in-depslist.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-unrelated-mutation-in-depslist.expect.md index 7831aeac15d..fe0bf6c22f6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-unrelated-mutation-in-depslist.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-unrelated-mutation-in-depslist.expect.md @@ -32,7 +32,7 @@ function useFoo(input1) { ``` Found 1 error: -Error: Found missing/extra memoization dependencies +Error: Found missing memoization dependencies Missing dependencies can cause a value to update less often than it should, resulting in stale UI. From 2ce8e3a452f0472b773b5a296a45502fa3c36c5a Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Thu, 4 Dec 2025 18:10:49 -0500 Subject: [PATCH 4/6] Handle useEffectEvent - should never be included in effect deps --- .../ValidateExhaustiveDependencies.ts | 19 ++++ ...or.exhaustive-deps-effect-events.expect.md | 80 ++++++++++++++ .../error.exhaustive-deps-effect-events.js | 27 +++++ ...ctive-stable-types-as-extra-deps.expect.md | 58 +++++++++- ...-nonreactive-stable-types-as-extra-deps.js | 21 +++- .../exhaustive-deps-effect-events.expect.md | 104 ++++++++++++++++++ .../exhaustive-deps-effect-events.js | 22 ++++ 7 files changed, 325 insertions(+), 6 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index e22e63ead50..81a5dbfb200 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -25,6 +25,7 @@ import { Identifier, IdentifierId, InstructionKind, + isEffectEventFunctionType, isPrimitiveType, isStableType, isSubPath, @@ -317,6 +318,12 @@ function validateDependencies( reason: 'Unexpected function dependency', loc: inferredDependency.loc, }); + /** + * Skip effect event functions as they are not valid dependencies + */ + if (isEffectEventFunctionType(inferredDependency.identifier)) { + continue; + } let hasMatchingManualDependency = false; for (const manualDependency of manualDependencies) { if ( @@ -339,6 +346,7 @@ function validateDependencies( ) { continue; } + missing.push(inferredDependency); } @@ -416,6 +424,17 @@ function validateDependencies( }, ); if ( + matchingInferred != null && + isEffectEventFunctionType(matchingInferred.identifier) + ) { + diagnostic.withDetails({ + kind: 'error', + message: + `Functions returned from \`useEffectEvent\` must not be included in the dependency array. ` + + `Remove \`${printManualMemoDependency(dep)}\` from the dependencies.`, + loc: dep.loc ?? manualMemoLoc, + }); + } else if ( matchingInferred != null && !isOptionalDependency(matchingInferred, reactive) ) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.expect.md new file mode 100644 index 00000000000..5e44239950f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.expect.md @@ -0,0 +1,80 @@ + +## Input + +```javascript +// @validateExhaustiveEffectDependencies +import {useEffect, useEffectEvent} from 'react'; + +function Component({x, y, z}) { + const effectEvent = useEffectEvent(() => { + log(x); + }); + + const effectEvent2 = useEffectEvent(z => { + log(y, z); + }); + + // error - do not include effect event in deps + useEffect(() => { + effectEvent(); + }, [effectEvent]); + + // error - do not include effect event in deps + useEffect(() => { + effectEvent2(z); + }, [effectEvent2, z]); + + // error - do not include effect event captured values in deps + useEffect(() => { + effectEvent2(z); + }, [y, z]); +} + +``` + + +## Error + +``` +Found 3 errors: + +Error: Found extra effect dependencies + +Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects. + +error.exhaustive-deps-effect-events.ts:16:6 + 14 | useEffect(() => { + 15 | effectEvent(); +> 16 | }, [effectEvent]); + | ^^^^^^^^^^^ Functions returned from `useEffectEvent` must not be included in the dependency array. Remove `effectEvent` from the dependencies. + 17 | + 18 | // error - do not include effect event in deps + 19 | useEffect(() => { + +Error: Found extra effect dependencies + +Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects. + +error.exhaustive-deps-effect-events.ts:21:6 + 19 | useEffect(() => { + 20 | effectEvent2(z); +> 21 | }, [effectEvent2, z]); + | ^^^^^^^^^^^^ Functions returned from `useEffectEvent` must not be included in the dependency array. Remove `effectEvent2` from the dependencies. + 22 | + 23 | // error - do not include effect event captured values in deps + 24 | useEffect(() => { + +Error: Found extra effect dependencies + +Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects. + +error.exhaustive-deps-effect-events.ts:26:6 + 24 | useEffect(() => { + 25 | effectEvent2(z); +> 26 | }, [y, z]); + | ^ Unnecessary dependency `y` + 27 | } + 28 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.js new file mode 100644 index 00000000000..f8e9c8d700e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.js @@ -0,0 +1,27 @@ +// @validateExhaustiveEffectDependencies +import {useEffect, useEffectEvent} from 'react'; + +function Component({x, y, z}) { + const effectEvent = useEffectEvent(() => { + log(x); + }); + + const effectEvent2 = useEffectEvent(z => { + log(y, z); + }); + + // error - do not include effect event in deps + useEffect(() => { + effectEvent(); + }, [effectEvent]); + + // error - do not include effect event in deps + useEffect(() => { + effectEvent2(z); + }, [effectEvent2, z]); + + // error - do not include effect event captured values in deps + useEffect(() => { + effectEvent2(z); + }, [y, z]); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md index 5b6319ed7cf..2c449ddc5e5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateExhaustiveMemoizationDependencies +// @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies import { useCallback, useTransition, @@ -11,6 +11,7 @@ import { useActionState, useRef, useReducer, + useEffect, } from 'react'; function useFoo() { @@ -21,6 +22,24 @@ function useFoo() { const [v, dispatch] = useReducer(() => {}, null); const [isPending, dispatchAction] = useActionState(() => {}, null); + useEffect(() => { + dispatch(); + startTransition(() => {}); + addOptimistic(); + setState(null); + dispatchAction(); + ref.current = true; + }, [ + // intentionally adding unnecessary deps on nonreactive stable values + // to check that they're allowed + dispatch, + startTransition, + addOptimistic, + setState, + dispatchAction, + ref, + ]); + return useCallback(() => { dispatch(); startTransition(() => {}); @@ -50,7 +69,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies +import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies import { useCallback, useTransition, @@ -59,10 +78,11 @@ import { useActionState, useRef, useReducer, + useEffect, } from "react"; function useFoo() { - const $ = _c(1); + const $ = _c(3); const [, setState] = useState(); const ref = useRef(null); const [, startTransition] = useTransition(); @@ -70,6 +90,7 @@ function useFoo() { const [, dispatch] = useReducer(_temp, null); const [, dispatchAction] = useActionState(_temp2, null); let t0; + let t1; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = () => { dispatch(); @@ -79,12 +100,38 @@ function useFoo() { dispatchAction(); ref.current = true; }; + t1 = [ + dispatch, + startTransition, + addOptimistic, + setState, + dispatchAction, + ref, + ]; $[0] = t0; + $[1] = t1; } else { t0 = $[0]; + t1 = $[1]; + } + useEffect(t0, t1); + let t2; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t2 = () => { + dispatch(); + startTransition(_temp4); + addOptimistic(); + setState(null); + dispatchAction(); + ref.current = true; + }; + $[2] = t2; + } else { + t2 = $[2]; } - return t0; + return t2; } +function _temp4() {} function _temp3() {} function _temp2() {} function _temp() {} @@ -97,4 +144,5 @@ export const FIXTURE_ENTRYPOINT = { ``` ### Eval output -(kind: ok) "[[ function params=0 ]]" \ No newline at end of file +(kind: ok) "[[ function params=0 ]]" +logs: ['An optimistic state update occurred outside a transition or action. To fix, move the update to an action, or wrap with startTransition.'] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js index 71000afcc42..721832eac1b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js @@ -1,4 +1,4 @@ -// @validateExhaustiveMemoizationDependencies +// @validateExhaustiveMemoizationDependencies @validateExhaustiveEffectDependencies import { useCallback, useTransition, @@ -7,6 +7,7 @@ import { useActionState, useRef, useReducer, + useEffect, } from 'react'; function useFoo() { @@ -17,6 +18,24 @@ function useFoo() { const [v, dispatch] = useReducer(() => {}, null); const [isPending, dispatchAction] = useActionState(() => {}, null); + useEffect(() => { + dispatch(); + startTransition(() => {}); + addOptimistic(); + setState(null); + dispatchAction(); + ref.current = true; + }, [ + // intentionally adding unnecessary deps on nonreactive stable values + // to check that they're allowed + dispatch, + startTransition, + addOptimistic, + setState, + dispatchAction, + ref, + ]); + return useCallback(() => { dispatch(); startTransition(() => {}); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.expect.md new file mode 100644 index 00000000000..3b0e696911a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.expect.md @@ -0,0 +1,104 @@ + +## Input + +```javascript +// @validateExhaustiveEffectDependencies +import {useEffect, useEffectEvent} from 'react'; + +function Component({x, y, z}) { + const effectEvent = useEffectEvent(() => { + log(x); + }); + + const effectEvent2 = useEffectEvent(z => { + log(y, z); + }); + + // ok - effectEvent not included in deps + useEffect(() => { + effectEvent(); + }, []); + + // ok - effectEvent2 not included in deps, z included + useEffect(() => { + effectEvent2(z); + }, [z]); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveEffectDependencies +import { useEffect, useEffectEvent } from "react"; + +function Component(t0) { + const $ = _c(12); + const { x, y, z } = t0; + let t1; + if ($[0] !== x) { + t1 = () => { + log(x); + }; + $[0] = x; + $[1] = t1; + } else { + t1 = $[1]; + } + const effectEvent = useEffectEvent(t1); + let t2; + if ($[2] !== y) { + t2 = (z_0) => { + log(y, z_0); + }; + $[2] = y; + $[3] = t2; + } else { + t2 = $[3]; + } + const effectEvent2 = useEffectEvent(t2); + let t3; + if ($[4] !== effectEvent) { + t3 = () => { + effectEvent(); + }; + $[4] = effectEvent; + $[5] = t3; + } else { + t3 = $[5]; + } + let t4; + if ($[6] === Symbol.for("react.memo_cache_sentinel")) { + t4 = []; + $[6] = t4; + } else { + t4 = $[6]; + } + useEffect(t3, t4); + let t5; + if ($[7] !== effectEvent2 || $[8] !== z) { + t5 = () => { + effectEvent2(z); + }; + $[7] = effectEvent2; + $[8] = z; + $[9] = t5; + } else { + t5 = $[9]; + } + let t6; + if ($[10] !== z) { + t6 = [z]; + $[10] = z; + $[11] = t6; + } else { + t6 = $[11]; + } + useEffect(t5, t6); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.js new file mode 100644 index 00000000000..1c8c710d6cb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.js @@ -0,0 +1,22 @@ +// @validateExhaustiveEffectDependencies +import {useEffect, useEffectEvent} from 'react'; + +function Component({x, y, z}) { + const effectEvent = useEffectEvent(() => { + log(x); + }); + + const effectEvent2 = useEffectEvent(z => { + log(y, z); + }); + + // ok - effectEvent not included in deps + useEffect(() => { + effectEvent(); + }, []); + + // ok - effectEvent2 not included in deps, z included + useEffect(() => { + effectEvent2(z); + }, [z]); +} From e1872717efdf146b0670b10d71e52713c625e27f Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Fri, 5 Dec 2025 10:35:19 -0500 Subject: [PATCH 5/6] Don't pass env to validator --- .../babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts | 2 +- .../src/Validation/ValidateExhaustiveDependencies.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 3d2b5f6e743..ab5ac590d90 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -309,7 +309,7 @@ function runWithEnvironment( env.config.validateExhaustiveEffectDependencies ) { // NOTE: this relies on reactivity inference running first - validateExhaustiveDependencies(hir, env).unwrap(); + validateExhaustiveDependencies(hir).unwrap(); } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index 81a5dbfb200..bb3d4cd1d5e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -18,7 +18,6 @@ import { areEqualPaths, BlockId, DependencyPath, - Environment, FinishMemoize, GeneratedSource, HIRFunction, @@ -89,8 +88,8 @@ const DEBUG = false; */ export function validateExhaustiveDependencies( fn: HIRFunction, - env: Environment, ): Result { + const env = fn.env; const reactive = collectReactiveIdentifiersHIR(fn); const temporaries: Map = new Map(); From 6da57774f063e13ded5baa34d08b6944c0d34ca3 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Fri, 5 Dec 2025 18:07:10 -0500 Subject: [PATCH 6/6] Support suggestions for effect deps --- .../ValidateExhaustiveDependencies.ts | 23 +++++++++++++++---- ...or.exhaustive-deps-effect-events.expect.md | 6 +++++ ...r.invalid-exhaustive-effect-deps.expect.md | 8 +++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts index bb3d4cd1d5e..e364c025f38 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -158,7 +158,7 @@ export function validateExhaustiveDependencies( { onStartMemoize, onFinishMemoize, - onEffect: (inferred, manual) => { + onEffect: (inferred, manual, manualMemoLoc) => { if (env.config.validateExhaustiveEffectDependencies === false) { return; } @@ -199,7 +199,7 @@ export function validateExhaustiveDependencies( Array.from(inferred), manualDeps, reactive, - null, + manualMemoLoc, ErrorCategory.EffectExhaustiveDependencies, ); if (diagnostic != null) { @@ -379,7 +379,10 @@ function validateDependencies( op: CompilerSuggestionOperation.Replace, text: `[${inferred .filter( - dep => dep.kind === 'Local' && !isOptionalDependency(dep, reactive), + dep => + dep.kind === 'Local' && + !isOptionalDependency(dep, reactive) && + !isEffectEventFunctionType(dep.identifier), ) .map(printInferredDependency) .join(', ')}]`, @@ -559,6 +562,7 @@ function collectDependencies( onEffect: ( inferred: Set, manual: Set, + manualMemoLoc: SourceLocation | null, ) => void; } | null, isFunctionExpression: boolean, @@ -778,6 +782,7 @@ function collectDependencies( temporaries.set(lvalue.identifier.id, { kind: 'Aggregate', dependencies: arrayDeps, + loc: value.loc, }); break; } @@ -796,7 +801,11 @@ function collectDependencies( fnDeps?.kind === 'Aggregate' && manualDeps?.kind === 'Aggregate' ) { - onEffect(fnDeps.dependencies, manualDeps.dependencies); + onEffect( + fnDeps.dependencies, + manualDeps.dependencies, + manualDeps.loc ?? null, + ); } } } @@ -893,7 +902,11 @@ type Temporary = context: boolean; loc: SourceLocation; } - | {kind: 'Aggregate'; dependencies: Set}; + | { + kind: 'Aggregate'; + dependencies: Set; + loc?: SourceLocation; + }; type InferredDependency = Extract; function collectReactiveIdentifiersHIR(fn: HIRFunction): Set { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.expect.md index 5e44239950f..e2ab53dd05f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.expect.md @@ -51,6 +51,8 @@ error.exhaustive-deps-effect-events.ts:16:6 18 | // error - do not include effect event in deps 19 | useEffect(() => { +Inferred dependencies: `[]` + Error: Found extra effect dependencies Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects. @@ -64,6 +66,8 @@ error.exhaustive-deps-effect-events.ts:21:6 23 | // error - do not include effect event captured values in deps 24 | useEffect(() => { +Inferred dependencies: `[z]` + Error: Found extra effect dependencies Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects. @@ -75,6 +79,8 @@ error.exhaustive-deps-effect-events.ts:26:6 | ^ Unnecessary dependency `y` 27 | } 28 | + +Inferred dependencies: `[z]` ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md index bb067e7a956..78332c7071b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md @@ -48,6 +48,8 @@ error.invalid-exhaustive-effect-deps.ts:7:8 9 | 10 | // error: extra dep - y +Inferred dependencies: `[x]` + Error: Found extra effect dependencies Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects. @@ -61,6 +63,8 @@ error.invalid-exhaustive-effect-deps.ts:13:9 15 | // error: missing dep - z; extra dep - y 16 | useEffect(() => { +Inferred dependencies: `[x]` + Error: Found missing/extra effect dependencies Missing dependencies can cause an effect to fire less often than it should. Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects. @@ -83,6 +87,8 @@ error.invalid-exhaustive-effect-deps.ts:18:9 20 | // error: missing dep x 21 | useEffect(() => { +Inferred dependencies: `[x, z]` + Error: Found missing/extra effect dependencies Missing dependencies can cause an effect to fire less often than it should. Extra dependencies can cause an effect to fire more often than it should, resulting in performance problems such as excessive renders and side effects. @@ -103,6 +109,8 @@ error.invalid-exhaustive-effect-deps.ts:23:6 | ^^^ Overly precise dependency `x.y`, use `x` instead 24 | } 25 | + +Inferred dependencies: `[x]` ``` \ No newline at end of file