[compiler] Enable validateExhaustiveMemoizationDependencies by default #35201
+224
−184
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Nov 24, 2025
github-actions
bot
added
the
React Core Team
josephsavona
force-pushed
the
pr35201
branch
from
c412ad5 to
70f5aa1
Compare
josephsavona
added a commit
that referenced
this pull request
Nov 24, 2025
…35184) With `ValidateExhaustiveMemoDependencies` we can now check exhaustive dependencies for useMemo and useCallback within the compiler, without relying on the separate exhaustive-deps rule. Until now we've bailed out of any component/hook that suppresses this rule, since the suppression _might_ affect a memoization value. Compiling code with incorrect memo deps can change behavior so this wasn't safe. The downside was that a suppression within a useEffect could prevent memoization, even though non-exhaustive deps for effects do not cause problems for memoization specifically. So here, we change to ignore ESLint suppressions if we have both the compiler's hooks validation and memo deps validations enabled. Now we just have to test out the new validation and refine before we can enable this by default. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35184). * #35201 * #35202 * #35192 * #35190 * #35186 * #35185 * __->__ #35184
josephsavona
force-pushed
the
pr35201
branch
from
70f5aa1 to
9822142
Compare
josephsavona
added a commit
that referenced
this pull request
Nov 24, 2025
When checking ValidateExhaustiveDeps internally, this seems to be the most common case that it flags. The current exhaustive-deps rule allows extraneous deps if they are a set of stable types. So here we reuse our existing isStableType() util in the compiler to allow this case. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35185). * #35201 * #35202 * #35192 * #35190 * #35186 * __->__ #35185
josephsavona
force-pushed
the
pr35201
branch
from
9822142 to
afcc183
Compare
josephsavona
added a commit
that referenced
this pull request
Nov 24, 2025
…an inferred deps (#35186) Since adding this validation we've already changed our inference to use knowledge from manual memoization to inform when values are frozen and which values are non-nullable. To align with that, if the user chooses to use different optionality btw the deps and the memo block/callback, that's fine. The key is that eg `x?.y` will invalidate whenever `x.y` would, so from a memoization correctness perspective its fine. It's not our job to be a type checker: if a value is potentially nullable, it should likely use a nullable property access in both places but TypeScript/Flow can check that. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35186). * #35201 * #35202 * #35192 * #35190 * __->__ #35186
josephsavona
force-pushed
the
pr35201
branch
from
afcc183 to
34ccbc1
Compare
josephsavona
added a commit
that referenced
this pull request
Nov 24, 2025
The existing exhaustive-deps rule allows omitting non-reactive dependencies, even if they're not memoized. Conceptually, if a value is non-reactive then it cannot semantically change. Even if the value is a new object, that object represents the exact same value and doesn't necessitate redoing downstream computation. Thus its fine to exclude nonreactive dependencies, whether they're a stable type or not. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35190). * #35201 * #35202 * #35192 * __->__ #35190
josephsavona
force-pushed
the
pr35201
branch
from
34ccbc1 to
0c69c04
Compare
josephsavona
added a commit
that referenced
this pull request
Nov 24, 2025
…rule (#35192) Similar to ValidateHookUsage, we implement this check in the compiler for safety but (for now) continue to rely on the existing rule for actually reporting errors to users. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35192). * #35201 * #35202 * __->__ #35192
josephsavona
force-pushed
the
pr35201
branch
from
0c69c04 to
f42791b
Compare
josephsavona
added a commit
that referenced
this pull request
Nov 24, 2025
…5202) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35202). * #35201 * __->__ #35202
github-actions bot
pushed a commit
that referenced
this pull request
Nov 24, 2025
When checking ValidateExhaustiveDeps internally, this seems to be the most common case that it flags. The current exhaustive-deps rule allows extraneous deps if they are a set of stable types. So here we reuse our existing isStableType() util in the compiler to allow this case. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35185). * #35201 * #35202 * #35192 * #35190 * #35186 * __->__ #35185 DiffTrain build for [c9a8cf3](c9a8cf3)
josephsavona
force-pushed
the
pr35201
branch
from
f42791b to
cdee3c8
Compare
github-actions bot
pushed a commit
that referenced
this pull request
Nov 24, 2025
When checking ValidateExhaustiveDeps internally, this seems to be the most common case that it flags. The current exhaustive-deps rule allows extraneous deps if they are a set of stable types. So here we reuse our existing isStableType() util in the compiler to allow this case. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35185). * #35201 * #35202 * #35192 * #35190 * #35186 * __->__ #35185 DiffTrain build for [c9a8cf3](c9a8cf3)
github-actions bot
pushed a commit
that referenced
this pull request
Nov 24, 2025
…an inferred deps (#35186) Since adding this validation we've already changed our inference to use knowledge from manual memoization to inform when values are frozen and which values are non-nullable. To align with that, if the user chooses to use different optionality btw the deps and the memo block/callback, that's fine. The key is that eg `x?.y` will invalidate whenever `x.y` would, so from a memoization correctness perspective its fine. It's not our job to be a type checker: if a value is potentially nullable, it should likely use a nullable property access in both places but TypeScript/Flow can check that. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35186). * #35201 * #35202 * #35192 * #35190 * __->__ #35186 DiffTrain build for [454e01e](454e01e)
github-actions bot
pushed a commit
that referenced
this pull request
Nov 24, 2025
The existing exhaustive-deps rule allows omitting non-reactive dependencies, even if they're not memoized. Conceptually, if a value is non-reactive then it cannot semantically change. Even if the value is a new object, that object represents the exact same value and doesn't necessitate redoing downstream computation. Thus its fine to exclude nonreactive dependencies, whether they're a stable type or not. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35190). * #35201 * #35202 * #35192 * __->__ #35190 DiffTrain build for [67c1487](67c1487)
github-actions bot
pushed a commit
that referenced
this pull request
Nov 24, 2025
The existing exhaustive-deps rule allows omitting non-reactive dependencies, even if they're not memoized. Conceptually, if a value is non-reactive then it cannot semantically change. Even if the value is a new object, that object represents the exact same value and doesn't necessitate redoing downstream computation. Thus its fine to exclude nonreactive dependencies, whether they're a stable type or not. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35190). * #35201 * #35202 * #35192 * __->__ #35190 DiffTrain build for [67c1487](67c1487)
github-actions bot
pushed a commit
that referenced
this pull request
Nov 24, 2025
…rule (#35192) Similar to ValidateHookUsage, we implement this check in the compiler for safety but (for now) continue to rely on the existing rule for actually reporting errors to users. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35192). * #35201 * #35202 * __->__ #35192 DiffTrain build for [9599e7a](9599e7a)
github-actions bot
pushed a commit
that referenced
this pull request
Nov 24, 2025
…5202) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35202). * #35201 * __->__ #35202 DiffTrain build for [16e16ec](16e16ec)
github-actions bot
pushed a commit
that referenced
this pull request
Nov 24, 2025
…35184) With `ValidateExhaustiveMemoDependencies` we can now check exhaustive dependencies for useMemo and useCallback within the compiler, without relying on the separate exhaustive-deps rule. Until now we've bailed out of any component/hook that suppresses this rule, since the suppression _might_ affect a memoization value. Compiling code with incorrect memo deps can change behavior so this wasn't safe. The downside was that a suppression within a useEffect could prevent memoization, even though non-exhaustive deps for effects do not cause problems for memoization specifically. So here, we change to ignore ESLint suppressions if we have both the compiler's hooks validation and memo deps validations enabled. Now we just have to test out the new validation and refine before we can enable this by default. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35184). * #35201 * #35202 * #35192 * #35190 * #35186 * #35185 * __->__ #35184 DiffTrain build for [fca172e](fca172e)
github-actions bot
pushed a commit
that referenced
this pull request
Nov 24, 2025
…35184) With `ValidateExhaustiveMemoDependencies` we can now check exhaustive dependencies for useMemo and useCallback within the compiler, without relying on the separate exhaustive-deps rule. Until now we've bailed out of any component/hook that suppresses this rule, since the suppression _might_ affect a memoization value. Compiling code with incorrect memo deps can change behavior so this wasn't safe. The downside was that a suppression within a useEffect could prevent memoization, even though non-exhaustive deps for effects do not cause problems for memoization specifically. So here, we change to ignore ESLint suppressions if we have both the compiler's hooks validation and memo deps validations enabled. Now we just have to test out the new validation and refine before we can enable this by default. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35184). * #35201 * #35202 * #35192 * #35190 * #35186 * #35185 * __->__ #35184 DiffTrain build for [fca172e](fca172e)
josephsavona
force-pushed
the
pr35201
branch
from
cdee3c8 to
b0790cf
Compare
josephsavona
force-pushed
the
pr35201
branch
4 times, most recently
from
700f03f to
919b1ca
Compare
mofeiZ
approved these changes
Nov 25, 2025
josephsavona
added a commit
that referenced
this pull request
Nov 25, 2025
In ValidateExhaustiveDependencies, I previously changed to allow extraneous dependencies as long as they were non-reactive. Here we make that more precise, and distinguish between values that are definitely referenced in the memo function but optional as dependencies vs values that are not even referenced in the memo function. The latter now error as extraneous even if they're non-reactive. This also turned up a case where constant-folded primitives could show up as false positives of the latter category, so now we track manual deps which quality for constant folding and don't error on them. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35204). * #35213 * #35201 * __->__ #35204
Enables `@validateExhaustiveMemoizationDependencies` feature flag by default, and disables it in select tests that failed due to the change. Some of our tests intentionally use incorrect memo dependencies in order to test edge cases.
josephsavona
force-pushed
the
pr35201
branch
from
919b1ca to
330d28e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enables
@validateExhaustiveMemoizationDependenciesfeature flag by default, and disables it in select tests that failed due to the change. Some of our tests intentionally use incorrect memo dependencies in order to test edge cases.Stack created with Sapling. Best reviewed with ReviewStack.