Skip to content

Commit 96014db

Browse files
committed
[compiler] Implement exhaustive dependency checking for manual memoization
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).
1 parent acada30 commit 96014db

File tree

9 files changed

+1117
-0
lines changed

9 files changed

+1117
-0
lines changed

compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,30 @@ export class CompilerError extends Error {
282282
disabledDetails: Array<CompilerErrorDetail | CompilerDiagnostic> = [];
283283
printedMessage: string | null = null;
284284

285+
static simpleInvariant(
286+
condition: unknown,
287+
options: {
288+
reason: CompilerDiagnosticOptions['reason'];
289+
description?: CompilerDiagnosticOptions['description'];
290+
loc: SourceLocation;
291+
},
292+
): asserts condition {
293+
if (!condition) {
294+
const errors = new CompilerError();
295+
errors.pushDiagnostic(
296+
CompilerDiagnostic.create({
297+
reason: options.reason,
298+
description: options.description ?? null,
299+
category: ErrorCategory.Invariant,
300+
}).withDetails({
301+
kind: 'error',
302+
loc: options.loc,
303+
message: options.reason,
304+
}),
305+
);
306+
throw errors;
307+
}
308+
}
285309
static invariant(
286310
condition: unknown,
287311
options: Omit<CompilerDiagnosticOptions, 'category'>,

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEf
104104
import {inferMutationAliasingRanges} from '../Inference/InferMutationAliasingRanges';
105105
import {validateNoDerivedComputationsInEffects} from '../Validation/ValidateNoDerivedComputationsInEffects';
106106
import {nameAnonymousFunctions} from '../Transform/NameAnonymousFunctions';
107+
import {validateExhaustiveDependencies} from '../Validation/ValidateExhaustiveDependencies';
107108

108109
export type CompilerPipelineValue =
109110
| {kind: 'ast'; name: string; value: CodegenFunction}
@@ -293,6 +294,11 @@ function runWithEnvironment(
293294
inferReactivePlaces(hir);
294295
log({kind: 'hir', name: 'InferReactivePlaces', value: hir});
295296

297+
if (env.config.validateExhaustiveMemoizationDependencies) {
298+
// NOTE: this relies on reactivity inference running first
299+
validateExhaustiveDependencies(hir).unwrap();
300+
}
301+
296302
rewriteInstructionKindsBasedOnReassignment(hir);
297303
log({
298304
kind: 'hir',

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,11 @@ export const EnvironmentConfigSchema = z.object({
227227
*/
228228
validatePreserveExistingMemoizationGuarantees: z.boolean().default(true),
229229

230+
/**
231+
* Validate that dependencies supplied to manual memoization calls are exhaustive.
232+
*/
233+
validateExhaustiveMemoizationDependencies: z.boolean().default(false),
234+
230235
/**
231236
* When this is true, rather than pruning existing manual memoization but ensuring or validating
232237
* that the memoized values remain memoized, the compiler will simply not prune existing calls to

compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,6 +1680,28 @@ export function areEqualPaths(a: DependencyPath, b: DependencyPath): boolean {
16801680
)
16811681
);
16821682
}
1683+
export function isSubPath(
1684+
subpath: DependencyPath,
1685+
path: DependencyPath,
1686+
): boolean {
1687+
return (
1688+
subpath.length <= path.length &&
1689+
subpath.every(
1690+
(item, ix) =>
1691+
item.property === path[ix].property &&
1692+
item.optional === path[ix].optional,
1693+
)
1694+
);
1695+
}
1696+
export function isSubPathIgnoringOptionals(
1697+
subpath: DependencyPath,
1698+
path: DependencyPath,
1699+
): boolean {
1700+
return (
1701+
subpath.length <= path.length &&
1702+
subpath.every((item, ix) => item.property === path[ix].property)
1703+
);
1704+
}
16831705

16841706
export function getPlaceScope(
16851707
id: InstructionId,

0 commit comments

Comments
 (0)