[compiler] Correctly infer context mutation places as outer (context) places

The issue in the previous PR was due to a ContextMutation function effect having a place that wasn't one of the functions' context variables. What was happening is that the `getContextRefOperand()` helper wasn't following aliases. If an operand had a context type, we recorded the operand as the context place — but instead we should be looking through to the context places of the abstract value.

With this change the fixture now fails for a different reason — we infer this as a mutation of `params` and reject it because `params` is frozen (hook return value). This case is clearly a false positive: the mutation is on the outer, new `nextParams` object and can't possibly mutate `params`. Need to think more about what to do here but this is clearly more precise in terms of which variable we record as the context variable.

ghstack-source-id: ad3902b12d
Pull Request resolved: https://github.com/facebook/react/pull/33114
This commit is contained in:
Joe Savona
2025-05-12 17:00:39 -07:00
parent 451b00eb09
commit d3c74d0373
2 changed files with 21 additions and 24 deletions
@@ -37,7 +37,7 @@ import {
import {FunctionSignature} from '../HIR/ObjectShape';
import {
printIdentifier,
printMixedHIR,
printInstructionValue,
printPlace,
printSourceLocation,
} from '../HIR/PrintHIR';
@@ -669,7 +669,14 @@ class InferenceState {
}
for (const [value, kind] of this.#values) {
const id = identify(value);
result.values[id] = {kind, value: printMixedHIR(value)};
result.values[id] = {
abstract: {
kind: kind.kind,
context: [...kind.context].map(printPlace),
reason: [...kind.reason],
},
value: printInstructionValue(value),
};
}
for (const [variable, values] of this.#variables) {
result.variables[`$${variable}`] = [...values].map(identify);
@@ -1844,11 +1851,11 @@ function getContextRefOperand(
): Array<Place> {
const result = [];
for (const place of eachInstructionValueOperand(instrValue)) {
if (
state.isDefined(place) &&
state.kind(place).kind === ValueKind.Context
) {
result.push(place);
if (state.isDefined(place)) {
const kind = state.kind(place);
if (kind.kind === ValueKind.Context) {
result.push(...kind.context);
}
}
}
return result;
@@ -36,23 +36,13 @@ function Component() {
## Error
```
18 | );
19 | const ref = useRef(null);
> 20 | useEffect(() => {
| ^^^^^^^
> 21 | if (ref.current === null) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 22 | update();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 23 | }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 24 | }, [update]);
| ^^^^ InvalidReact: This argument is a function which modifies local variables when called, which can bypass memoization and cause the UI not to update. Functions that are returned from hooks, passed as arguments to hooks, or passed as props to components may not mutate local variables (20:24)
InvalidReact: The function modifies a local variable here (14:14)
25 |
26 | return 'ok';
27 | }
12 | ...partialParams,
13 | };
> 14 | nextParams.param = 'value';
| ^^^^^^^^^^ InvalidReact: Mutating a value returned from a function whose return value should not be mutated. Found mutation of `params` (14:14)
15 | console.log(nextParams);
16 | },
17 | [params]
```