[compiler] Fix false positive hook return mutation error (#34424)
This was fun. We previously added the MaybeAlias effect in #33984 in order to describe the semantic that an unknown function call _may_ alias its return value in its result, but that we don't know this for sure. We record mutations through MaybeAlias edges when walking backward in the data flow graph, but downgrade them to conditional mutations. See the original PR for full context. That change was sufficient for the original case like ```js const frozen = useContext(); useEffect(() => { frozen.method().property = true; }, [...]); ``` But it wasn't sufficient for cases where the aliasing occured between operands: ```js const dispatch = useDispatch(); <div onClick={(e) => { dispatch(...e.target.value) e.target.value = ...; }} /> ``` Here we would record a `Capture dispatch <- e.target` effect. Then during processing of the `event.target.value = ...` assignment we'd eventually _forward_ from `event` to `dispatch` (along a MaybeAlias edge). But in #33984 I missed that this forward walk also has to downgrade to conditional. In addition to that change, we also have to be a bit more precise about which set of effects we create for alias/capture/maybe-alias. The new logic is a bit clearer, I think: * If the value is frozen, it's an ImmutableCapture edge * If the values are mutable, it's a Capture * If it's a context->context, context->mutable, or mutable->context, count it as MaybeAlias.
Joseph Savona committed
Sep 9, 2025 at 14:07 UTC
acada3035fe2a0dacfafc0ad78914e77d11fb823
4 files changed
+173
-28
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
+48
-26
@@ -748,10 +748,14 @@ function applyEffect(
748
case 'Alias':
749
case 'Capture': {
750
CompilerError.invariant(
751
- effect.kind === 'Capture' || initialized.has(effect.into.identifier.id),
751
+ effect.kind === 'Capture' ||
752
+ effect.kind === 'MaybeAlias' ||
753
+ initialized.has(effect.into.identifier.id),
754
{
753
- reason: `Expected destination value to already be initialized within this instruction for Alias effect`,
754
- description: `Destination ${printPlace(effect.into)} is not initialized in this instruction`,
755
+ reason: `Expected destination to already be initialized within this instruction`,
756
+ description:
757
+ `Destination ${printPlace(effect.into)} is not initialized in this ` +
758
+ `instruction for effect ${printAliasingEffect(effect)}`,
759
details: [
760
{
761
kind: 'error',
@@ -767,49 +771,67 @@ function applyEffect(
771
* copy-on-write semantics, then we can prune the effect
772
*/
773
const intoKind = state.kind(effect.into).kind;
770
- let isMutableDesination: boolean;
774
+ let destinationType: 'context' | 'mutable' | null = null;
775
switch (intoKind) {
772
- case ValueKind.Context:
773
- case ValueKind.Mutable:
774
- case ValueKind.MaybeFrozen: {
775
- isMutableDesination = true;
776
+ case ValueKind.Context: {
777
+ destinationType = 'context';
778
break;
779
}
778
- default: {
779
- isMutableDesination = false;
780
+ case ValueKind.Mutable:
781
+ case ValueKind.MaybeFrozen: {
782
+ destinationType = 'mutable';
783
break;
784
}
785
}
786
const fromKind = state.kind(effect.from).kind;
784
- let isMutableReferenceType: boolean;
787
+ let sourceType: 'context' | 'mutable' | 'frozen' | null = null;
788
switch (fromKind) {
789
+ case ValueKind.Context: {
790
+ sourceType = 'context';
791
+ break;
792
+ }
793
case ValueKind.Global:
794
case ValueKind.Primitive: {
788
- isMutableReferenceType = false;
795
break;
796
}
797
case ValueKind.Frozen: {
792
- isMutableReferenceType = false;
793
- applyEffect(
794
- context,
795
- state,
796
- {
797
- kind: 'ImmutableCapture',
798
- from: effect.from,
799
- into: effect.into,
800
- },
801
- initialized,
802
- effects,
803
- );
798
+ sourceType = 'frozen';
799
break;
800
}
801
default: {
807
- isMutableReferenceType = true;
802
+ sourceType = 'mutable';
803
break;
804
}
805
}
811
- if (isMutableDesination && isMutableReferenceType) {
806
+
807
+ if (sourceType === 'frozen') {
808
+ applyEffect(
809
+ context,
810
+ state,
811
+ {
812
+ kind: 'ImmutableCapture',
813
+ from: effect.from,
814
+ into: effect.into,
815
+ },
816
+ initialized,
817
+ effects,
818
+ );
819
+ } else if (
820
+ (sourceType === 'mutable' && destinationType === 'mutable') ||
821
+ effect.kind === 'MaybeAlias'
822
+ ) {
823
effects.push(effect);
824
+ } else if (
825
+ (sourceType === 'context' && destinationType != null) ||
826
+ (sourceType === 'mutable' && destinationType === 'context')
827
+ ) {
828
+ applyEffect(
829
+ context,
830
+ state,
831
+ {kind: 'MaybeAlias', from: effect.from, into: effect.into},
832
+ initialized,
833
+ effects,
834
+ );
835
}
836
break;
837
}
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts
+13
-2
@@ -779,7 +779,13 @@ class AliasingState {
779
if (edge.index >= index) {
780
break;
781
}
782
- queue.push({place: edge.node, transitive, direction: 'forwards', kind});
782
+ queue.push({
783
+ place: edge.node,
784
+ transitive,
785
+ direction: 'forwards',
786
+ // Traversing a maybeAlias edge always downgrades to conditional mutation
787
+ kind: edge.kind === 'maybeAlias' ? MutationKind.Conditional : kind,
788
+ });
789
}
790
for (const [alias, when] of node.createdFrom) {
791
if (when >= index) {
@@ -807,7 +813,12 @@ class AliasingState {
813
if (when >= index) {
814
continue;
815
}
810
- queue.push({place: alias, transitive, direction: 'backwards', kind});
816
+ queue.push({
817
+ place: alias,
818
+ transitive,
819
+ direction: 'backwards',
820
+ kind,
821
+ });
822
}
823
/**
824
* MaybeAlias indicates potential data flow from unknown function calls,
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.expect.md
new
+82
@@ -0,0 +1,82 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @compilationMode:"infer"
6
+function Component() {
7
+ const dispatch = useDispatch();
8
+ // const [state, setState] = useState(0);
9
+
10
+ return (
11
+ <div>
12
+ <input
13
+ type="file"
14
+ onChange={event => {
15
+ dispatch(...event.target);
16
+ event.target.value = '';
17
+ }}
18
+ />
19
+ </div>
20
+ );
21
+}
22
+
23
+function useDispatch() {
24
+ 'use no memo';
25
+ // skip compilation to make it easier to debug the above function
26
+ return (...values) => {
27
+ console.log(...values);
28
+ };
29
+}
30
+
31
+export const FIXTURE_ENTRYPOINT = {
32
+ fn: Component,
33
+ params: [{}],
34
+};
35
+
36
+```
37
+
38
+## Code
39
+
40
+```javascript
41
+import { c as _c } from "react/compiler-runtime"; // @compilationMode:"infer"
42
+function Component() {
43
+ const $ = _c(2);
44
+ const dispatch = useDispatch();
45
+ let t0;
46
+ if ($[0] !== dispatch) {
47
+ t0 = (
48
+ <div>
49
+ <input
50
+ type="file"
51
+ onChange={(event) => {
52
+ dispatch(...event.target);
53
+ event.target.value = "";
54
+ }}
55
+ />
56
+ </div>
57
+ );
58
+ $[0] = dispatch;
59
+ $[1] = t0;
60
+ } else {
61
+ t0 = $[1];
62
+ }
63
+ return t0;
64
+}
65
+
66
+function useDispatch() {
67
+ "use no memo";
68
+ // skip compilation to make it easier to debug the above function
69
+ return (...values) => {
70
+ console.log(...values);
71
+ };
72
+}
73
+
74
+export const FIXTURE_ENTRYPOINT = {
75
+ fn: Component,
76
+ params: [{}],
77
+};
78
+
79
+```
80
+
81
+### Eval output
82
+(kind: ok) <div><input type="file"></div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-dispatch-spread-event-marks-event-frozen.js
new
+30
@@ -0,0 +1,30 @@
1
+// @compilationMode:"infer"
2
+function Component() {
3
+ const dispatch = useDispatch();
4
+ // const [state, setState] = useState(0);
5
+
6
+ return (
7
+ <div>
8
+ <input
9
+ type="file"
10
+ onChange={event => {
11
+ dispatch(...event.target);
12
+ event.target.value = '';
13
+ }}
14
+ />
15
+ </div>
16
+ );
17
+}
18
+
19
+function useDispatch() {
20
+ 'use no memo';
21
+ // skip compilation to make it easier to debug the above function
22
+ return (...values) => {
23
+ console.log(...values);
24
+ };
25
+}
26
+
27
+export const FIXTURE_ENTRYPOINT = {
28
+ fn: Component,
29
+ params: [{}],
30
+};