Skip to content

[compiler][rfc] enableTreatFunctionDepsAsConditional by default#66

Open
everettbu wants to merge 1 commit into
mainfrom
pr31584
Open

[compiler][rfc] enableTreatFunctionDepsAsConditional by default#66
everettbu wants to merge 1 commit into
mainfrom
pr31584

Conversation

@everettbu

@everettbu everettbu commented Dec 12, 2025

Copy link
Copy Markdown

Mirror of facebook/react#31584
Original author: mofeiZ


See react/react#31551 for context

We've rolled out internally with enableTreatFunctionDepsAsConditional: false and encountered no issues, but this is technically an unsound optimization as both flow and typescript have sources of unsoundness:

  • typing array accesses / other unknown keys as TValue | undefined (typescript's noUncheckedIndexedAccess)
  • explicit and inferred any
  • unsound typecasts

Note that removing this optimization results in less granular dependencies for ~3% of files on a large Meta project (link).

See react/react#31551 for context

We've rolled out internally with `enableTreatFunctionDepsAsConditional: false` and encountered no issues, but this is technically an unsound optimization as both flow and typescript have sources of unsoundness:
- typing array accesses / other unknown keys as `TValue | undefined`  (typescript's noUncheckedIndexedAccess)
- explicit and inferred `any`
- unsound typecasts

Note that removing this optimization results in less granular dependencies for ~3% of files on a large Meta project ([link](https://www.internalfb.com/phabricator/paste/view/P1681742214)).
@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Dec 12, 2025
@greptile-apps

greptile-apps Bot commented Dec 12, 2025

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

Changed the default value of enableTreatFunctionDepsAsConditional from false to true in the React Compiler configuration. This makes the compiler treat function expression dependencies as conditional by default, preventing potential runtime errors when accessing properties on potentially null/undefined objects.

Key changes:

  • The compiler now memoizes on root objects (e.g., foo) instead of nested properties (e.g., foo.current) inside function expressions
  • This prevents null pointer exceptions when the property access would be unsafe in the outer context
  • Trade-off: Results in coarser-grained memoization (~3% of files at Meta affected), meaning some values may re-compute more frequently
  • New behavior is safer for codebases with TypeScript/Flow unsoundness (untyped array accesses, any types, unsafe casts)
  • Two error test cases removed as they now pass with the new default
  • 35 test expectations updated to reflect the new dependency inference behavior

Confidence Score: 5/5

  • Safe to merge - well-tested behavior change with upstream validation
  • Configuration change has been validated internally at Meta with no issues. All test expectations properly updated. Trade-off between safety and optimization granularity is well-documented and acceptable.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts 5/5 Changed default value of enableTreatFunctionDepsAsConditional from false to true, with typo fix and expanded documentation
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/preserve-memo-validation/maybe-invalid-useCallback-read-maybeRef.expect.md 5/5 Test now passes - compiler correctly memoizes on maybeRef instead of erroring
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/preserve-memo-validation/error.maybe-invalid-useCallback-read-maybeRef.expect.md 5/5 Error test deleted - no longer produces CannotPreserveMemoization error with new default
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-function-uncond-access-hoisted.expect.md 5/5 Dependencies changed from a.b.c to a for safer memoization
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/useContext-read-context-in-callback.expect.md 5/5 Dependencies changed from foo.current to foo preventing potential null access

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

37 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants