Skip to content

Commit bcc3fd8

Browse files
authored
[compiler] Implement exhaustive dependency checking for manual memoization (facebook#34394)
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). --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34394). * facebook#34472 * facebook#34471 * __->__ facebook#34394
1 parent 50e7ec8 commit bcc3fd8

File tree

9 files changed

+1144
-0
lines changed

9 files changed

+1144
-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
@@ -304,6 +304,30 @@ export class CompilerError extends Error {
304304
disabledDetails: Array<CompilerErrorDetail | CompilerDiagnostic> = [];
305305
printedMessage: string | null = null;
306306

307+
static simpleInvariant(
308+
condition: unknown,
309+
options: {
310+
reason: CompilerDiagnosticOptions['reason'];
311+
description?: CompilerDiagnosticOptions['description'];
312+
loc: SourceLocation;
313+
},
314+
): asserts condition {
315+
if (!condition) {
316+
const errors = new CompilerError();
317+
errors.pushDiagnostic(
318+
CompilerDiagnostic.create({
319+
reason: options.reason,
320+
description: options.description ?? null,
321+
category: ErrorCategory.Invariant,
322+
}).withDetails({
323+
kind: 'error',
324+
loc: options.loc,
325+
message: options.reason,
326+
}),
327+
);
328+
throw errors;
329+
}
330+
}
307331
static invariant(
308332
condition: unknown,
309333
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
@@ -105,6 +105,7 @@ import {validateNoDerivedComputationsInEffects} from '../Validation/ValidateNoDe
105105
import {validateNoDerivedComputationsInEffects_exp} from '../Validation/ValidateNoDerivedComputationsInEffects_exp';
106106
import {nameAnonymousFunctions} from '../Transform/NameAnonymousFunctions';
107107
import {optimizeForSSR} from '../Optimization/OptimizeForSSR';
108+
import {validateExhaustiveDependencies} from '../Validation/ValidateExhaustiveDependencies';
108109
import {validateSourceLocations} from '../Validation/ValidateSourceLocations';
109110

110111
export type CompilerPipelineValue =
@@ -302,6 +303,11 @@ function runWithEnvironment(
302303
inferReactivePlaces(hir);
303304
log({kind: 'hir', name: 'InferReactivePlaces', value: hir});
304305

306+
if (env.config.validateExhaustiveMemoizationDependencies) {
307+
// NOTE: this relies on reactivity inference running first
308+
validateExhaustiveDependencies(hir).unwrap();
309+
}
310+
305311
rewriteInstructionKindsBasedOnReassignment(hir);
306312
log({
307313
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
@@ -218,6 +218,11 @@ export const EnvironmentConfigSchema = z.object({
218218
*/
219219
validatePreserveExistingMemoizationGuarantees: z.boolean().default(true),
220220

221+
/**
222+
* Validate that dependencies supplied to manual memoization calls are exhaustive.
223+
*/
224+
validateExhaustiveMemoizationDependencies: z.boolean().default(false),
225+
221226
/**
222227
* When this is true, rather than pruning existing manual memoization but ensuring or validating
223228
* 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)