[compiler] Treat hook calls as reactive scope barriers by vismayIO · Pull Request #36857 · react/react · GitHub
Skip to content

[compiler] Treat hook calls as reactive scope barriers#36857

Open
vismayIO wants to merge 6 commits into
react:mainfrom
vismayIO:fix-36807-ref-callback-hook-scope
Open

[compiler] Treat hook calls as reactive scope barriers#36857
vismayIO wants to merge 6 commits into
react:mainfrom
vismayIO:fix-36807-ref-callback-hook-scope

Conversation

@vismayIO

@vismayIO vismayIO commented Jun 23, 2026

Copy link
Copy Markdown

Summary

Fixes #36800 — a ref callback (and other values) failing to memoize "in all situations."

A conservatively-mutable value created before a hook call (e.g. const isChromatic = getIsChromatic() before useResizeDetector()), then read by a ref callback and captured by another expression, gets a mutable range that spans the hook. InferReactiveScopeVariables then unions the ref callback, the other consumer, and that value into a single reactive scope whose range straddles the hook. FlattenScopesWithHooksOrUse prunes any hook-spanning scope wholesale (scopes can't be conditional on a hook), so the ref callback is never memoized — producing a fresh ref identity every render and forcing React to detach/reattach the ref each render.

Fix

Enforce the existing invariant "a reactive scope can never span a hook/use call" during scope formation instead of only cleaning it up destructively afterwards:

  • InferReactiveScopeVariables.findDisjointMutableValues — never union an instruction's operands across a preceding hook/use call. The instruction's own lvalue is always retained, so post-hook values still scope together; the cross-hook value becomes a scope dependency instead.
  • MergeOverlappingReactiveScopesHIR.visitPlace — don't re-merge inner scopes into a hook-spanning outer scope for non-mutating references (!isMutableEffect). A plain read/freeze does not require the merge for correctness; only a real mutation does, so reads stay independently memoizeable.

Result: the ref callback memoizes independently (depending on the stable ref setter); an expression that genuinely captures the value still merges and stays un-memoized, which is correct.

Test plan

  • New fixture ref-callback-with-hook-spanning-dependency — ref callback is now memoized; sprout eval passes.
  • Full fixture suite: no regressions. One fixture (todo-round3_promote_used_temps) gains memoization (cache size grows); its expect is updated.
  • typecheck + eslint clean.

Fixes react#36807. A conservatively-mutable value created before a hook call
and read by a ref callback was dragged into a reactive scope spanning the
hook, which FlattenScopesWithHooksOrUse prunes wholesale, de-memoizing the
ref callback (unstable ref identity every render).

InferReactiveScopeVariables no longer unions operands across a hook
barrier, and MergeOverlappingReactiveScopes no longer re-merges scopes
across a hook for non-mutating references. Reads stay independently
memoized; genuine captures still merge.
@meta-cla

meta-cla Bot commented Jun 23, 2026

Copy link
Copy Markdown

@meta-cla meta-cla Bot added the CLA Signed label Jun 23, 2026
@meta-cla

meta-cla Bot commented Jun 23, 2026

Copy link
Copy Markdown

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@daltino daltino 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.

This PR improves the compiler by treating hook and use() calls as reactive scope barriers. Notably, it addresses an issue where hooks incorrectly spanned reactive scopes, which could lead to unintended side effects during compilation. The changes are well-structured, and the addition of tests, including a regression test for a linked issue, ensures correctness. The implementation and its impact are clear.

@vismayIO vismayIO requested a review from daltino June 24, 2026 05:23
@vismayIO vismayIO force-pushed the fix-36807-ref-callback-hook-scope branch from acf7be4 to 2fccc4f Compare June 24, 2026 05:32
@vismayIO

Copy link
Copy Markdown
Author

@daltino, Could you please let me know what changes are required in this?

@vismayIO

vismayIO commented Jun 25, 2026

Copy link
Copy Markdown
Author

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Compiler Bug]: ref callback not memoized in all situations

2 participants