OSSForks/react
1
0
Fork 0
mirror of https://github.com/zebrajr/react.git synced 2026-01-15 12:15:22 +00:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

[compiler][poc] Reuse ValidateExhaustiveDeps for effect dep validation (#35285)

Browse Source 
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.
* 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

---------

Co-authored-by: Jack Pope <jackpope1@gmail.com>
This commit is contained in:
Joseph Savona
2025-12-08 07:58:38 -08:00
committed by GitHub
parent 380778d296
commit ec9cc003d2
20 changed files with 940 additions and 319 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts
View File

@@ -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,

5
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts
View File

@@ -304,7 +304,10 @@ 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();
}

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

@@ -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

712
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts
View File

@@ -10,6 +10,7 @@ import {
CompilerDiagnostic,
CompilerError,
CompilerSuggestionOperation,
Effect,
SourceLocation,
} from '..';
import {CompilerSuggestion, ErrorCategory} from '../CompilerError';
@@ -18,10 +19,12 @@ import {
BlockId,
DependencyPath,
FinishMemoize,
GeneratedSource,
HIRFunction,
Identifier,
IdentifierId,
InstructionKind,
isEffectEventFunctionType,
isPrimitiveType,
isStableType,
isSubPath,
@@ -40,6 +43,7 @@ import {
} from '../HIR/visitors';
import {Result} from '../Utils/Result';
import {retainWhere} from '../Utils/utils';
import {isEffectHook} from './ValidateMemoizedEffectDependencies';
const DEBUG = false;
@@ -85,6 +89,7 @@ const DEBUG = false;
export function validateExhaustiveDependencies(
fn: HIRFunction,
): Result<void, CompilerError> {
const env = fn.env;
const reactive = collectReactiveIdentifiersHIR(fn);
const temporaries: Map<IdentifierId, Temporary> = new Map();
@@ -126,253 +131,20 @@ export function validateExhaustiveDependencies(
loc: value.loc,
},
);
visitCandidateDependency(value.decl, temporaries, dependencies, locals);
const inferred: Array<InferredDependency> = 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;
}
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),
);
}
}
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(
(startMemo.deps ?? [])
.map(x => ' ' + printManualMemoDependency(x))
.join('\n'),
);
console.log('inferred');
console.log(
inferred.map(x => ' ' + printInferredDependency(x)).join('\n'),
);
}
const manualDependencies = startMemo.deps ?? [];
const matched: Set<ManualMemoDependency> = new Set();
const missing: Array<Extract<InferredDependency, {kind: 'Local'}>> = [];
const extra: Array<ManualMemoDependency> = [];
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);
}
}
continue;
}
CompilerError.simpleInvariant(inferredDependency.kind === 'Local', {
reason: 'Unexpected function dependency',
loc: value.loc,
});
let hasMatchingManualDependency = false;
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,
))
) {
hasMatchingManualDependency = true;
matched.add(manualDependency);
}
}
if (
hasMatchingManualDependency ||
isOptionalDependency(inferredDependency, reactive)
) {
continue;
}
missing.push(inferredDependency);
}
if (env.config.validateExhaustiveMemoizationDependencies) {
visitCandidateDependency(value.decl, temporaries, dependencies, locals);
const inferred: Array<InferredDependency> = Array.from(dependencies);
for (const dep of startMemo.deps ?? []) {
if (matched.has(dep)) {
continue;
const diagnostic = validateDependencies(
inferred,
startMemo.deps ?? [],
reactive,
startMemo.depsLoc,
ErrorCategory.MemoDependencies,
);
if (diagnostic != null) {
error.pushDiagnostic(diagnostic);
}
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(', ')}]`,
};
}
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';
}
diagnostic.withDetails({
kind: 'error',
message: `Missing dependency \`${printInferredDependency(dep)}\`${reactiveStableValueHint}`,
loc: dep.loc,
});
}
for (const dep of extra) {
if (dep.root.kind === 'Global') {
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,
});
error.pushDiagnostic(diagnostic);
} else {
const root = dep.root.value;
const matchingInferred = inferred.find(
(
inferredDep,
): inferredDep is Extract<InferredDependency, {kind: 'Local'}> => {
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,
});
}
}
}
if (suggestion != null) {
diagnostic.withDetails({
kind: 'hint',
message: `Inferred dependencies: \`${suggestion.text}\``,
});
}
error.pushDiagnostic(diagnostic);
}
dependencies.clear();
@@ -386,18 +158,325 @@ export function validateExhaustiveDependencies(
{
onStartMemoize,
onFinishMemoize,
onEffect: (inferred, manual, manualMemoLoc) => {
if (env.config.validateExhaustiveEffectDependencies === false) {
return;
}
if (DEBUG) {
console.log(Array.from(inferred, printInferredDependency));
console.log(Array.from(manual, printInferredDependency));
}
const manualDeps: Array<ManualMemoDependency> = [];
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,
});
}
}
const diagnostic = validateDependencies(
Array.from(inferred),
manualDeps,
reactive,
manualMemoLoc,
ErrorCategory.EffectExhaustiveDependencies,
);
if (diagnostic != null) {
error.pushDiagnostic(diagnostic);
}
},
},
false, // isFunctionExpression
);
return error.asResult();
}
function validateDependencies(
inferred: Array<InferredDependency>,
manualDependencies: Array<ManualMemoDependency>,
reactive: Set<IdentifierId>,
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) => {
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;
}
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),
);
}
}
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<ManualMemoDependency> = new Set();
const missing: Array<Extract<InferredDependency, {kind: 'Local'}>> = [];
const extra: Array<ManualMemoDependency> = [];
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);
}
}
continue;
}
CompilerError.simpleInvariant(inferredDependency.kind === 'Local', {
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 (
manualDependency.root.kind === 'NamedLocal' &&
manualDependency.root.value.identifier.id ===
inferredDependency.identifier.id &&
(areEqualPaths(manualDependency.path, inferredDependency.path) ||
isSubPathIgnoringOptionals(
manualDependency.path,
inferredDependency.path,
))
) {
hasMatchingManualDependency = true;
matched.add(manualDependency);
}
}
if (
hasMatchingManualDependency ||
isOptionalDependency(inferredDependency, reactive)
) {
continue;
}
missing.push(inferredDependency);
}
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 (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) &&
!isEffectEventFunctionType(dep.identifier),
)
.map(printInferredDependency)
.join(', ')}]`,
};
}
const diagnostic = createDiagnostic(category, missing, extra, suggestion);
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';
}
diagnostic.withDetails({
kind: 'error',
message: `Missing dependency \`${printInferredDependency(dep)}\`${reactiveStableValueHint}`,
loc: dep.loc,
});
}
for (const dep of extra) {
if (dep.root.kind === 'Global') {
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 ?? manualMemoLoc,
});
} else {
const root = dep.root.value;
const matchingInferred = inferred.find(
(
inferredDep,
): inferredDep is Extract<InferredDependency, {kind: 'Local'}> => {
return (
inferredDep.kind === 'Local' &&
inferredDep.identifier.id === root.identifier.id &&
isSubPathIgnoringOptionals(inferredDep.path, dep.path)
);
},
);
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)
) {
diagnostic.withDetails({
kind: 'error',
message:
`Overly precise dependency \`${printManualMemoDependency(dep)}\`, ` +
`use \`${printInferredDependency(matchingInferred)}\` instead`,
loc: dep.loc ?? manualMemoLoc,
});
} 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 ?? manualMemoLoc,
});
}
}
}
if (suggestion != null) {
diagnostic.withDetails({
kind: 'hint',
message: `Inferred dependencies: \`${suggestion.text}\``,
});
}
return diagnostic;
}
return null;
}
function addDependency(
dep: Temporary,
dependencies: Set<InferredDependency>,
locals: Set<IdentifierId>,
): void {
if (dep.kind === 'Function') {
if (dep.kind === 'Aggregate') {
for (const x of dep.dependencies) {
addDependency(x, dependencies, locals);
}
@@ -480,9 +559,14 @@ function collectDependencies(
dependencies: Set<InferredDependency>,
locals: Set<IdentifierId>,
) => void;
onEffect: (
inferred: Set<InferredDependency>,
manual: Set<InferredDependency>,
manualMemoLoc: SourceLocation | null,
) => void;
} | null,
isFunctionExpression: boolean,
): Extract<Temporary, {kind: 'Function'}> {
): Extract<Temporary, {kind: 'Aggregate'}> {
const optionals = findOptionalPlaces(fn);
if (DEBUG) {
console.log(prettyFormat(optionals));
@@ -501,25 +585,25 @@ function collectDependencies(
}
for (const block of fn.body.blocks.values()) {
for (const phi of block.phis) {
let deps: Array<Temporary> | null = null;
const deps: Array<InferredDependency> = [];
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 +621,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 +629,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 +767,55 @@ function collectDependencies(
}
break;
}
case 'ArrayExpression': {
const arrayDeps: Set<InferredDependency> = 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,
loc: value.loc,
});
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,
manualDeps.loc ?? null,
);
}
}
}
// 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 +839,7 @@ function collectDependencies(
visit(operand);
}
}
return {kind: 'Function', dependencies};
return {kind: 'Aggregate', dependencies};
}
function printInferredDependency(dep: InferredDependency): string {
@@ -748,7 +877,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 +902,11 @@ type Temporary =
context: boolean;
loc: SourceLocation;
}
| {kind: 'Function'; dependencies: Set<Temporary>};
| {
kind: 'Aggregate';
dependencies: Set<InferredDependency>;
loc?: SourceLocation;
};
type InferredDependency = Extract<Temporary, {kind: 'Local' | 'Global'}>;
function collectReactiveIdentifiersHIR(fn: HIRFunction): Set<IdentifierId> {
@@ -888,3 +1021,64 @@ function isOptionalDependency(
isPrimitiveType(inferredDependency.identifier))
);
}
function createDiagnostic(
category:
| ErrorCategory.MemoDependencies
| ErrorCategory.EffectExhaustiveDependencies,
missing: Array<InferredDependency>,
extra: Array<ManualMemoDependency>,
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,
});
}

