From 40ba0571c2f7256c19a1191fbbcc03822a573388 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 9 Sep 2025 19:34:52 -0700 Subject: [PATCH] [compiler] Implement exhaustive dependency checking for manual memoization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compiler currently drops manual memoization and rewrites it using its own inference. If the existing manual memo dependencies has missing or extra dependencies, compilation can change behavior by running the computation more often (if deps were missing) or less often (if there were extra deps). We currently address this by relying on the developer to use the ESLint plugin and have `eslint-disable-next-line react-hooks/exhaustive-deps` suppressions in their code. If a suppression exists, we skip compilation. But not everyone is using the linter! Relying on the linter is also imprecise since it forces us to bail out on exhaustive-deps checks that only effect (ahem) effects — and while it isn't good to have incorrect deps on effects, it isn't a problem for compilation. So this PR is a rough sketch of validating manual memoization dependencies in the compiler. Long-term we could use this to also check effect deps and replace the ExhaustiveDeps lint rule, but for now I'm focused specifically on manual memoization use-cases. If this works, we can stop bailing out on ESLint suppressions, since the compiler will implement all the appropriate checks (we already check rules of hooks). --- .../src/CompilerError.ts | 24 + .../src/Entrypoint/Pipeline.ts | 6 + .../src/HIR/Environment.ts | 5 + .../src/HIR/HIR.ts | 22 + .../ValidateExhaustiveDependencies.ts | 624 ++++++++++++++++++ .../error.invalid-exhaustive-deps.expect.md | 84 +++ .../compiler/error.invalid-exhaustive-deps.js | 24 + .../compiler/exhaustive-deps.expect.md | 121 ++++ .../fixtures/compiler/exhaustive-deps.js | 24 + 9 files changed, 934 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts index 2e7acac254..38b5507581 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts @@ -282,6 +282,30 @@ export class CompilerError extends Error { disabledDetails: Array = []; printedMessage: string | null = null; + static simpleInvariant( + condition: unknown, + options: { + reason: CompilerDiagnosticOptions['reason']; + description?: CompilerDiagnosticOptions['description']; + loc: SourceLocation; + }, + ): asserts condition { + if (!condition) { + const errors = new CompilerError(); + errors.pushDiagnostic( + CompilerDiagnostic.create({ + reason: options.reason, + description: options.description ?? null, + category: ErrorCategory.Invariant, + }).withDetails({ + kind: 'error', + loc: options.loc, + message: options.reason, + }), + ); + throw errors; + } + } static invariant( condition: unknown, options: Omit, 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 e0b0536f28..f0d5d5acae 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -104,6 +104,7 @@ import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEf import {inferMutationAliasingRanges} from '../Inference/InferMutationAliasingRanges'; import {validateNoDerivedComputationsInEffects} from '../Validation/ValidateNoDerivedComputationsInEffects'; import {nameAnonymousFunctions} from '../Transform/NameAnonymousFunctions'; +import {validateExhaustiveDependencies} from '../Validation/ValidateExhaustiveDependencies'; export type CompilerPipelineValue = | {kind: 'ast'; name: string; value: CodegenFunction} @@ -293,6 +294,11 @@ function runWithEnvironment( inferReactivePlaces(hir); log({kind: 'hir', name: 'InferReactivePlaces', value: hir}); + if (env.config.validateExhaustiveMemoizationDependencies) { + // NOTE: this relies on reactivity inference running first + validateExhaustiveDependencies(hir).unwrap(); + } + rewriteInstructionKindsBasedOnReassignment(hir); log({ kind: 'hir', 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 8e6816a3d5..5e9b89ffa4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -227,6 +227,11 @@ export const EnvironmentConfigSchema = z.object({ */ validatePreserveExistingMemoizationGuarantees: z.boolean().default(true), + /** + * Validate that dependencies supplied to manual memoization calls are exhaustive. + */ + validateExhaustiveMemoizationDependencies: 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/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 4d2d4ed80d..bf70021001 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1680,6 +1680,28 @@ export function areEqualPaths(a: DependencyPath, b: DependencyPath): boolean { ) ); } +export function isSubPath( + subpath: DependencyPath, + path: DependencyPath, +): boolean { + return ( + subpath.length <= path.length && + subpath.every( + (item, ix) => + item.property === path[ix].property && + item.optional === path[ix].optional, + ) + ); +} +export function isSubPathIgnoringOptionals( + subpath: DependencyPath, + path: DependencyPath, +): boolean { + return ( + subpath.length <= path.length && + subpath.every((item, ix) => item.property === path[ix].property) + ); +} export function getPlaceScope( id: InstructionId, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts new file mode 100644 index 0000000000..efa4803668 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -0,0 +1,624 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import prettyFormat from 'pretty-format'; +import {CompilerDiagnostic, CompilerError, SourceLocation} from '..'; +import {ErrorCategory} from '../CompilerError'; +import { + areEqualPaths, + BlockId, + DependencyPath, + FinishMemoize, + HIRFunction, + Identifier, + IdentifierId, + InstructionKind, + isSubPath, + isSubPathIgnoringOptionals, + LoadGlobal, + ManualMemoDependency, + Place, + StartMemoize, +} from '../HIR'; +import {printIdentifier, printManualMemoDependency} from '../HIR/PrintHIR'; +import { + eachInstructionLValue, + eachInstructionValueLValue, + eachInstructionValueOperand, + eachTerminalOperand, +} from '../HIR/visitors'; +import {Result} from '../Utils/Result'; + +const DEBUG = false; + +/** + * Validates that existing manual memoization had exhaustive dependencies. + * Memoization with missing or extra reactive dependencies is invalid React + * and compilation can change behavior, causing a value to be computed more + * or less times. + * + * TODOs: + * - Better handling of cases where we infer multiple dependencies related to a single + * variable. Eg if the user has dep `x` and we inferred `x.y, x.z`, the user's dep + * is sufficient. + * - Handle cases where the user deps were not simple identifiers + property chains. + * We try to detect this in ValidateUseMemo but we miss some cases. The problem + * is that invalid forms can be value blocks or function calls that don't get + * removed by DCE, leaving a structure like: + * + * StartMemoize + * t0 = + * ...non-DCE'd code for manual deps... + * FinishMemoize decl=t0 + * + * When we go to compute the dependencies, we then think that the user's manual dep + * logic is part of what the memo computation logic. + */ +export function validateExhaustiveDependencies( + fn: HIRFunction, +): Result { + const reactive = collectReactiveIdentifiersHIR(fn); + + const temporaries: Map = new Map(); + for (const param of fn.params) { + const place = param.kind === 'Identifier' ? param : param.place; + temporaries.set(place.identifier.id, { + kind: 'Local', + identifier: place.identifier, + path: [], + context: false, + loc: place.loc, + }); + } + const error = new CompilerError(); + let startMemo: StartMemoize | null = null; + + function onStartMemoize( + value: StartMemoize, + dependencies: Set, + locals: Set, + ): void { + CompilerError.simpleInvariant(startMemo == null, { + reason: 'Unexpected nested memo calls', + loc: value.loc, + }); + startMemo = value; + dependencies.clear(); + locals.clear(); + } + function onFinishMemoize( + value: FinishMemoize, + dependencies: Set, + locals: Set, + ): void { + CompilerError.simpleInvariant( + startMemo != null && startMemo.manualMemoId === value.manualMemoId, + { + reason: 'Found FinishMemoize without corresponding StartMemoize', + loc: value.loc, + }, + ); + visitCandidateDependency(value.decl, temporaries, dependencies); + const inferred: Array = []; + for (const dep of dependencies) { + if (inferred.find(x => isEqualTemporary(x, dep)) != null) { + continue; + } + inferred.push(dep); + } + // Validate that all manual dependencies belong there + if (DEBUG) { + console.log('manual'); + console.log( + (startMemo.deps ?? []) + .map(x => ' ' + printManualMemoDependency(x, false)) + .join('\n'), + ); + console.log('inferred'); + console.log(inferred.map(x => ' ' + _printTemporary(x)).join('\n')); + } + const manualDependencies = startMemo.deps ?? []; + const matched: Set = new Set(); + for (const inferredDependency of inferred) { + if (inferredDependency.kind === 'Global') { + continue; + } else if (inferredDependency.kind === 'Function') { + CompilerError.simpleInvariant(false, { + 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) || + isSubPath(manualDependency.path, inferredDependency.path)) + ) { + hasMatchingManualDependency = true; + matched.add(manualDependency); + } + } + if (!hasMatchingManualDependency) { + /** + * Find any "extra" dependencies that are more precise versions of the dependency + * For example, the dep may be `x.y`, if the user specified `x.y.z` we want to give + * a hint + */ + const morePreciseDependencies = []; + for (const manualDependency of manualDependencies) { + if ( + manualDependency.root.kind === 'NamedLocal' && + manualDependency.root.value.identifier.id === + inferredDependency.identifier.id && + isSubPathIgnoringOptionals( + inferredDependency.path, + manualDependency.path, + ) + ) { + matched.add(manualDependency); + morePreciseDependencies.push(manualDependency); + } + } + + const diagnostic = CompilerDiagnostic.create({ + category: ErrorCategory.PreserveManualMemo, + reason: 'Found missing memoization dependency', + description: + 'Missing dependencies can cause a value not to update when those inputs change, ' + + 'resulting in stale UI. This memoization cannot be safely rewritten by the compiler.', + }).withDetails({ + kind: 'error', + message: + 'Missing dependency' + ` ${_printTemporary(inferredDependency)}`, + loc: inferredDependency.loc, + }); + for (const extra of morePreciseDependencies) { + diagnostic.withDetails({ + kind: 'hint', + message: `Found similar dependency \`${printManualMemoDependency(extra, false)}\``, + }); + } + error.pushDiagnostic(diagnostic); + } + } + + for (const dep of startMemo.deps ?? []) { + if ( + matched.has(dep) || + (dep.root.kind === 'NamedLocal' && + !reactive.has(dep.root.value.identifier.id)) + ) { + continue; + } + error.pushDiagnostic( + CompilerDiagnostic.create({ + category: ErrorCategory.PreserveManualMemo, + reason: 'Found unnecessary memoization dependency', + description: + 'Adding unnecessary memoization dependencies can cause a value to recompute ' + + 'more often than necessary and change behavior. This memoization cannot be safely rewritten by the compiler.', + }).withDetails({ + kind: 'error', + message: + 'Unnecessary dependency' + + ` ${printManualMemoDependency(dep, false)}`, + loc: startMemo.loc, + }), + ); + } + // TODO: validate that all inferred dependencies were in manual deps list too + dependencies.clear(); + locals.clear(); + startMemo = null; + } + + collectTemporaries(fn, temporaries, { + onStartMemoize, + onFinishMemoize, + }); + return error.asResult(); +} + +function visitCandidateDependency( + place: Place, + temporaries: Map, + dependencies: Set, +): void { + const dep = temporaries.get(place.identifier.id); + if (dep != null) { + if (dep.kind === 'Function') { + dep.dependencies.forEach(x => dependencies.add(x)); + } else { + dependencies.add(dep); + } + } +} + +function collectTemporaries( + fn: HIRFunction, + temporaries: Map, + callbacks: { + onStartMemoize: ( + startMemo: StartMemoize, + dependencies: Set, + locals: Set, + ) => void; + onFinishMemoize: ( + finishMemo: FinishMemoize, + dependencies: Set, + locals: Set, + ) => void; + } | null, +): Extract { + const optionals = findOptionalPlaces(fn); + if (DEBUG) { + console.log(prettyFormat(optionals)); + } + const locals: Set = new Set(); + const dependencies: Set = new Set(); + function visit(place: Place): void { + visitCandidateDependency(place, temporaries, dependencies); + } + for (const block of fn.body.blocks.values()) { + for (const phi of block.phis) { + let deps: Array | null = null; + for (const operand of phi.operands.values()) { + const dep = temporaries.get(operand.identifier.id); + if (dep == null) { + continue; + } + if (deps == null) { + deps = [dep]; + } else { + deps.push(dep); + } + } + if (deps == null) { + continue; + } else if (deps.length === 1) { + temporaries.set(phi.place.identifier.id, deps[0]!); + } else { + temporaries.set(phi.place.identifier.id, { + kind: 'Function', + dependencies: new Set(deps), + }); + } + } + + for (const instr of block.instructions) { + const {lvalue, value} = instr; + switch (value.kind) { + case 'LoadGlobal': { + temporaries.set(lvalue.identifier.id, { + kind: 'Global', + binding: value.binding, + }); + break; + } + 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') { + const local: Temporary = {...temp, loc: value.place.loc}; + temporaries.set(lvalue.identifier.id, local); + } else { + temporaries.set(lvalue.identifier.id, temp); + } + } + break; + } + case 'DeclareLocal': { + const local: Temporary = { + kind: 'Local', + identifier: value.lvalue.place.identifier, + path: [], + context: false, + loc: value.lvalue.place.loc, + }; + temporaries.set(value.lvalue.place.identifier.id, local); + locals.add(value.lvalue.place.identifier.id); + break; + } + case 'StoreLocal': { + if (value.lvalue.place.identifier.name == null) { + const temp = temporaries.get(value.value.identifier.id); + if (temp != null) { + temporaries.set(value.lvalue.place.identifier.id, temp); + } + break; + } + visit(value.value); + if (value.lvalue.kind !== InstructionKind.Reassign) { + const local: Temporary = { + kind: 'Local', + identifier: value.lvalue.place.identifier, + path: [], + context: false, + loc: value.lvalue.place.loc, + }; + temporaries.set(value.lvalue.place.identifier.id, local); + locals.add(value.lvalue.place.identifier.id); + } + break; + } + case 'DeclareContext': { + const local: Temporary = { + kind: 'Local', + identifier: value.lvalue.place.identifier, + path: [], + context: true, + loc: value.lvalue.place.loc, + }; + temporaries.set(value.lvalue.place.identifier.id, local); + break; + } + case 'StoreContext': { + visit(value.value); + if (value.lvalue.kind !== InstructionKind.Reassign) { + const local: Temporary = { + kind: 'Local', + identifier: value.lvalue.place.identifier, + path: [], + context: true, + loc: value.lvalue.place.loc, + }; + temporaries.set(value.lvalue.place.identifier.id, local); + locals.add(value.lvalue.place.identifier.id); + } + break; + } + case 'Destructure': { + visit(value.value); + if (value.lvalue.kind !== InstructionKind.Reassign) { + for (const lvalue of eachInstructionValueLValue(value)) { + const local: Temporary = { + kind: 'Local', + identifier: lvalue.identifier, + path: [], + context: false, + loc: lvalue.loc, + }; + temporaries.set(lvalue.identifier.id, local); + locals.add(lvalue.identifier.id); + } + } + break; + } + case 'PropertyLoad': { + if (typeof value.property === 'number') { + visit(value.object); + break; + } + const object = temporaries.get(value.object.identifier.id); + if (object != null && object.kind === 'Local') { + const optional = optionals.get(value.object.identifier.id) ?? false; + const local: Temporary = { + kind: 'Local', + identifier: object.identifier, + context: object.context, + path: [ + ...object.path, + { + optional, + property: value.property, + }, + ], + loc: value.loc, + }; + temporaries.set(lvalue.identifier.id, local); + } + break; + } + case 'FunctionExpression': + case 'ObjectMethod': { + const functionDeps = collectTemporaries( + value.loweredFunc.func, + temporaries, + null, + ); + temporaries.set(lvalue.identifier.id, functionDeps); + for (const dep of functionDeps.dependencies) { + dependencies.add(dep); + } + break; + } + case 'StartMemoize': { + const onStartMemoize = callbacks?.onStartMemoize; + if (onStartMemoize != null) { + onStartMemoize(value, dependencies, locals); + } + break; + } + case 'FinishMemoize': { + const onFinishMemoize = callbacks?.onFinishMemoize; + if (onFinishMemoize != null) { + onFinishMemoize(value, dependencies, locals); + } + break; + } + case 'MethodCall': { + // Ignore the method itself + for (const operand of eachInstructionValueOperand(value)) { + if (operand.identifier.id === value.property.identifier.id) { + continue; + } + visit(operand); + } + break; + } + default: { + for (const operand of eachInstructionValueOperand(value)) { + visit(operand); + } + for (const lvalue of eachInstructionLValue(instr)) { + locals.add(lvalue.identifier.id); + } + } + } + } + for (const operand of eachTerminalOperand(block.terminal)) { + if (optionals.has(operand.identifier.id)) { + continue; + } + visit(operand); + } + } + return {kind: 'Function', dependencies}; +} + +function _printTemporary(temporary: Temporary): string { + switch (temporary.kind) { + case 'Global': { + return `Global ${temporary.binding.name} [${temporary.binding.kind}]`; + } + case 'Local': { + return `Local${temporary.context ? ' (Context)' : ''} ${printIdentifier(temporary.identifier)}${temporary.path.map(p => (p.optional ? '?' : '') + '.' + p.property).join('')}`; + } + case 'Function': { + return `Function dependencies=[${Array.from(temporary.dependencies).map(_printTemporary).join(', ')}]`; + } + } +} + +function isEqualTemporary(a: Temporary, b: Temporary): boolean { + switch (a.kind) { + case 'Function': { + // TODO: ideally remove Function kind + return false; + } + case 'Global': { + return b.kind === 'Global' && a.binding.name === b.binding.name; + } + case 'Local': { + return ( + b.kind === 'Local' && + a.identifier.id === b.identifier.id && + areEqualPaths(a.path, b.path) + ); + } + } +} + +type Temporary = + | {kind: 'Global'; binding: LoadGlobal['binding']} + | { + kind: 'Local'; + identifier: Identifier; + path: DependencyPath; + context: boolean; + loc: SourceLocation; + } + | {kind: 'Function'; dependencies: Set}; + +function collectReactiveIdentifiersHIR(fn: HIRFunction): Set { + const reactive = new Set(); + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + for (const lvalue of eachInstructionLValue(instr)) { + if (lvalue.reactive) { + reactive.add(lvalue.identifier.id); + } + } + for (const operand of eachInstructionValueOperand(instr.value)) { + if (operand.reactive) { + reactive.add(operand.identifier.id); + } + } + } + for (const operand of eachTerminalOperand(block.terminal)) { + if (operand.reactive) { + reactive.add(operand.identifier.id); + } + } + } + return reactive; +} + +export function findOptionalPlaces( + fn: HIRFunction, +): Map { + const optionals = new Map(); + const visited: Set = new Set(); + for (const [, block] of fn.body.blocks) { + if (visited.has(block.id)) { + continue; + } + if (block.terminal.kind === 'optional') { + visited.add(block.id); + const optionalTerminal = block.terminal; + let testBlock = fn.body.blocks.get(block.terminal.test)!; + const queue: Array = [block.terminal.optional]; + loop: while (true) { + visited.add(testBlock.id); + const terminal = testBlock.terminal; + switch (terminal.kind) { + case 'branch': { + const isOptional = queue.pop(); + CompilerError.simpleInvariant(isOptional !== undefined, { + reason: + 'Expected an optional value for each optional test condition', + loc: terminal.test.loc, + }); + if (isOptional != null) { + optionals.set(terminal.test.identifier.id, isOptional); + } + if (terminal.fallthrough === optionalTerminal.fallthrough) { + // found it + const consequent = fn.body.blocks.get(terminal.consequent)!; + const last = consequent.instructions.at(-1); + if (last !== undefined && last.value.kind === 'StoreLocal') { + if (isOptional != null) { + optionals.set(last.value.value.identifier.id, isOptional); + } + } + break loop; + } else { + testBlock = fn.body.blocks.get(terminal.fallthrough)!; + } + break; + } + case 'optional': { + queue.push(terminal.optional); + testBlock = fn.body.blocks.get(terminal.test)!; + break; + } + case 'logical': + case 'ternary': { + queue.push(null); + testBlock = fn.body.blocks.get(terminal.test)!; + break; + } + + case 'sequence': { + // Do we need sequence?? In any case, don't push to queue bc there is no corresponding branch terminal + testBlock = fn.body.blocks.get(terminal.block)!; + break; + } + default: { + CompilerError.simpleInvariant(false, { + reason: `Unexpected terminal in optional`, + loc: terminal.loc, + }); + } + } + } + CompilerError.simpleInvariant(queue.length === 0, { + reason: + 'Expected a matching number of conditional blocks and branch points', + loc: block.terminal.loc, + }); + } + } + return optionals; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md new file mode 100644 index 0000000000..32a18f61ce --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md @@ -0,0 +1,84 @@ + +## Input + +```javascript +// @validateExhaustiveMemoizationDependencies +import {useMemo} from 'react'; +import {Stringify} from 'shared-runtime'; + +function Component({x, y, z}) { + const a = useMemo(() => { + return x?.y.z?.a; + }, [x?.y.z?.a.b]); + const b = useMemo(() => { + return x.y.z?.a; + }, [x.y.z.a]); + const c = useMemo(() => { + return x?.y.z.a?.b; + }, [x?.y.z.a?.b.z]); + const d = useMemo(() => { + return x?.y?.[(console.log(y), z?.b)]; + }, [x?.y, y, z?.b]); + const e = useMemo(() => { + const e = []; + e.push(x); + return e; + }, [x]); + return ; +} + +``` + + +## Error + +``` +Found 3 errors: + +Compilation Skipped: Found missing memoization dependency + +Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. This memoization cannot be safely rewritten by the compiler.. + +error.invalid-exhaustive-deps.ts:7:11 + 5 | function Component({x, y, z}) { + 6 | const a = useMemo(() => { +> 7 | return x?.y.z?.a; + | ^^^^^^^^^ Missing dependency Local x$181?.y.z?.a + 8 | }, [x?.y.z?.a.b]); + 9 | const b = useMemo(() => { + 10 | return x.y.z?.a; + +Found similar dependency `x$181?.y.z?.a.b` + +Compilation Skipped: Found missing memoization dependency + +Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. This memoization cannot be safely rewritten by the compiler.. + +error.invalid-exhaustive-deps.ts:10:11 + 8 | }, [x?.y.z?.a.b]); + 9 | const b = useMemo(() => { +> 10 | return x.y.z?.a; + | ^^^^^^^^ Missing dependency Local x$181.y.z?.a + 11 | }, [x.y.z.a]); + 12 | const c = useMemo(() => { + 13 | return x?.y.z.a?.b; + +Found similar dependency `x$181.y.z.a` + +Compilation Skipped: Found missing memoization dependency + +Missing dependencies can cause a value not to update when those inputs change, resulting in stale UI. This memoization cannot be safely rewritten by the compiler.. + +error.invalid-exhaustive-deps.ts:13:11 + 11 | }, [x.y.z.a]); + 12 | const c = useMemo(() => { +> 13 | return x?.y.z.a?.b; + | ^^^^^^^^^^^ Missing dependency Local x$181?.y.z.a?.b + 14 | }, [x?.y.z.a?.b.z]); + 15 | const d = useMemo(() => { + 16 | return x?.y?.[(console.log(y), z?.b)]; + +Found similar dependency `x$181?.y.z.a?.b.z` +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js new file mode 100644 index 0000000000..2227287620 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js @@ -0,0 +1,24 @@ +// @validateExhaustiveMemoizationDependencies +import {useMemo} from 'react'; +import {Stringify} from 'shared-runtime'; + +function Component({x, y, z}) { + const a = useMemo(() => { + return x?.y.z?.a; + }, [x?.y.z?.a.b]); + const b = useMemo(() => { + return x.y.z?.a; + }, [x.y.z.a]); + const c = useMemo(() => { + return x?.y.z.a?.b; + }, [x?.y.z.a?.b.z]); + const d = useMemo(() => { + return x?.y?.[(console.log(y), z?.b)]; + }, [x?.y, y, z?.b]); + const e = useMemo(() => { + const e = []; + e.push(x); + return e; + }, [x]); + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.expect.md new file mode 100644 index 0000000000..390e6a46be --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.expect.md @@ -0,0 +1,121 @@ + +## Input + +```javascript +// @validateExhaustiveMemoizationDependencies +import {useMemo} from 'react'; +import {Stringify} from 'shared-runtime'; + +function Component({x, y, z}) { + const a = useMemo(() => { + return x?.y.z?.a; + }, [x?.y.z?.a]); + const b = useMemo(() => { + return x.y.z?.a; + }, [x.y.z?.a]); + const c = useMemo(() => { + return x?.y.z.a?.b; + }, [x?.y.z.a?.b]); + const d = useMemo(() => { + return x?.y?.[(console.log(y), z?.b)]; + }, [x?.y, y, z?.b]); + const e = useMemo(() => { + const e = []; + e.push(x); + return e; + }, [x]); + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies +import { useMemo } from "react"; +import { Stringify } from "shared-runtime"; + +function Component(t0) { + const $ = _c(18); + const { x, y, z } = t0; + + x?.y.z?.a; + const a = x?.y.z?.a; + let t1; + if ($[0] !== x.y.z.a) { + t1 = () => x.y.z?.a; + $[0] = x.y.z.a; + $[1] = t1; + } else { + t1 = $[1]; + } + x.y.z?.a; + let t2; + if ($[2] !== t1) { + t2 = t1(); + $[2] = t1; + $[3] = t2; + } else { + t2 = $[3]; + } + const b = t2; + + x?.y.z.a?.b; + const c = x?.y.z.a?.b; + let t3; + if ($[4] !== x.y || $[5] !== y || $[6] !== z?.b) { + t3 = () => x?.y?.[(console.log(y), z?.b)]; + $[4] = x.y; + $[5] = y; + $[6] = z?.b; + $[7] = t3; + } else { + t3 = $[7]; + } + x?.y; + z?.b; + let t4; + if ($[8] !== t3) { + t4 = t3(); + $[8] = t3; + $[9] = t4; + } else { + t4 = $[9]; + } + const d = t4; + let e; + if ($[10] !== x) { + e = []; + e.push(x); + $[10] = x; + $[11] = e; + } else { + e = $[11]; + } + const e_0 = e; + let t5; + if ( + $[12] !== a || + $[13] !== b || + $[14] !== c || + $[15] !== d || + $[16] !== e_0 + ) { + t5 = ; + $[12] = a; + $[13] = b; + $[14] = c; + $[15] = d; + $[16] = e_0; + $[17] = t5; + } else { + t5 = $[17]; + } + return t5; +} + +``` + +### 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.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.js new file mode 100644 index 0000000000..3bd4526085 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.js @@ -0,0 +1,24 @@ +// @validateExhaustiveMemoizationDependencies +import {useMemo} from 'react'; +import {Stringify} from 'shared-runtime'; + +function Component({x, y, z}) { + const a = useMemo(() => { + return x?.y.z?.a; + }, [x?.y.z?.a]); + const b = useMemo(() => { + return x.y.z?.a; + }, [x.y.z?.a]); + const c = useMemo(() => { + return x?.y.z.a?.b; + }, [x?.y.z.a?.b]); + const d = useMemo(() => { + return x?.y?.[(console.log(y), z?.b)]; + }, [x?.y, y, z?.b]); + const e = useMemo(() => { + const e = []; + e.push(x); + return e; + }, [x]); + return ; +}