[compiler] Validate against mutable functions being frozen
This revisits a validation I built a while ago, trying to make it more strict this time to ensure that it's high-signal. We detect function expressions which are *known* mutable — they definitely can modify a variable defined outside of the function expression itself (modulo control flow). This uses types to look for known Store and Mutate effects only, and disregards mutations of effects. Any such function passed to a location with a Freeze effect is reported as a validation error. This is behind a flag and disabled by default. If folks agree this makes sense to revisit, i'll test out internally and we can consider enabling by default. ghstack-source-id: 075a731444ce95e52dbd5ea3be85c16d428927f5 Pull Request resolved: https://github.com/facebook/react/pull/33079
Joe Savona committed
May 3, 2025 at 09:15 UTC
0db8db178c1521f979535bdba32bf9db9f47ca05
9 files changed
+266
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts
+5
@@ -103,6 +103,7 @@ import {transformFire} from '../Transform';
103
import {validateNoImpureFunctionsInRender} from '../Validation/ValidateNoImpureFunctionsInRender';
104
import {CompilerError} from '..';
105
import {validateStaticComponents} from '../Validation/ValidateStaticComponents';
106
+import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoFreezingKnownMutableFunctions';
107
108
export type CompilerPipelineValue =
109
| {kind: 'ast'; name: string; value: CodegenFunction}
@@ -274,6 +275,10 @@ function runWithEnvironment(
275
if (env.config.validateNoImpureFunctionsInRender) {
276
validateNoImpureFunctionsInRender(hir).unwrap();
277
}
278
+
279
+ if (env.config.validateNoFreezingKnownMutableFunctions) {
280
+ validateNoFreezingKnownMutableFunctions(hir).unwrap();
281
+ }
282
}
283
284
inferReactivePlaces(hir);
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
+5
@@ -367,6 +367,11 @@ const EnvironmentConfigSchema = z.object({
367
*/
368
validateNoImpureFunctionsInRender: z.boolean().default(false),
369
370
+ /**
371
+ * Validate against passing mutable functions to hooks
372
+ */
373
+ validateNoFreezingKnownMutableFunctions: z.boolean().default(false),
374
+
375
/*
376
* When enabled, the compiler assumes that hooks follow the Rules of React:
377
* - Hooks may memoize computation based on any of their parameters, thus
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoFreezingKnownMutableFunctions.ts
new
+130
@@ -0,0 +1,130 @@
1
+/**
2
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ */
7
+
8
+import {CompilerError, Effect, ErrorSeverity} from '..';
9
+import {
10
+ FunctionEffect,
11
+ HIRFunction,
12
+ IdentifierId,
13
+ isMutableEffect,
14
+ isRefOrRefLikeMutableType,
15
+ Place,
16
+} from '../HIR';
17
+import {
18
+ eachInstructionValueOperand,
19
+ eachTerminalOperand,
20
+} from '../HIR/visitors';
21
+import {Result} from '../Utils/Result';
22
+import {Iterable_some} from '../Utils/utils';
23
+
24
+/**
25
+ * Validates that functions with known mutations (ie due to types) cannot be passed
26
+ * where a frozen value is expected. Example:
27
+ *
28
+ * ```
29
+ * function Component() {
30
+ * const cache = new Map();
31
+ * const onClick = () => {
32
+ * cache.set(...);
33
+ * }
34
+ * useHook(onClick); // ERROR: cannot pass a mutable value
35
+ * return <Foo onClick={onClick} /> // ERROR: cannot pass a mutable value
36
+ * }
37
+ * ```
38
+ *
39
+ * Because `onClick` function mutates `cache` when called, `onClick` is equivalent to a mutable
40
+ * variables. But unlike other mutables values like an array, the receiver of the function has
41
+ * no way to avoid mutation — for example, a function can receive an array and choose not to mutate
42
+ * it, but there's no way to know that a function is mutable and avoid calling it.
43
+ *
44
+ * This pass detects functions with *known* mutations (Store or Mutate, not ConditionallyMutate)
45
+ * that are passed where a frozen value is expected and rejects them.
46
+ */
47
+export function validateNoFreezingKnownMutableFunctions(
48
+ fn: HIRFunction,
49
+): Result<void, CompilerError> {
50
+ const errors = new CompilerError();
51
+ const contextMutationEffects: Map<
52
+ IdentifierId,
53
+ Extract<FunctionEffect, {kind: 'ContextMutation'}>
54
+ > = new Map();
55
+
56
+ function visitOperand(operand: Place): void {
57
+ if (operand.effect === Effect.Freeze) {
58
+ const effect = contextMutationEffects.get(operand.identifier.id);
59
+ if (effect != null) {
60
+ errors.push({
61
+ reason: `This argument is a function which modifies local variables when called, which can bypass memoization and cause the UI not to update`,
62
+ description: `Functions that are returned from hooks, passed as arguments to hooks, or passed as props to components may not mutate local variables`,
63
+ loc: operand.loc,
64
+ severity: ErrorSeverity.InvalidReact,
65
+ });
66
+ errors.push({
67
+ reason: `The function modifies a local variable here`,
68
+ loc: effect.loc,
69
+ severity: ErrorSeverity.InvalidReact,
70
+ });
71
+ }
72
+ }
73
+ }
74
+
75
+ for (const block of fn.body.blocks.values()) {
76
+ for (const instr of block.instructions) {
77
+ const {lvalue, value} = instr;
78
+ switch (value.kind) {
79
+ case 'LoadLocal': {
80
+ const effect = contextMutationEffects.get(value.place.identifier.id);
81
+ if (effect != null) {
82
+ contextMutationEffects.set(lvalue.identifier.id, effect);
83
+ }
84
+ break;
85
+ }
86
+ case 'StoreLocal': {
87
+ const effect = contextMutationEffects.get(value.value.identifier.id);
88
+ if (effect != null) {
89
+ contextMutationEffects.set(lvalue.identifier.id, effect);
90
+ contextMutationEffects.set(
91
+ value.lvalue.place.identifier.id,
92
+ effect,
93
+ );
94
+ }
95
+ break;
96
+ }
97
+ case 'FunctionExpression': {
98
+ const knownMutation = (value.loweredFunc.func.effects ?? []).find(
99
+ effect => {
100
+ return (
101
+ effect.kind === 'ContextMutation' &&
102
+ (effect.effect === Effect.Store ||
103
+ effect.effect === Effect.Mutate) &&
104
+ Iterable_some(effect.places, place => {
105
+ return (
106
+ isMutableEffect(place.effect, place.loc) &&
107
+ !isRefOrRefLikeMutableType(place.identifier.type)
108
+ );
109
+ })
110
+ );
111
+ },
112
+ );
113
+ if (knownMutation && knownMutation.kind === 'ContextMutation') {
114
+ contextMutationEffects.set(lvalue.identifier.id, knownMutation);
115
+ }
116
+ break;
117
+ }
118
+ default: {
119
+ for (const operand of eachInstructionValueOperand(value)) {
120
+ visitOperand(operand);
121
+ }
122
+ }
123
+ }
124
+ }
125
+ for (const operand of eachTerminalOperand(block.terminal)) {
126
+ visitOperand(operand);
127
+ }
128
+ }
129
+ return errors.asResult();
130
+}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hook-function-argument-mutates-local-variable.expect.md
new
+34
@@ -0,0 +1,34 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validateNoFreezingKnownMutableFunctions
6
+
7
+function useFoo() {
8
+ const cache = new Map();
9
+ useHook(() => {
10
+ cache.set('key', 'value');
11
+ });
12
+}
13
+
14
+```
15
+
16
+
17
+## Error
18
+
19
+```
20
+ 3 | function useFoo() {
21
+ 4 | const cache = new Map();
22
+> 5 | useHook(() => {
23
+ | ^^^^^^^
24
+> 6 | cache.set('key', 'value');
25
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
26
+> 7 | });
27
+ | ^^^^ 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 (5:7)
28
+
29
+InvalidReact: The function modifies a local variable here (6:6)
30
+ 8 | }
31
+ 9 |
32
+```
33
+
34
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hook-function-argument-mutates-local-variable.js
new
+8
@@ -0,0 +1,8 @@
1
+// @validateNoFreezingKnownMutableFunctions
2
+
3
+function useFoo() {
4
+ const cache = new Map();
5
+ useHook(() => {
6
+ cache.set('key', 'value');
7
+ });
8
+}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-pass-mutable-function-as-prop.expect.md
new
+30
@@ -0,0 +1,30 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validateNoFreezingKnownMutableFunctions
6
+function Component() {
7
+ const cache = new Map();
8
+ const fn = () => {
9
+ cache.set('key', 'value');
10
+ };
11
+ return <Foo fn={fn} />;
12
+}
13
+
14
+```
15
+
16
+
17
+## Error
18
+
19
+```
20
+ 5 | cache.set('key', 'value');
21
+ 6 | };
22
+> 7 | return <Foo fn={fn} />;
23
+ | ^^ 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 (7:7)
24
+
25
+InvalidReact: The function modifies a local variable here (5:5)
26
+ 8 | }
27
+ 9 |
28
+```
29
+
30
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-pass-mutable-function-as-prop.js
new
+8
@@ -0,0 +1,8 @@
1
+// @validateNoFreezingKnownMutableFunctions
2
+function Component() {
3
+ const cache = new Map();
4
+ const fn = () => {
5
+ cache.set('key', 'value');
6
+ };
7
+ return <Foo fn={fn} />;
8
+}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-return-mutable-function-from-hook.expect.md
new
+36
@@ -0,0 +1,36 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validateNoFreezingKnownMutableFunctions
6
+import {useHook} from 'shared-runtime';
7
+
8
+function useFoo() {
9
+ useHook(); // for inference to kick in
10
+ const cache = new Map();
11
+ return () => {
12
+ cache.set('key', 'value');
13
+ };
14
+}
15
+
16
+```
17
+
18
+
19
+## Error
20
+
21
+```
22
+ 5 | useHook(); // for inference to kick in
23
+ 6 | const cache = new Map();
24
+> 7 | return () => {
25
+ | ^^^^^^^
26
+> 8 | cache.set('key', 'value');
27
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
28
+> 9 | };
29
+ | ^^^^ 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 (7:9)
30
+
31
+InvalidReact: The function modifies a local variable here (8:8)
32
+ 10 | }
33
+ 11 |
34
+```
35
+
36
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-return-mutable-function-from-hook.js
new
+10
@@ -0,0 +1,10 @@
1
+// @validateNoFreezingKnownMutableFunctions
2
+import {useHook} from 'shared-runtime';
3
+
4
+function useFoo() {
5
+ useHook(); // for inference to kick in
6
+ const cache = new Map();
7
+ return () => {
8
+ cache.set('key', 'value');
9
+ };
10
+}