diff --git a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts index 18bcd7791d0..ed64936610f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts @@ -304,6 +304,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 c01aceb6e8c..db5660cee13 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -106,6 +106,7 @@ import {validateNoDerivedComputationsInEffects} from '../Validation/ValidateNoDe import {validateNoDerivedComputationsInEffects_exp} from '../Validation/ValidateNoDerivedComputationsInEffects_exp'; import {nameAnonymousFunctions} from '../Transform/NameAnonymousFunctions'; import {optimizeForSSR} from '../Optimization/OptimizeForSSR'; +import {validateExhaustiveDependencies} from '../Validation/ValidateExhaustiveDependencies'; import {validateSourceLocations} from '../Validation/ValidateSourceLocations'; export type CompilerPipelineValue = @@ -303,6 +304,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 0e3654dccaf..f1387efb2c8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -217,6 +217,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 87ca692a95f..5440035183d 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 00000000000..ad9b83e3927 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts @@ -0,0 +1,749 @@ +/** + * 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, + LoadGlobal, + ManualMemoDependency, + Place, + StartMemoize, +} from '../HIR'; +import { + eachInstructionLValue, + eachInstructionValueLValue, + eachInstructionValueOperand, + eachTerminalOperand, +} from '../HIR/visitors'; +import {Result} from '../Utils/Result'; +import {retainWhere} from '../Utils/utils'; + +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, 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; + } + 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 = 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); + } + } + 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) || + isSubPath(manualDependency.path, inferredDependency.path)) + ) { + hasMatchingManualDependency = true; + matched.add(manualDependency); + } + } + if (!hasMatchingManualDependency) { + missing.push(inferredDependency); + } + } + + for (const dep of startMemo.deps ?? []) { + if ( + matched.has(dep) || + (dep.root.kind === 'NamedLocal' && + !reactive.has(dep.root.value.identifier.id)) + ) { + continue; + } + extra.push(dep); + } + + if (missing.length !== 0) { + // Error + const diagnostic = CompilerDiagnostic.create({ + category: ErrorCategory.PreserveManualMemo, + reason: 'Found non-exhaustive dependencies', + 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.', + }); + for (const dep of missing) { + diagnostic.withDetails({ + kind: 'error', + message: `Missing dependency \`${printInferredDependency(dep)}\``, + loc: dep.loc, + }); + } + error.pushDiagnostic(diagnostic); + } else if (extra.length !== 0) { + const diagnostic = CompilerDiagnostic.create({ + category: ErrorCategory.PreserveManualMemo, + reason: 'Found unnecessary memoization dependencies', + description: + 'Unnecessary dependencies can cause a value to update more often than necessary, ' + + 'which can cause effects to run more than expected. This memoization cannot be safely ' + + 'rewritten by the compiler', + }); + diagnostic.withDetails({ + kind: 'error', + message: `Unnecessary dependencies ${extra.map(dep => `\`${printManualMemoDependency(dep)}\``).join(', ')}`, + loc: value.loc, + }); + error.pushDiagnostic(diagnostic); + } + + dependencies.clear(); + locals.clear(); + startMemo = null; + } + + collectDependencies(fn, temporaries, { + onStartMemoize, + onFinishMemoize, + }); + return error.asResult(); +} + +function addDependency( + dep: Temporary, + dependencies: Set, + locals: Set, +): void { + if (dep.kind === 'Function') { + for (const x of dep.dependencies) { + addDependency(x, dependencies, locals); + } + } else if (dep.kind === 'Global') { + dependencies.add(dep); + } else if (!locals.has(dep.identifier.id)) { + dependencies.add(dep); + } +} + +function visitCandidateDependency( + place: Place, + temporaries: Map, + dependencies: Set, + locals: Set, +): void { + const dep = temporaries.get(place.identifier.id); + if (dep != null) { + addDependency(dep, dependencies, locals); + } +} + +/** + * This function determines the dependencies of the given function relative to + * its external context. Dependencies are collected eagerly, the first time an + * external variable is referenced, as opposed to trying to delay or aggregate + * calculation of dependencies until they are later "used". + * + * For example, in + * + * ``` + * function f() { + * let x = y; // we record a dependency on `y` here + * ... + * use(x); // as opposed to trying to delay that dependency until here + * } + * ``` + * + * That said, LoadLocal/LoadContext does not immediately take a dependency, + * we store the dependency in a temporary and set it as used when that temporary + * is referenced as an operand. + * + * As we proceed through the function we track local variables that it creates + * and don't consider later references to these variables as dependencies. + * + * For function expressions we first collect the function's dependencies by + * calling this function recursively, _without_ taking into account whether + * the "external" variables it accesses are actually external or just locals + * in the parent. We then prune any locals and immediately consider any + * remaining externals that it accesses as a dependency: + * + * ``` + * function Component() { + * const local = ...; + * const f = () => { return [external, local] }; + * } + * ``` + * + * Here we calculate `f` as having dependencies `external, `local` and save + * this into `temporaries`. We then also immediately take these as dependencies + * at the Component scope, at which point we filter out `local` as a local variable, + * leaving just a dependency on `external`. + * + * When calling this function on a top-level component or hook, the collected dependencies + * will only contain the globals that it accesses which isn't useful. Instead, passing + * onStartMemoize/onFinishMemoize callbacks allows looking at the dependencies within + * blocks of manual memoization. + */ +function collectDependencies( + 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, locals); + } + 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 = collectDependencies( + value.loweredFunc.func, + temporaries, + null, + ); + temporaries.set(lvalue.identifier.id, functionDeps); + addDependency(functionDeps, dependencies, locals); + 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 printInferredDependency(dep: InferredDependency): string { + switch (dep.kind) { + case 'Global': { + return dep.binding.name; + } + case 'Local': { + CompilerError.simpleInvariant( + dep.identifier.name != null && dep.identifier.name.kind === 'named', + { + reason: 'Expected dependencies to be named variables', + loc: dep.loc, + }, + ); + return `${dep.identifier.name.value}${dep.path.map(p => (p.optional ? '?' : '') + '.' + p.property).join('')}`; + } + } +} + +function printManualMemoDependency(dep: ManualMemoDependency): string { + let identifierName: string; + if (dep.root.kind === 'Global') { + identifierName = dep.root.identifierName; + } else { + const name = dep.root.value.identifier.name; + CompilerError.simpleInvariant(name != null && name.kind === 'named', { + reason: 'Expected manual dependencies to be named variables', + loc: dep.root.value.loc, + }); + identifierName = name.value; + } + return `${identifierName}${dep.path.map(p => (p.optional ? '?' : '') + '.' + p.property).join('')}`; +} + +function isEqualTemporary(a: Temporary, b: Temporary): boolean { + switch (a.kind) { + case 'Function': { + 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}; +type InferredDependency = Extract; + +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 00000000000..3af5cc2d6b4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md @@ -0,0 +1,98 @@ + +## 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]); + const f = useMemo(() => { + return []; + }, [x, y.z, z?.y?.a]); + return ; +} + +``` + + +## Error + +``` +Found 4 errors: + +Compilation Skipped: Found non-exhaustive dependencies + +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 `x?.y.z?.a` + 8 | }, [x?.y.z?.a.b]); + 9 | const b = useMemo(() => { + 10 | return x.y.z?.a; + +Compilation Skipped: Found non-exhaustive dependencies + +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 `x.y.z?.a` + 11 | }, [x.y.z.a]); + 12 | const c = useMemo(() => { + 13 | return x?.y.z.a?.b; + +Compilation Skipped: Found non-exhaustive dependencies + +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 `x?.y.z.a?.b` + 14 | }, [x?.y.z.a?.b.z]); + 15 | const d = useMemo(() => { + 16 | return x?.y?.[(console.log(y), z?.b)]; + +Compilation Skipped: Found unnecessary memoization dependencies + +Unnecessary dependencies can cause a value to update more often than necessary, which can cause effects to run more than expected. This memoization cannot be safely rewritten by the compiler. + +error.invalid-exhaustive-deps.ts:23:20 + 21 | return e; + 22 | }, [x]); +> 23 | const f = useMemo(() => { + | ^^^^^^^ +> 24 | return []; + | ^^^^^^^^^^^^^^ +> 25 | }, [x, y.z, z?.y?.a]); + | ^^^^ Unnecessary dependencies `x`, `y.z`, `z?.y?.a` + 26 | return ; + 27 | } + 28 | +``` + + \ 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 00000000000..7a16a210a43 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.js @@ -0,0 +1,27 @@ +// @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]); + const f = useMemo(() => { + return []; + }, [x, y.z, z?.y?.a]); + 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 00000000000..655d6bff415 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.expect.md @@ -0,0 +1,159 @@ + +## Input + +```javascript +// @validateExhaustiveMemoizationDependencies +import {useMemo} from 'react'; +import {makeObject_Primitives, Stringify} from 'shared-runtime'; + +function useHook1(x) { + return useMemo(() => { + return x?.y.z?.a; + }, [x?.y.z?.a]); +} +function useHook2(x) { + useMemo(() => { + return x.y.z?.a; + }, [x.y.z?.a]); +} +function useHook3(x) { + return useMemo(() => { + return x?.y.z.a?.b; + }, [x?.y.z.a?.b]); +} +function useHook4(x, y, z) { + return useMemo(() => { + return x?.y?.[(console.log(y), z?.b)]; + }, [x?.y, y, z?.b]); +} +function useHook5(x) { + return useMemo(() => { + const e = []; + const local = makeObject_Primitives(x); + const fn = () => { + e.push(local); + }; + fn(); + return e; + }, [x]); +} +function useHook6(x) { + return useMemo(() => { + const f = []; + f.push(x.y.z); + f.push(x.y); + f.push(x); + return f; + }, [x]); +} + +function Component({x, y, z}) { + const a = useHook1(x); + const b = useHook2(x); + const c = useHook3(x); + const d = useHook4(x, y, z); + const e = useHook5(x); + const f = useHook6(x); + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateExhaustiveMemoizationDependencies +import { useMemo } from "react"; +import { makeObject_Primitives, Stringify } from "shared-runtime"; + +function useHook1(x) { + x?.y.z?.a; + return x?.y.z?.a; +} + +function useHook2(x) { + x.y.z?.a; +} + +function useHook3(x) { + x?.y.z.a?.b; + return x?.y.z.a?.b; +} + +function useHook4(x, y, z) { + x?.y; + z?.b; + return x?.y?.[(console.log(y), z?.b)]; +} + +function useHook5(x) { + const $ = _c(2); + let e; + if ($[0] !== x) { + e = []; + const local = makeObject_Primitives(x); + const fn = () => { + e.push(local); + }; + + fn(); + $[0] = x; + $[1] = e; + } else { + e = $[1]; + } + return e; +} + +function useHook6(x) { + const $ = _c(2); + let f; + if ($[0] !== x) { + f = []; + f.push(x.y.z); + f.push(x.y); + f.push(x); + $[0] = x; + $[1] = f; + } else { + f = $[1]; + } + return f; +} + +function Component(t0) { + const $ = _c(7); + const { x, y, z } = t0; + const a = useHook1(x); + const b = useHook2(x); + const c = useHook3(x); + const d = useHook4(x, y, z); + const e = useHook5(x); + const f = useHook6(x); + let t1; + if ( + $[0] !== a || + $[1] !== b || + $[2] !== c || + $[3] !== d || + $[4] !== e || + $[5] !== f + ) { + t1 = ; + $[0] = a; + $[1] = b; + $[2] = c; + $[3] = d; + $[4] = e; + $[5] = f; + $[6] = t1; + } else { + t1 = $[6]; + } + return t1; +} + +``` + +### 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 00000000000..d4c833c7d29 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/exhaustive-deps.js @@ -0,0 +1,54 @@ +// @validateExhaustiveMemoizationDependencies +import {useMemo} from 'react'; +import {makeObject_Primitives, Stringify} from 'shared-runtime'; + +function useHook1(x) { + return useMemo(() => { + return x?.y.z?.a; + }, [x?.y.z?.a]); +} +function useHook2(x) { + useMemo(() => { + return x.y.z?.a; + }, [x.y.z?.a]); +} +function useHook3(x) { + return useMemo(() => { + return x?.y.z.a?.b; + }, [x?.y.z.a?.b]); +} +function useHook4(x, y, z) { + return useMemo(() => { + return x?.y?.[(console.log(y), z?.b)]; + }, [x?.y, y, z?.b]); +} +function useHook5(x) { + return useMemo(() => { + const e = []; + const local = makeObject_Primitives(x); + const fn = () => { + e.push(local); + }; + fn(); + return e; + }, [x]); +} +function useHook6(x) { + return useMemo(() => { + const f = []; + f.push(x.y.z); + f.push(x.y); + f.push(x); + return f; + }, [x]); +} + +function Component({x, y, z}) { + const a = useHook1(x); + const b = useHook2(x); + const c = useHook3(x); + const d = useHook4(x, y, z); + const e = useHook5(x); + const f = useHook6(x); + return ; +}