[compiler] Patch for reactive refs in inferred effect dependencies (#32991)
Inferred effect dependencies and inlined jsx (both experimental features) rely on `InferReactivePlaces` to determine their dependencies. Since adding type inference for phi nodes (https://github.com/facebook/react/pull/30796), we have been incorrectly inferring stable-typed value blocks (e.g. `props.cond ? setState1 : setState2`) as non-reactive. This fix patches InferReactivePlaces instead of adding a new pass since we want non-reactivity propagated correctly
mofeiZ committed
Apr 25, 2025 at 15:42 UTC
2d0a5e399f195bfc98fc5e1efa37aab9fa53e097
5 files changed
+234
-5
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+34
@@ -1738,6 +1738,40 @@ export function isStableType(id: Identifier): boolean {
1738
);
1739
}
1740
1741
+export function isStableTypeContainer(id: Identifier): boolean {
1742
+ const type_ = id.type;
1743
+ if (type_.kind !== 'Object') {
1744
+ return false;
1745
+ }
1746
+ return (
1747
+ isUseStateType(id) || // setState
1748
+ type_.shapeId === 'BuiltInUseActionState' || // setActionState
1749
+ isUseReducerType(id) || // dispatcher
1750
+ type_.shapeId === 'BuiltInUseTransition' // startTransition
1751
+ );
1752
+}
1753
+
1754
+export function evaluatesToStableTypeOrContainer(
1755
+ env: Environment,
1756
+ {value}: Instruction,
1757
+): boolean {
1758
+ if (value.kind === 'CallExpression' || value.kind === 'MethodCall') {
1759
+ const callee =
1760
+ value.kind === 'CallExpression' ? value.callee : value.property;
1761
+
1762
+ const calleeHookKind = getHookKind(env, callee.identifier);
1763
+ switch (calleeHookKind) {
1764
+ case 'useState':
1765
+ case 'useReducer':
1766
+ case 'useActionState':
1767
+ case 'useRef':
1768
+ case 'useTransition':
1769
+ return true;
1770
+ }
1771
+ }
1772
+ return false;
1773
+}
1774
+
1775
export function isUseEffectHookType(id: Identifier): boolean {
1776
return (
1777
id.type.kind === 'Function' && id.type.shapeId === 'BuiltInUseEffectHook'
compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts
+111
-1
@@ -9,14 +9,19 @@ import {CompilerError} from '..';
9
import {
10
BlockId,
11
Effect,
12
+ Environment,
13
HIRFunction,
14
Identifier,
15
IdentifierId,
16
+ Instruction,
17
Place,
18
computePostDominatorTree,
19
+ evaluatesToStableTypeOrContainer,
20
getHookKind,
21
isStableType,
22
+ isStableTypeContainer,
23
isUseOperator,
24
+ isUseRefType,
25
} from '../HIR';
26
import {PostDominator} from '../HIR/Dominator';
27
import {
@@ -31,6 +36,103 @@ import {
36
import DisjointSet from '../Utils/DisjointSet';
37
import {assertExhaustive} from '../Utils/utils';
38
39
+/**
40
+ * Side map to track and propagate sources of stability (i.e. hook calls such as
41
+ * `useRef()` and property reads such as `useState()[1]). Note that this
42
+ * requires forward data flow analysis since stability is not part of React
43
+ * Compiler's type system.
44
+ */
45
+class StableSidemap {
46
+ map: Map<IdentifierId, {isStable: boolean}> = new Map();
47
+ env: Environment;
48
+
49
+ constructor(env: Environment) {
50
+ this.env = env;
51
+ }
52
+
53
+ handleInstruction(instr: Instruction): void {
54
+ const {value, lvalue} = instr;
55
+
56
+ switch (value.kind) {
57
+ case 'CallExpression':
58
+ case 'MethodCall': {
59
+ /**
60
+ * Sources of stability are known hook calls
61
+ */
62
+ if (evaluatesToStableTypeOrContainer(this.env, instr)) {
63
+ if (isStableType(lvalue.identifier)) {
64
+ this.map.set(lvalue.identifier.id, {
65
+ isStable: true,
66
+ });
67
+ } else {
68
+ this.map.set(lvalue.identifier.id, {
69
+ isStable: false,
70
+ });
71
+ }
72
+ } else if (
73
+ this.env.config.enableTreatRefLikeIdentifiersAsRefs &&
74
+ isUseRefType(lvalue.identifier)
75
+ ) {
76
+ this.map.set(lvalue.identifier.id, {
77
+ isStable: true,
78
+ });
79
+ }
80
+ break;
81
+ }
82
+
83
+ case 'Destructure':
84
+ case 'PropertyLoad': {
85
+ /**
86
+ * PropertyLoads may from stable containers may also produce stable
87
+ * values. ComputedLoads are technically safe for now (as all stable
88
+ * containers have differently-typed elements), but are not handled as
89
+ * they should be rare anyways.
90
+ */
91
+ const source =
92
+ value.kind === 'Destructure'
93
+ ? value.value.identifier.id
94
+ : value.object.identifier.id;
95
+ const entry = this.map.get(source);
96
+ if (entry) {
97
+ for (const lvalue of eachInstructionLValue(instr)) {
98
+ if (isStableTypeContainer(lvalue.identifier)) {
99
+ this.map.set(lvalue.identifier.id, {
100
+ isStable: false,
101
+ });
102
+ } else if (isStableType(lvalue.identifier)) {
103
+ this.map.set(lvalue.identifier.id, {
104
+ isStable: true,
105
+ });
106
+ }
107
+ }
108
+ }
109
+ break;
110
+ }
111
+
112
+ case 'StoreLocal': {
113
+ const entry = this.map.get(value.value.identifier.id);
114
+ if (entry) {
115
+ this.map.set(lvalue.identifier.id, entry);
116
+ this.map.set(value.lvalue.place.identifier.id, entry);
117
+ }
118
+ break;
119
+ }
120
+
121
+ case 'LoadLocal': {
122
+ const entry = this.map.get(value.place.identifier.id);
123
+ if (entry) {
124
+ this.map.set(lvalue.identifier.id, entry);
125
+ }
126
+ break;
127
+ }
128
+ }
129
+ }
130
+
131
+ isStable(id: IdentifierId): boolean {
132
+ const entry = this.map.get(id);
133
+ return entry != null ? entry.isStable : false;
134
+ }
135
+}
136
/*
137
* Infers which `Place`s are reactive, ie may *semantically* change
138
* over the course of the component/hook's lifetime. Places are reactive
@@ -111,6 +213,7 @@ import {assertExhaustive} from '../Utils/utils';
213
*/
214
export function inferReactivePlaces(fn: HIRFunction): void {
215
const reactiveIdentifiers = new ReactivityMap(findDisjointMutableValues(fn));
216
+ const stableIdentifierSources = new StableSidemap(fn.env);
217
for (const param of fn.params) {
218
const place = param.kind === 'Identifier' ? param : param.place;
219
reactiveIdentifiers.markReactive(place);
@@ -184,6 +287,7 @@ export function inferReactivePlaces(fn: HIRFunction): void {
287
}
288
}
289
for (const instruction of block.instructions) {
290
+ stableIdentifierSources.handleInstruction(instruction);
291
const {value} = instruction;
292
let hasReactiveInput = false;
293
/*
@@ -218,7 +322,13 @@ export function inferReactivePlaces(fn: HIRFunction): void {
322
323
if (hasReactiveInput) {
324
for (const lvalue of eachInstructionLValue(instruction)) {
221
- if (isStableType(lvalue.identifier)) {
325
+ /**
326
+ * Note that it's not correct to mark all stable-typed identifiers
327
+ * as non-reactive, since ternaries and other value blocks can
328
+ * produce reactive identifiers typed as these.
329
+ * (e.g. `props.cond ? setState1 : setState2`)
330
+ */
331
+ if (stableIdentifierSources.isStable(lvalue.identifier.id)) {
332
continue;
333
}
334
reactiveIdentifiers.markReactive(lvalue);
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-ref-ternary.expect.md
new
+69
@@ -0,0 +1,69 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @inferEffectDependencies
6
+import {useRef, useEffect} from 'react';
7
+import {print, mutate} from 'shared-runtime';
8
+
9
+function Component({cond}) {
10
+ const arr = useRef([]);
11
+ const other = useRef([]);
12
+ // Although arr and other are both stable, derived is not
13
+ const derived = cond ? arr : other;
14
+ useEffect(() => {
15
+ mutate(derived.current);
16
+ print(derived.current);
17
+ });
18
+ return arr;
19
+}
20
+
21
+```
22
+
23
+## Code
24
+
25
+```javascript
26
+import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
27
+import { useRef, useEffect } from "react";
28
+import { print, mutate } from "shared-runtime";
29
+
30
+function Component(t0) {
31
+ const $ = _c(4);
32
+ const { cond } = t0;
33
+ let t1;
34
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
35
+ t1 = [];
36
+ $[0] = t1;
37
+ } else {
38
+ t1 = $[0];
39
+ }
40
+ const arr = useRef(t1);
41
+ let t2;
42
+ if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
43
+ t2 = [];
44
+ $[1] = t2;
45
+ } else {
46
+ t2 = $[1];
47
+ }
48
+ const other = useRef(t2);
49
+
50
+ const derived = cond ? arr : other;
51
+ let t3;
52
+ if ($[2] !== derived) {
53
+ t3 = () => {
54
+ mutate(derived.current);
55
+ print(derived.current);
56
+ };
57
+ $[2] = derived;
58
+ $[3] = t3;
59
+ } else {
60
+ t3 = $[3];
61
+ }
62
+ useEffect(t3, [derived]);
63
+ return arr;
64
+}
65
+
66
+```
67
+
68
+### Eval output
69
+(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-ref-ternary.js
new
+15
@@ -0,0 +1,15 @@
1
+// @inferEffectDependencies
2
+import {useRef, useEffect} from 'react';
3
+import {print, mutate} from 'shared-runtime';
4
+
5
+function Component({cond}) {
6
+ const arr = useRef([]);
7
+ const other = useRef([]);
8
+ // Although arr and other are both stable, derived is not
9
+ const derived = cond ? arr : other;
10
+ useEffect(() => {
11
+ mutate(derived.current);
12
+ print(derived.current);
13
+ });
14
+ return arr;
15
+}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.expect.md
+5
-4
@@ -83,10 +83,10 @@ export const FIXTURE_ENTRYPOINT = {
83
import { c as _c2 } from "react/compiler-runtime"; // @inlineJsxTransform
84
85
function Parent(t0) {
86
- const $ = _c2(2);
86
+ const $ = _c2(3);
87
const { children, ref } = t0;
88
let t1;
89
- if ($[0] !== children) {
89
+ if ($[0] !== children || $[1] !== ref) {
90
if (DEV) {
91
t1 = <div ref={ref}>{children}</div>;
92
} else {
@@ -99,9 +99,10 @@ function Parent(t0) {
99
};
100
}
101
$[0] = children;
102
- $[1] = t1;
102
+ $[1] = ref;
103
+ $[2] = t1;
104
} else {
104
- t1 = $[1];
105
+ t1 = $[2];
106
}
107
return t1;
108
}