Skip to content

Commit a1cd347

Browse files
author
Joe Savona
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 0fba073 commit a1cd347

File tree

6 files changed

+815
-0
lines changed

6 files changed

+815
-0
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoF
103103
import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEffects';
104104
import {inferMutationAliasingRanges} from '../Inference/InferMutationAliasingRanges';
105105
import {validateNoDerivedComputationsInEffects} from '../Validation/ValidateNoDerivedComputationsInEffects';
106+
import {validateExhaustiveDependencies} from '../Validation/ValidateExhaustiveDependencies';
106107

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

296+
if (env.config.validateExhaustiveMemoizationDependencies) {
297+
// NOTE: this relies on reactivity inference running first
298+
validateExhaustiveDependencies(hir).unwrap();
299+
}
300+
295301
rewriteInstructionKindsBasedOnReassignment(hir);
296302
log({
297303
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: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1633,6 +1633,19 @@ export function areEqualPaths(a: DependencyPath, b: DependencyPath): boolean {
16331633
)
16341634
);
16351635
}
1636+
export function isSubPath(
1637+
subpath: DependencyPath,
1638+
path: DependencyPath,
1639+
): boolean {
1640+
return (
1641+
subpath.length <= path.length &&
1642+
subpath.every(
1643+
(item, ix) =>
1644+
item.property === path[ix].property &&
1645+
item.optional === path[ix].optional,
1646+
)
1647+
);
1648+
}
16361649

16371650
export function getPlaceScope(
16381651
id: InstructionId,

0 commit comments

Comments
 (0)