[compiler] fix false positive "mutate frozen" validation with refs #33993
+664
−113
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
Fixes two related cases of mutation of potentially frozen values.
The first is method calls on frozen values. Previously, we modeled unknown function calls as potentially aliasing their receiver+args into the return value. If the receiver or argument were known to be frozen, then we would downgrade the `Alias` effect into an `ImmutableCapture`. However, within a function expression it's possible to call a function using a frozen value as an argument (that gets `Alias`-ed into the return) but where we don't have the context locally to know that the value is frozen.
This results in cases like this:
```js
const frozen = useContext(...);
useEffect(() => {
frozen.method().property = true;
^^^^^^^^^^^^^^^^^^^^^^^^ cannot mutate frozen value
}, [...]);
```
Within the function we would infer:
```
t0 = MethodCall ...
Create t0 = mutable
Alias t0 <- frozen
t1 = PropertyStore ...
Mutate t0
```
And then transitively infer the function expression as having a `Mutate 'frozen'` effect, which when evaluated against the outer context (`frozen` is frozen) is an error.
The fix is to model unknown function calls as _maybe_ aliasing their receiver/args in the return, and then considering mutations of a maybe-aliased value to only be a conditional mutation of the source:
```
t0 = MethodCall ...
Create t0 = mutable
MaybeAlias t0 <- frozen // maybe alias now
t1 = PropertyStore ...
Mutate t0
```
Then, the `Mutate t0` turns into a `MutateConditional 'frozen'`, which just gets ignored when we process the outer context.
The second, related fix is for known mutation of phis that may be a frozen value. The previous inference model correctly recorded these as errors, the new model does not. We now correctly report a validation error for this case in the new model.
This was referenced Jul 25, 2025
github-actions
bot
added
the
React Core Team
The test case here previously reported a "Cannot modify local variables after render completes" error (from ValidateNoFreezingKnownMutableFunctions). This happens because one of the functions passed to a hook clearly mutates a ref — except that we try to ignore mutations of refs! The problem in this case is that the `const ref = ...` was getting converted to a context variable since the ref is accessed in a function before its declaration. We don't infer types for context variables at all, and our ref handling is based on types, so we failed to ignore this ref mutation. The fix is to recognize that `StoreLocal const ...` is a special case: the variable may be referenced in code before the declaration, but at runtime it's either a TDZ error or the variable will have the type from the declaration. So we can safely infer a type.
josephsavona
added a commit
that referenced
this pull request
Jul 25, 2025
#33984) Fixes two related cases of mutation of potentially frozen values. The first is method calls on frozen values. Previously, we modeled unknown function calls as potentially aliasing their receiver+args into the return value. If the receiver or argument were known to be frozen, then we would downgrade the `Alias` effect into an `ImmutableCapture`. However, within a function expression it's possible to call a function using a frozen value as an argument (that gets `Alias`-ed into the return) but where we don't have the context locally to know that the value is frozen. This results in cases like this: ```js const frozen = useContext(...); useEffect(() => { frozen.method().property = true; ^^^^^^^^^^^^^^^^^^^^^^^^ cannot mutate frozen value }, [...]); ``` Within the function we would infer: ``` t0 = MethodCall ... Create t0 = mutable Alias t0 <- frozen t1 = PropertyStore ... Mutate t0 ``` And then transitively infer the function expression as having a `Mutate 'frozen'` effect, which when evaluated against the outer context (`frozen` is frozen) is an error. The fix is to model unknown function calls as _maybe_ aliasing their receiver/args in the return, and then considering mutations of a maybe-aliased value to only be a conditional mutation of the source: ``` t0 = MethodCall ... Create t0 = mutable MaybeAlias t0 <- frozen // maybe alias now t1 = PropertyStore ... Mutate t0 ``` Then, the `Mutate t0` turns into a `MutateConditional 'frozen'`, which just gets ignored when we process the outer context. The second, related fix is for known mutation of phis that may be a frozen value. The previous inference model correctly recorded these as errors, the new model does not. We now correctly report a validation error for this case in the new model. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33984). * #33993 * #33991 * __->__ #33984
josephsavona
added a commit
that referenced
this pull request
Jul 25, 2025
--- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33991). * #33993 * __->__ #33991 * #33984
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.
The test case here previously reported a "Cannot modify local variables after render completes" error (from ValidateNoFreezingKnownMutableFunctions). This happens because one of the functions passed to a hook clearly mutates a ref — except that we try to ignore mutations of refs! The problem in this case is that the
const ref = ...was getting converted to a context variable since the ref is accessed in a function before its declaration. We don't infer types for context variables at all, and our ref handling is based on types, so we failed to ignore this ref mutation.The fix is to recognize that
StoreLocal const ...is a special case: the variable may be referenced in code before the declaration, but at runtime it's either a TDZ error or the variable will have the type from the declaration. So we can safely infer a type.Stack created with Sapling. Best reviewed with ReviewStack.