[compiler] Allow global mutation effects in arguments passed to hooks and in return values
ghstack-source-id: f9ea675ead6eb61b3afc2a3deace0da270612d9d Pull Request resolved: https://github.com/facebook/react/pull/30576
Mike Vitousek committed
Aug 2, 2024 at 12:24 UTC
47337a842ab3afd7344154a86dfb4797cfc26602
7 files changed
+182
-9
compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts
+15
-9
@@ -26,6 +26,7 @@ import {
26
Type,
27
ValueKind,
28
ValueReason,
29
+ getHookKind,
30
isArrayType,
31
isMutableEffect,
32
isObjectType,
@@ -48,7 +49,6 @@ import {
49
eachTerminalSuccessor,
50
} from '../HIR/visitors';
51
import {assertExhaustive} from '../Utils/utils';
51
-import {isEffectHook} from '../Validation/ValidateMemoizedEffectDependencies';
52
53
const UndefinedValue: InstructionValue = {
54
kind: 'Primitive',
@@ -1151,7 +1151,7 @@ function inferBlock(
1151
);
1152
functionEffects.push(
1153
...propEffects.filter(
1154
- propEffect => propEffect.kind !== 'GlobalMutation',
1154
+ effect => !isEffectSafeOutsideRender(effect),
1155
),
1156
);
1157
}
@@ -1330,7 +1330,7 @@ function inferBlock(
1330
context: new Set(),
1331
};
1332
let hasCaptureArgument = false;
1333
- let isUseEffect = isEffectHook(instrValue.callee.identifier);
1333
+ let isHook = getHookKind(env, instrValue.callee.identifier) != null;
1334
for (let i = 0; i < instrValue.args.length; i++) {
1335
const argumentEffects: Array<FunctionEffect> = [];
1336
const arg = instrValue.args[i];
@@ -1356,8 +1356,7 @@ function inferBlock(
1356
*/
1357
functionEffects.push(
1358
...argumentEffects.filter(
1359
- argEffect =>
1360
- !isUseEffect || i !== 0 || argEffect.kind !== 'GlobalMutation',
1359
+ argEffect => !isHook || !isEffectSafeOutsideRender(argEffect),
1360
),
1361
);
1362
hasCaptureArgument ||= place.effect === Effect.Capture;
@@ -1455,7 +1454,7 @@ function inferBlock(
1454
const effects =
1455
signature !== null ? getFunctionEffects(instrValue, signature) : null;
1456
let hasCaptureArgument = false;
1458
- let isUseEffect = isEffectHook(instrValue.property.identifier);
1457
+ let isHook = getHookKind(env, instrValue.property.identifier) != null;
1458
for (let i = 0; i < instrValue.args.length; i++) {
1459
const argumentEffects: Array<FunctionEffect> = [];
1460
const arg = instrValue.args[i];
@@ -1485,8 +1484,7 @@ function inferBlock(
1484
*/
1485
functionEffects.push(
1486
...argumentEffects.filter(
1488
- argEffect =>
1489
- !isUseEffect || i !== 0 || argEffect.kind !== 'GlobalMutation',
1487
+ argEffect => !isHook || !isEffectSafeOutsideRender(argEffect),
1488
),
1489
);
1490
hasCaptureArgument ||= place.effect === Effect.Capture;
@@ -2010,11 +2008,15 @@ function inferBlock(
2008
} else {
2009
effect = Effect.Read;
2010
}
2011
+ const propEffects: Array<FunctionEffect> = [];
2012
state.referenceAndRecordEffects(
2013
operand,
2014
effect,
2015
ValueReason.Other,
2017
- functionEffects,
2016
+ propEffects,
2017
+ );
2018
+ functionEffects.push(
2019
+ ...propEffects.filter(effect => !isEffectSafeOutsideRender(effect)),
2020
);
2021
}
2022
}
@@ -2128,6 +2130,10 @@ function areArgumentsImmutableAndNonMutating(
2130
return true;
2131
}
2132
2133
+function isEffectSafeOutsideRender(effect: FunctionEffect): boolean {
2134
+ return effect.kind === 'GlobalMutation';
2135
+}
2136
+
2137
function getWriteErrorReason(abstractValue: AbstractValue): string {
2138
if (abstractValue.reason.has(ValueReason.Global)) {
2139
return 'Writing to a variable defined outside a component or hook is not allowed. Consider using an effect';
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassign-global-fn-arg.expect.md
new
+36
@@ -0,0 +1,36 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+let b = 1;
6
+
7
+export default function MyApp() {
8
+ const fn = () => {
9
+ b = 2;
10
+ };
11
+ return foo(fn);
12
+}
13
+
14
+function foo(fn) {}
15
+
16
+export const FIXTURE_ENTRYPOINT = {
17
+ fn: MyApp,
18
+ params: [],
19
+};
20
+
21
+```
22
+
23
+
24
+## Error
25
+
26
+```
27
+ 3 | export default function MyApp() {
28
+ 4 | const fn = () => {
29
+> 5 | b = 2;
30
+ | ^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (5:5)
31
+ 6 | };
32
+ 7 | return foo(fn);
33
+ 8 | }
34
+```
35
+
36
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassign-global-fn-arg.js
new
+15
@@ -0,0 +1,15 @@
1
+let b = 1;
2
+
3
+export default function MyApp() {
4
+ const fn = () => {
5
+ b = 2;
6
+ };
7
+ return foo(fn);
8
+}
9
+
10
+function foo(fn) {}
11
+
12
+export const FIXTURE_ENTRYPOINT = {
13
+ fn: MyApp,
14
+ params: [],
15
+};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-hook-arg.expect.md
new
+46
@@ -0,0 +1,46 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+let b = 1;
6
+
7
+export default function MyApp() {
8
+ const fn = () => {
9
+ b = 2;
10
+ };
11
+ return useFoo(fn);
12
+}
13
+
14
+function useFoo(fn) {}
15
+
16
+export const FIXTURE_ENTRYPOINT = {
17
+ fn: MyApp,
18
+ params: [],
19
+};
20
+
21
+```
22
+
23
+## Code
24
+
25
+```javascript
26
+let b = 1;
27
+
28
+export default function MyApp() {
29
+ const fn = _temp;
30
+ return useFoo(fn);
31
+}
32
+function _temp() {
33
+ b = 2;
34
+}
35
+
36
+function useFoo(fn) {}
37
+
38
+export const FIXTURE_ENTRYPOINT = {
39
+ fn: MyApp,
40
+ params: [],
41
+};
42
+
43
+```
44
+
45
+### Eval output
46
+(kind: ok)
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-hook-arg.js
new
+15
@@ -0,0 +1,15 @@
1
+let b = 1;
2
+
3
+export default function MyApp() {
4
+ const fn = () => {
5
+ b = 2;
6
+ };
7
+ return useFoo(fn);
8
+}
9
+
10
+function useFoo(fn) {}
11
+
12
+export const FIXTURE_ENTRYPOINT = {
13
+ fn: MyApp,
14
+ params: [],
15
+};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-return.expect.md
new
+42
@@ -0,0 +1,42 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+let b = 1;
6
+
7
+export default function useMyHook() {
8
+ const fn = () => {
9
+ b = 2;
10
+ };
11
+ return fn;
12
+}
13
+
14
+export const FIXTURE_ENTRYPOINT = {
15
+ fn: useMyHook,
16
+ params: [],
17
+};
18
+
19
+```
20
+
21
+## Code
22
+
23
+```javascript
24
+let b = 1;
25
+
26
+export default function useMyHook() {
27
+ const fn = _temp;
28
+ return fn;
29
+}
30
+function _temp() {
31
+ b = 2;
32
+}
33
+
34
+export const FIXTURE_ENTRYPOINT = {
35
+ fn: useMyHook,
36
+ params: [],
37
+};
38
+
39
+```
40
+
41
+### Eval output
42
+(kind: ok) "[[ function params=0 ]]"
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-global-return.js
new
+13
@@ -0,0 +1,13 @@
1
+let b = 1;
2
+
3
+export default function useMyHook() {
4
+ const fn = () => {
5
+ b = 2;
6
+ };
7
+ return fn;
8
+}
9
+
10
+export const FIXTURE_ENTRYPOINT = {
11
+ fn: useMyHook,
12
+ params: [],
13
+};