{{ message }}
[compiler] Treat hook calls as reactive scope barriers#36857
Open
vismayIO wants to merge 6 commits into
Open
Conversation
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.
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
daltino
approved these changes
Jun 23, 2026
daltino
left a comment
There was a problem hiding this comment.
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.
acf7be4 to
2fccc4f
Compare
2fccc4f to
7048eca
Compare
4 tasks
Author
|
@daltino, Could you please let me know what changes are required in this? |
Author
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
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.

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()beforeuseResizeDetector()), then read by a ref callback and captured by another expression, gets a mutable range that spans the hook.InferReactiveScopeVariablesthen unions the ref callback, the other consumer, and that value into a single reactive scope whose range straddles the hook.FlattenScopesWithHooksOrUseprunes 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
ref-callback-with-hook-spanning-dependency— ref callback is now memoized; sprout eval passes.todo-round3_promote_used_temps) gains memoization (cache size grows); its expect is updated.