useContext returns frozen values
This was an oversight in the original definition of useContext (oops my bad). Context values are owned by React and should not be modified. I found this because some cases of existing useMemo were not preserved (tested via the validatePreserveExistingManualMemo flag) due to function calls referencing context being assumed to mutate. This change will allow more memoization, it's also just more correct for the rules of React. Note the new ValueReason variant so that we can provide a precise error message about mutating context values.
Joe Savona committed
Jan 10, 2024 at 16:12 UTC
c80f0f022c135704e955d36516bf81d071f40ba6
10 files changed
+75
-44
compiler/packages/babel-plugin-react-forget/src/HIR/Globals.ts
+3
-2
@@ -5,7 +5,7 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
-import { Effect, ValueKind } from "./HIR";
8
+import { Effect, ValueKind, ValueReason } from "./HIR";
9
import {
10
BUILTIN_SHAPES,
11
BuiltInArrayId,
@@ -248,7 +248,8 @@ const BUILTIN_HOOKS: Array<[string, BuiltInType]> = [
248
returnType: { kind: "Poly" },
249
calleeEffect: Effect.Read,
250
hookKind: "useContext",
251
- returnValueKind: ValueKind.Mutable,
251
+ returnValueKind: ValueKind.Frozen,
252
+ returnValueReason: ValueReason.Context,
253
}),
254
],
255
[
compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts
+5
@@ -977,6 +977,11 @@ export enum ValueReason {
977
*/
978
KnownReturnSignature = "known-return-signature",
979
980
+ /**
981
+ * A value returned from `useContext`
982
+ */
983
+ Context = "context",
984
+
985
Other = "other",
986
}
987
compiler/packages/babel-plugin-react-forget/src/HIR/ObjectShape.ts
+8
-1
@@ -6,7 +6,7 @@
6
*/
7
8
import { CompilerError } from "../CompilerError";
9
-import { Effect, ValueKind } from "./HIR";
9
+import { Effect, ValueKind, ValueReason } from "./HIR";
10
import {
11
BuiltInType,
12
FunctionType,
@@ -139,6 +139,13 @@ export type FunctionSignature = {
139
restParam: Effect | null;
140
returnType: BuiltInType | PolyType;
141
returnValueKind: ValueKind;
142
+
143
+ /**
144
+ * For functions that return frozen/immutable values, the reason provides a more
145
+ * precise error message for any (invalid) mutations of the value.
146
+ */
147
+ returnValueReason?: ValueReason;
148
+
149
calleeEffect: Effect;
150
hookKind: HookKind | null;
151
/*
compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts
+6
-1
@@ -935,7 +935,10 @@ function inferBlock(
935
signature !== null
936
? {
937
kind: signature.returnValueKind,
938
- reason: new Set([ValueReason.KnownReturnSignature]),
938
+ reason: new Set([
939
+ signature.returnValueReason ??
940
+ ValueReason.KnownReturnSignature,
941
+ ]),
942
}
943
: { kind: ValueKind.Mutable, reason: new Set([ValueReason.Other]) };
944
let hasCaptureArgument = false;
@@ -1496,6 +1499,8 @@ function getWriteErrorReason(abstractValue: AbstractValue): string {
1499
return "Writing to a variable defined outside a component or hook is not allowed. Consider using an effect.";
1500
} else if (abstractValue.reason.has(ValueReason.JsxCaptured)) {
1501
return "Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX.";
1502
+ } else if (abstractValue.reason.has(ValueReason.Context)) {
1503
+ return `Mutating a value returned from 'useContext()', which should not be mutated.`;
1504
} else if (abstractValue.reason.has(ValueReason.KnownReturnSignature)) {
1505
return "Mutating a value returned from a function that should not be mutated.";
1506
} else {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-context-in-callback.expect.md
renamed
+4
-20
@@ -20,27 +20,11 @@ function Component(props) {
20
21
```
22
23
-## Code
23
25
-```javascript
26
-import { unstable_useMemoCache as useMemoCache } from "react";
27
-function Component(props) {
28
- const $ = useMemoCache(2);
29
- const FooContext = useContext(Foo);
30
-
31
- const onClick = () => {
32
- FooContext.current = true;
33
- };
34
- let t0;
35
- if ($[0] !== onClick) {
36
- t0 = <div onClick={onClick} />;
37
- $[0] = onClick;
38
- $[1] = t0;
39
- } else {
40
- t0 = $[1];
41
- }
42
- return t0;
43
-}
24
+## Error
25
26
```
27
+[ReactForget] InvalidReact: Mutating a value returned from 'useContext()', which should not be mutated. (12:12)
28
+```
29
+
30
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-context-in-callback.js
renamed
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-context.expect.md
new
+20
@@ -0,0 +1,20 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const context = useContext(FooContext);
7
+ context.value = props.value;
8
+ return context.value;
9
+}
10
+
11
+```
12
+
13
+
14
+## Error
15
+
16
+```
17
+[ReactForget] InvalidReact: Mutating a value returned from 'useContext()', which should not be mutated. (3:3)
18
+```
19
+
20
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-context.js
new
+5
@@ -0,0 +1,5 @@
1
+function Component(props) {
2
+ const context = useContext(FooContext);
3
+ context.value = props.value;
4
+ return context.value;
5
+}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/react-namespace.expect.md
+24
-19
@@ -11,7 +11,6 @@ function Component(props) {
11
const onClick = () => {
12
setX(true);
13
ref.current = true;
14
- foo.current = true;
14
};
15
return <div onClick={onClick}>{React.cloneElement(props.children)}</div>;
16
}
@@ -30,33 +29,39 @@ import { unstable_useMemoCache as useMemoCache } from "react";
29
const FooContext = React.createContext({ current: null });
30
31
function Component(props) {
33
- const $ = useMemoCache(5);
34
- const foo = React.useContext(FooContext);
32
+ const $ = useMemoCache(6);
33
+ React.useContext(FooContext);
34
const ref = React.useRef();
35
const [x, setX] = React.useState(false);
37
- const onClick = () => {
38
- setX(true);
39
- ref.current = true;
40
- foo.current = true;
41
- };
36
let t0;
43
- if ($[0] !== props.children) {
44
- t0 = React.cloneElement(props.children);
45
- $[0] = props.children;
46
- $[1] = t0;
37
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
38
+ t0 = () => {
39
+ setX(true);
40
+ ref.current = true;
41
+ };
42
+ $[0] = t0;
43
} else {
48
- t0 = $[1];
44
+ t0 = $[0];
45
}
46
+ const onClick = t0;
47
let t1;
51
- if ($[2] !== onClick || $[3] !== t0) {
52
- t1 = <div onClick={onClick}>{t0}</div>;
53
- $[2] = onClick;
54
- $[3] = t0;
48
+ if ($[1] !== props.children) {
49
+ t1 = React.cloneElement(props.children);
50
+ $[1] = props.children;
51
+ $[2] = t1;
52
+ } else {
53
+ t1 = $[2];
54
+ }
55
+ let t2;
56
+ if ($[3] !== onClick || $[4] !== t1) {
57
+ t2 = <div onClick={onClick}>{t1}</div>;
58
+ $[3] = onClick;
59
$[4] = t1;
60
+ $[5] = t2;
61
} else {
57
- t1 = $[4];
62
+ t2 = $[5];
63
}
59
- return t1;
64
+ return t2;
65
}
66
67
export const FIXTURE_ENTRYPOINT = {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/react-namespace.js
-1
@@ -7,7 +7,6 @@ function Component(props) {
7
const onClick = () => {
8
setX(true);
9
ref.current = true;
10
- foo.current = true;
10
};
11
return <div onClick={onClick}>{React.cloneElement(props.children)}</div>;
12
}