Skip to content

Commit d30159e

Browse files
committed
[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: 55f5aa0 Pull Request resolved: #33114
1 parent 8079391 commit d30159e

File tree

2 files changed

+21
-24
lines changed

2 files changed

+21
-24
lines changed

compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import {
3737
import {FunctionSignature} from '../HIR/ObjectShape';
3838
import {
3939
printIdentifier,
40-
printMixedHIR,
40+
printInstructionValue,
4141
printPlace,
4242
printSourceLocation,
4343
} from '../HIR/PrintHIR';
@@ -669,7 +669,14 @@ class InferenceState {
669669
}
670670
for (const [value, kind] of this.#values) {
671671
const id = identify(value);
672-
result.values[id] = {kind, value: printMixedHIR(value)};
672+
result.values[id] = {
673+
abstract: {
674+
kind: kind.kind,
675+
context: [...kind.context].map(printPlace),
676+
reason: [...kind.reason],
677+
},
678+
value: printInstructionValue(value),
679+
};
673680
}
674681
for (const [variable, values] of this.#variables) {
675682
result.variables[`$${variable}`] = [...values].map(identify);
@@ -1844,11 +1851,11 @@ function getContextRefOperand(
18441851
): Array<Place> {
18451852
const result = [];
18461853
for (const place of eachInstructionValueOperand(instrValue)) {
1847-
if (
1848-
state.isDefined(place) &&
1849-
state.kind(place).kind === ValueKind.Context
1850-
) {
1851-
result.push(place);
1854+
if (state.isDefined(place)) {
1855+
const kind = state.kind(place);
1856+
if (kind.kind === ValueKind.Context) {
1857+
result.push(...kind.context);
1858+
}
18521859
}
18531860
}
18541861
return result;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.expect.md

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,13 @@ function Component() {
3636
## Error
3737

3838
```
39-
18 | );
40-
19 | const ref = useRef(null);
41-
> 20 | useEffect(() => {
42-
| ^^^^^^^
43-
> 21 | if (ref.current === null) {
44-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
45-
> 22 | update();
46-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
47-
> 23 | }
48-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
49-
> 24 | }, [update]);
50-
| ^^^^ 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)
51-
52-
InvalidReact: The function modifies a local variable here (14:14)
53-
25 |
54-
26 | return 'ok';
55-
27 | }
39+
12 | ...partialParams,
40+
13 | };
41+
> 14 | nextParams.param = 'value';
42+
| ^^^^^^^^^^ InvalidReact: Mutating a value returned from a function whose return value should not be mutated. Found mutation of `params` (14:14)
43+
15 | console.log(nextParams);
44+
16 | },
45+
17 | [params]
5646
```
5747
5848

0 commit comments

Comments
 (0)