[compiler][bugfix] Returned functions are not always frozen (#33047)
Fixes an edge case in React Compiler's effects inference model. Returned values should only be typed as 'frozen' if they are (1) local and (2) not a function expression which may capture and mutate this function's outer context. See test fixtures for details --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33047). * #32765 * #32747 * __->__ #33047
mofeiZ committed
Apr 30, 2025 at 15:50 UTC
12f4cb85c5847758ece81fc95b7eaedc1fb9ebd0
5 files changed
+293
-4
compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts
+26
-4
@@ -111,7 +111,10 @@ export default function inferReferenceEffects(
111
* Initial state contains function params
112
* TODO: include module declarations here as well
113
*/
114
- const initialState = InferenceState.empty(fn.env);
114
+ const initialState = InferenceState.empty(
115
+ fn.env,
116
+ options.isFunctionExpression,
117
+ );
118
const value: InstructionValue = {
119
kind: 'Primitive',
120
loc: fn.loc,
@@ -255,6 +258,7 @@ type FreezeAction = {values: Set<InstructionValue>; reason: Set<ValueReason>};
258
// Maintains a mapping of top-level variables to the kind of value they hold
259
class InferenceState {
260
env: Environment;
261
+ #isFunctionExpression: boolean;
262
263
// The kind of each value, based on its allocation site
264
#values: Map<InstructionValue, AbstractValue>;
@@ -267,16 +271,25 @@ class InferenceState {
271
272
constructor(
273
env: Environment,
274
+ isFunctionExpression: boolean,
275
values: Map<InstructionValue, AbstractValue>,
276
variables: Map<IdentifierId, Set<InstructionValue>>,
277
) {
278
this.env = env;
279
+ this.#isFunctionExpression = isFunctionExpression;
280
this.#values = values;
281
this.#variables = variables;
282
}
283
278
- static empty(env: Environment): InferenceState {
279
- return new InferenceState(env, new Map(), new Map());
284
+ static empty(
285
+ env: Environment,
286
+ isFunctionExpression: boolean,
287
+ ): InferenceState {
288
+ return new InferenceState(env, isFunctionExpression, new Map(), new Map());
289
+ }
290
+
291
+ get isFunctionExpression(): boolean {
292
+ return this.#isFunctionExpression;
293
}
294
295
// (Re)initializes a @param value with its default @param kind.
@@ -613,6 +626,7 @@ class InferenceState {
626
} else {
627
return new InferenceState(
628
this.env,
629
+ this.#isFunctionExpression,
630
nextValues ?? new Map(this.#values),
631
nextVariables ?? new Map(this.#variables),
632
);
@@ -627,6 +641,7 @@ class InferenceState {
641
clone(): InferenceState {
642
return new InferenceState(
643
this.env,
644
+ this.#isFunctionExpression,
645
new Map(this.#values),
646
new Map(this.#variables),
647
);
@@ -1781,8 +1796,15 @@ function inferBlock(
1796
if (block.terminal.kind === 'return' || block.terminal.kind === 'throw') {
1797
if (
1798
state.isDefined(operand) &&
1784
- state.kind(operand).kind === ValueKind.Context
1799
+ ((operand.identifier.type.kind === 'Function' &&
1800
+ state.isFunctionExpression) ||
1801
+ state.kind(operand).kind === ValueKind.Context)
1802
) {
1803
+ /**
1804
+ * Returned values should only be typed as 'frozen' if they are both (1)
1805
+ * local and (2) not a function expression which may capture and mutate
1806
+ * this function's outer context.
1807
+ */
1808
effect = Effect.ConditionallyMutate;
1809
} else {
1810
effect = Effect.Freeze;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-returned-inner-fn-mutates-context.expect.md
new
+92
@@ -0,0 +1,92 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import {Stringify} from 'shared-runtime';
6
+
7
+/**
8
+ * Example showing that returned inner function expressions should not be
9
+ * typed with `freeze` effects.
10
+ */
11
+function Foo({a, b}) {
12
+ 'use memo';
13
+ const obj = {};
14
+ const updaterFactory = () => {
15
+ /**
16
+ * This returned function expression *is* a local value. But it might (1)
17
+ * capture and mutate its context environment and (2) be called during
18
+ * render.
19
+ * Typing it with `freeze` effects would be incorrect as it would mean
20
+ * inferring that calls to updaterFactory()() do not mutate its captured
21
+ * context.
22
+ */
23
+ return newValue => {
24
+ obj.value = newValue;
25
+ obj.a = a;
26
+ };
27
+ };
28
+
29
+ const updater = updaterFactory();
30
+ updater(b);
31
+ return <Stringify cb={obj} shouldInvokeFns={true} />;
32
+}
33
+
34
+export const FIXTURE_ENTRYPOINT = {
35
+ fn: Foo,
36
+ params: [{a: 1, b: 2}],
37
+ sequentialRenders: [
38
+ {a: 1, b: 2},
39
+ {a: 1, b: 3},
40
+ ],
41
+};
42
+
43
+```
44
+
45
+## Code
46
+
47
+```javascript
48
+import { c as _c } from "react/compiler-runtime";
49
+import { Stringify } from "shared-runtime";
50
+
51
+/**
52
+ * Example showing that returned inner function expressions should not be
53
+ * typed with `freeze` effects.
54
+ */
55
+function Foo(t0) {
56
+ "use memo";
57
+ const $ = _c(3);
58
+ const { a, b } = t0;
59
+ let t1;
60
+ if ($[0] !== a || $[1] !== b) {
61
+ const obj = {};
62
+ const updaterFactory = () => (newValue) => {
63
+ obj.value = newValue;
64
+ obj.a = a;
65
+ };
66
+
67
+ const updater = updaterFactory();
68
+ updater(b);
69
+ t1 = <Stringify cb={obj} shouldInvokeFns={true} />;
70
+ $[0] = a;
71
+ $[1] = b;
72
+ $[2] = t1;
73
+ } else {
74
+ t1 = $[2];
75
+ }
76
+ return t1;
77
+}
78
+
79
+export const FIXTURE_ENTRYPOINT = {
80
+ fn: Foo,
81
+ params: [{ a: 1, b: 2 }],
82
+ sequentialRenders: [
83
+ { a: 1, b: 2 },
84
+ { a: 1, b: 3 },
85
+ ],
86
+};
87
+
88
+```
89
+
90
+### Eval output
91
+(kind: ok) <div>{"cb":{"value":2,"a":1},"shouldInvokeFns":true}</div>
92
+<div>{"cb":{"value":3,"a":1},"shouldInvokeFns":true}</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-returned-inner-fn-mutates-context.js
new
+37
@@ -0,0 +1,37 @@
1
+import {Stringify} from 'shared-runtime';
2
+
3
+/**
4
+ * Example showing that returned inner function expressions should not be
5
+ * typed with `freeze` effects.
6
+ */
7
+function Foo({a, b}) {
8
+ 'use memo';
9
+ const obj = {};
10
+ const updaterFactory = () => {
11
+ /**
12
+ * This returned function expression *is* a local value. But it might (1)
13
+ * capture and mutate its context environment and (2) be called during
14
+ * render.
15
+ * Typing it with `freeze` effects would be incorrect as it would mean
16
+ * inferring that calls to updaterFactory()() do not mutate its captured
17
+ * context.
18
+ */
19
+ return newValue => {
20
+ obj.value = newValue;
21
+ obj.a = a;
22
+ };
23
+ };
24
+
25
+ const updater = updaterFactory();
26
+ updater(b);
27
+ return <Stringify cb={obj} shouldInvokeFns={true} />;
28
+}
29
+
30
+export const FIXTURE_ENTRYPOINT = {
31
+ fn: Foo,
32
+ params: [{a: 1, b: 2}],
33
+ sequentialRenders: [
34
+ {a: 1, b: 2},
35
+ {a: 1, b: 3},
36
+ ],
37
+};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-returned-inner-fn-reassigns-context.expect.md
new
+101
@@ -0,0 +1,101 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import {makeArray, Stringify, useIdentity} from 'shared-runtime';
6
+
7
+/**
8
+ * Example showing that returned inner function expressions should not be
9
+ * typed with `freeze` effects.
10
+ * Also see repro-returned-inner-fn-mutates-context
11
+ */
12
+function Foo({b}) {
13
+ 'use memo';
14
+
15
+ const fnFactory = () => {
16
+ /**
17
+ * This returned function expression *is* a local value. But it might (1)
18
+ * capture and mutate its context environment and (2) be called during
19
+ * render.
20
+ * Typing it with `freeze` effects would be incorrect as it would mean
21
+ * inferring that calls to updaterFactory()() do not mutate its captured
22
+ * context.
23
+ */
24
+ return () => {
25
+ myVar = () => console.log('a');
26
+ };
27
+ };
28
+ let myVar = () => console.log('b');
29
+ useIdentity();
30
+
31
+ const fn = fnFactory();
32
+ const arr = makeArray(b);
33
+ fn(arr);
34
+ return <Stringify cb={myVar} value={arr} shouldInvokeFns={true} />;
35
+}
36
+
37
+export const FIXTURE_ENTRYPOINT = {
38
+ fn: Foo,
39
+ params: [{b: 1}],
40
+ sequentialRenders: [{b: 1}, {b: 2}],
41
+};
42
+
43
+```
44
+
45
+## Code
46
+
47
+```javascript
48
+import { c as _c } from "react/compiler-runtime";
49
+import { makeArray, Stringify, useIdentity } from "shared-runtime";
50
+
51
+/**
52
+ * Example showing that returned inner function expressions should not be
53
+ * typed with `freeze` effects.
54
+ * Also see repro-returned-inner-fn-mutates-context
55
+ */
56
+function Foo(t0) {
57
+ "use memo";
58
+ const $ = _c(3);
59
+ const { b } = t0;
60
+
61
+ const fnFactory = () => () => {
62
+ myVar = _temp;
63
+ };
64
+
65
+ let myVar;
66
+ myVar = _temp2;
67
+ useIdentity();
68
+
69
+ const fn = fnFactory();
70
+ const arr = makeArray(b);
71
+ fn(arr);
72
+ let t1;
73
+ if ($[0] !== arr || $[1] !== myVar) {
74
+ t1 = <Stringify cb={myVar} value={arr} shouldInvokeFns={true} />;
75
+ $[0] = arr;
76
+ $[1] = myVar;
77
+ $[2] = t1;
78
+ } else {
79
+ t1 = $[2];
80
+ }
81
+ return t1;
82
+}
83
+function _temp2() {
84
+ return console.log("b");
85
+}
86
+function _temp() {
87
+ return console.log("a");
88
+}
89
+
90
+export const FIXTURE_ENTRYPOINT = {
91
+ fn: Foo,
92
+ params: [{ b: 1 }],
93
+ sequentialRenders: [{ b: 1 }, { b: 2 }],
94
+};
95
+
96
+```
97
+
98
+### Eval output
99
+(kind: ok) <div>{"cb":{"kind":"Function"},"value":[1],"shouldInvokeFns":true}</div>
100
+<div>{"cb":{"kind":"Function"},"value":[2],"shouldInvokeFns":true}</div>
101
+logs: ['a','a']
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-returned-inner-fn-reassigns-context.js
new
+37
@@ -0,0 +1,37 @@
1
+import {makeArray, Stringify, useIdentity} from 'shared-runtime';
2
+
3
+/**
4
+ * Example showing that returned inner function expressions should not be
5
+ * typed with `freeze` effects.
6
+ * Also see repro-returned-inner-fn-mutates-context
7
+ */
8
+function Foo({b}) {
9
+ 'use memo';
10
+
11
+ const fnFactory = () => {
12
+ /**
13
+ * This returned function expression *is* a local value. But it might (1)
14
+ * capture and mutate its context environment and (2) be called during
15
+ * render.
16
+ * Typing it with `freeze` effects would be incorrect as it would mean
17
+ * inferring that calls to updaterFactory()() do not mutate its captured
18
+ * context.
19
+ */
20
+ return () => {
21
+ myVar = () => console.log('a');
22
+ };
23
+ };
24
+ let myVar = () => console.log('b');
25
+ useIdentity();
26
+
27
+ const fn = fnFactory();
28
+ const arr = makeArray(b);
29
+ fn(arr);
30
+ return <Stringify cb={myVar} value={arr} shouldInvokeFns={true} />;
31
+}
32
+
33
+export const FIXTURE_ENTRYPOINT = {
34
+ fn: Foo,
35
+ params: [{b: 1}],
36
+ sequentialRenders: [{b: 1}, {b: 2}],
37
+};