86
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.expect.md Normal file
View File

@@ -0,0 +1,86 @@
## 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(() => {
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.
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(() => {
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.
error.exhaustive-deps-effect-events.ts:26:6
24 | useEffect(() => {
25 | effectEvent2(z);
> 26 | }, [y, z]);
| ^ Unnecessary dependency `y`
27 | }
28 |
Inferred dependencies: `[z]`
```

27
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.exhaustive-deps-effect-events.js Normal file
View File

@@ -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]);
}

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-dep-on-ref-current-value.expect.md
View File

@@ -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.

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps-disallow-unused-stable-types.expect.md
View File

@@ -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.

48
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-deps.expect.md
View File

@@ -51,7 +51,7 @@ function Component({x, y, z}) {
## Error
```
Found 5 errors:
Found 4 errors:
Error: Found missing/extra memoization dependencies
@@ -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,49 +143,7 @@ error.invalid-exhaustive-deps.ts:31:23
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
Error: Found missing memoization dependencies
Missing dependencies can cause a value to update less often than it should, resulting in stale UI.

116
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.expect.md Normal file
View File

@@ -0,0 +1,116 @@
## Input
```javascript
// @validateExhaustiveEffectDependencies
import {useEffect} from 'react';
function Component({x, y, z}) {
// error: missing dep - x
useEffect(() => {
log(x);
}, []);
// error: extra dep - y
useEffect(() => {
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]);
}
```
## Error
```
Found 4 errors:
Error: Found missing effect dependencies
Missing dependencies can cause an effect to fire less often than it should.
error.invalid-exhaustive-effect-deps.ts:7:8
5 | // error: missing dep - x
6 | useEffect(() => {
> 7 | log(x);
| ^ Missing dependency `x`
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.
error.invalid-exhaustive-effect-deps.ts:13:9
11 | useEffect(() => {
12 | log(x);
> 13 | }, [x, y]);
| ^ Unnecessary dependency `y`
14 |
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.
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(() => {
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.
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 |
Inferred dependencies: `[x]`
```

24
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-exhaustive-effect-deps.js Normal file
View File

@@ -0,0 +1,24 @@
// @validateExhaustiveEffectDependencies
import {useEffect} from 'react';
function Component({x, y, z}) {
// error: missing dep - x
useEffect(() => {
log(x);
}, []);
// error: extra dep - y
useEffect(() => {
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]);
}

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-inner-function.expect.md
View File

@@ -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.

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep-unmemoized.expect.md
View File

@@ -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.

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.invalid-missing-nonreactive-dep.expect.md
View File

@@ -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.

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/error.sketchy-code-exhaustive-deps.expect.md
View File

@@ -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.

58
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.expect.md
View File

@@ -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];
}
return t0;
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 t2;
}
function _temp4() {}
function _temp3() {}
function _temp2() {}
function _temp() {}
@@ -97,4 +144,5 @@ export const FIXTURE_ENTRYPOINT = {
```
### Eval output
(kind: ok) "[[ function params=0 ]]"
(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.']

21
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-allow-nonreactive-stable-types-as-extra-deps.js
View File

@@ -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(() => {});

104
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.expect.md Normal file
View File

@@ -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

22
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps/exhaustive-deps-effect-events.js Normal file
View File

@@ -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]);
}

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.useMemo-unrelated-mutation-in-depslist.expect.md
View File

@@ -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.