[compiler] Implement exhaustive dependency checking for manual memoization #34394

josephsavona · 2025-09-04T19:14:42Z

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).

Stack created with Sapling. Best reviewed with ReviewStack.

…34406) Adds missing locations to all the statement kinds that we produce in codegenInstruction(), and adds generic handling of source locations for the nodes produced by codegenInstructionValue(). There are definitely some places where we are still missing a location, but this should address some of the known issues we've seen such as missing location on `throw`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34406). * #34394 * __->__ #34406 * #34346

…34406) Adds missing locations to all the statement kinds that we produce in codegenInstruction(), and adds generic handling of source locations for the nodes produced by codegenInstructionValue(). There are definitely some places where we are still missing a location, but this should address some of the known issues we've seen such as missing location on `throw`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34406). * #34394 * __->__ #34406 * #34346 DiffTrain build for [f5e96b9](f5e96b9)

…acebook#34406) Adds missing locations to all the statement kinds that we produce in codegenInstruction(), and adds generic handling of source locations for the nodes produced by codegenInstructionValue(). There are definitely some places where we are still missing a location, but this should address some of the known issues we've seen such as missing location on `throw`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34406). * facebook#34394 * __->__ facebook#34406 * facebook#34346 DiffTrain build for [f5e96b9](facebook@f5e96b9)

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts

josephsavona · 2025-09-10T20:45:46Z

...lugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-exhaustive-deps.expect.md

+> 24 |     return [];
+     | ^^^^^^^^^^^^^^
+> 25 |   }, [x, y.z, z?.y?.a]);
+     | ^^^^ Unnecessary dependencies `x`, `y.z`, `z?.y?.a`


In follow-ups i want to align the way we calculate the manual deps in DropManualMemo and the inferred deps in this pass, including adding locations to all the deps. Then we could point to the specific locations here.

Further, we should track a location for the manual deps array literal, so that we can emit a code suggestion to replace w the correct deps.

Another todo list item: add a separate feature flag to control modes:

the compiler just needs to know that the deps are "correct". Having eg x and x.y as deps should be fine.

for linting, we probably want to have the most precise deps and no unnecessary extras. We'd want to error on deps [x, x.y] and suggest replacing with [x].

mofeiZ · 2025-09-15T22:27:56Z

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts

+      loc: value.loc,
+    });
+    startMemo = value;
+    dependencies.clear();


super nit: should this be an assertion instead since onFinishMemoize already clears dependencies and locals?

No, deps are always aggregated and have to be cleared before entering a scope. We don't technically need to clear them when exiting a scope in onFinishMemoize but it doesn't hurt either.

josephsavona · 2025-09-15T23:20:06Z

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts

+}
+
+/**
+ * This function determines the dependencies of the given function relative to


@mofeiZ more about the approach here

josephsavona · 2025-09-15T23:20:45Z

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts

+ * 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.


i'll add a note but this is necessary because of property chains and optionals

…ation 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).

…ation (#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). * #34472 * #34471 * __->__ #34394 DiffTrain build for [bcc3fd8](bcc3fd8)

josephsavona mentioned this pull request Sep 4, 2025

[compiler][wip] Allow suppressions within effects #34347

Closed

meta-cla bot added the CLA Signed label Sep 4, 2025

github-actions bot added the React Core Team Opened by a member of the React Core Team label Sep 4, 2025

josephsavona force-pushed the pr34394 branch 8 times, most recently from a1cd347 to f1c7583 Compare September 6, 2025 02:38

josephsavona mentioned this pull request Sep 6, 2025

[compiler] Add missing source locations to statements, expressions #34406

Merged

josephsavona force-pushed the pr34394 branch from f1c7583 to 40ba057 Compare September 10, 2025 03:11

josephsavona commented Sep 10, 2025

View reviewed changes

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateExhaustiveDependencies.ts Outdated Show resolved Hide resolved

josephsavona marked this pull request as ready for review September 10, 2025 15:11

josephsavona force-pushed the pr34394 branch from 40ba057 to bef52de Compare September 10, 2025 20:37

josephsavona commented Sep 10, 2025

View reviewed changes

josephsavona force-pushed the pr34394 branch from bef52de to 96014db Compare September 11, 2025 19:19

This was referenced Sep 11, 2025

[compiler] Auto-fix for non-exhaustive deps #34471

Merged

[compiler] ValidateExhaustiveDeps disallows unnecessary non-reactive deps #34472

Merged

mofeiZ reviewed Sep 15, 2025

View reviewed changes

josephsavona commented Sep 15, 2025

View reviewed changes

This was referenced Sep 16, 2025

[Compiler Bug]: The “one or more React ESLint rules were disabled” check is too aggressive #34261

Open

[compiler] enablePreserveMemo treats manual deps as non-nullable #34503

Merged

josephsavona force-pushed the pr34394 branch from 96014db to 611f607 Compare September 19, 2025 23:07

josephsavona force-pushed the pr34394 branch 2 times, most recently from 93696d8 to 5809d69 Compare October 2, 2025 16:19

josephsavona force-pushed the pr34394 branch from 5809d69 to 8409003 Compare November 20, 2025 23:06

josephsavona force-pushed the pr34394 branch from 8409003 to ca0a3c4 Compare November 21, 2025 03:17

josephsavona merged commit bcc3fd8 into main Nov 21, 2025
18 checks passed

This was referenced Nov 21, 2025

Upgrade React from 8ac5f4eb-20251119 to 40b4a5bf-20251120 vercel/next.js#86388

Closed

Upgrade React from 8ac5f4eb-20251119 to fd524fe0-20251121 vercel/next.js#86473

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[compiler] Implement exhaustive dependency checking for manual memoization #34394

[compiler] Implement exhaustive dependency checking for manual memoization #34394

Uh oh!

josephsavona commented Sep 4, 2025 •

edited

Loading

Uh oh!

Uh oh!

josephsavona Sep 10, 2025

Uh oh!

josephsavona Sep 10, 2025

Uh oh!

mofeiZ Sep 15, 2025 •

edited

Loading

Uh oh!

josephsavona Sep 15, 2025

Uh oh!

josephsavona Sep 15, 2025

Uh oh!

josephsavona Sep 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[compiler] Implement exhaustive dependency checking for manual memoization #34394

[compiler] Implement exhaustive dependency checking for manual memoization #34394

Uh oh!

Conversation

josephsavona commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

josephsavona Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

josephsavona Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

mofeiZ Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josephsavona Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

josephsavona Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

josephsavona Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

josephsavona commented Sep 4, 2025 •

edited

Loading

mofeiZ Sep 15, 2025 •

edited

Loading