InferReferenceEffects propagates context effects with precise locations
ghstack-source-id: f9517b98013613a3a80a78b4a6110006ee8b4de7 Pull Request resolved: https://github.com/facebook/react-forget/pull/2880
Joe Savona committed
Apr 19, 2024 at 19:03 UTC
c98c02f8e69145168f5747a0e2f5e6c03906c1ca
10 files changed
+171
-18
compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts
+1
-1
@@ -291,7 +291,7 @@ export type FunctionEffect =
291
}
292
| {
293
kind: "ContextMutation";
294
- places: Array<Place>;
294
+ places: ReadonlySet<Place>;
295
effect: Effect;
296
loc: SourceLocation;
297
};
compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts
+13
-1
@@ -521,7 +521,19 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
521
const context = instrValue.loweredFunc.func.context
522
.map((dep) => printPlace(dep))
523
.join(",");
524
- value = `${kind} ${name} @deps[${deps}] @context[${context}]:\n${fn}`;
524
+ const effects =
525
+ instrValue.loweredFunc.func.effects
526
+ ?.map((effect) => {
527
+ if (effect.kind === "ContextMutation") {
528
+ return `ContextMutation places=[${[...effect.places]
529
+ .map((place) => printPlace(place))
530
+ .join(", ")}] effect=${effect.effect}`;
531
+ } else {
532
+ return `GlobalMutation`;
533
+ }
534
+ })
535
+ .join(", ") ?? "";
536
+ value = `${kind} ${name} @deps[${deps}] @context[${context}] @effects[${effects}]:\n${fn}`;
537
break;
538
}
539
case "TaggedTemplateExpression": {
compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts
+39
-10
@@ -401,7 +401,22 @@ class InferenceState {
401
value.kind === "ObjectMethod") &&
402
value.loweredFunc.func.effects != null
403
) {
404
- functionEffects.push(...value.loweredFunc.func.effects);
404
+ for (const effect of value.loweredFunc.func.effects) {
405
+ if (effect.kind === "GlobalMutation") {
406
+ functionEffects.push(effect);
407
+ } else {
408
+ for (const place of effect.places) {
409
+ if (this.isDefined(place)) {
410
+ this.reference(
411
+ { ...place, loc: effect.loc },
412
+ functionEffects,
413
+ effect.effect,
414
+ reason
415
+ );
416
+ }
417
+ }
418
+ }
419
+ }
420
}
421
}
422
let valueKind: AbstractValue | null = this.kind(place);
@@ -460,10 +475,17 @@ class InferenceState {
475
break;
476
}
477
case Effect.Mutate: {
463
- if (
464
- valueKind.kind !== ValueKind.Mutable &&
465
- valueKind.kind !== ValueKind.Context
466
- ) {
478
+ if (valueKind.kind === ValueKind.Context) {
479
+ functionEffects.push({
480
+ kind: "ContextMutation",
481
+ loc: place.loc,
482
+ effect: effectKind,
483
+ places:
484
+ valueKind.context.size === 0
485
+ ? new Set([place])
486
+ : valueKind.context,
487
+ });
488
+ } else if (valueKind.kind !== ValueKind.Mutable) {
489
let reason = getWriteErrorReason(valueKind);
490
functionEffects.push({
491
kind: "GlobalMutation",
@@ -483,10 +505,17 @@ class InferenceState {
505
break;
506
}
507
case Effect.Store: {
486
- if (
487
- valueKind.kind !== ValueKind.Mutable &&
488
- valueKind.kind !== ValueKind.Context
489
- ) {
508
+ if (valueKind.kind === ValueKind.Context) {
509
+ functionEffects.push({
510
+ kind: "ContextMutation",
511
+ loc: place.loc,
512
+ effect: effectKind,
513
+ places:
514
+ valueKind.context.size === 0
515
+ ? new Set([place])
516
+ : valueKind.context,
517
+ });
518
+ } else if (valueKind.kind !== ValueKind.Mutable) {
519
let reason = getWriteErrorReason(valueKind);
520
functionEffects.push({
521
kind: "GlobalMutation",
@@ -1078,7 +1107,7 @@ function inferBlock(
1107
for (const operand of eachInstructionOperand(instr)) {
1108
state.reference(
1109
operand,
1081
- functionEffects,
1110
+ [],
1111
operand.effect === Effect.Unknown ? Effect.Read : operand.effect,
1112
ValueReason.Other
1113
);
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-function-expression-mutates-immutable-value.expect.md
+1
-1
@@ -21,7 +21,7 @@ function Component(props) {
21
3 | const onChange = (e) => {
22
4 | // INVALID! should use copy-on-write and pass the new value
23
> 5 | x.value = e.target.value;
24
- | ^^^^^^^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead. (5:5)
24
+ | ^ InvalidReact: Mutating a value returned from 'useState()', which should not be mutated. Use the setter function to update instead.. Found mutation of [object Object] (5:5)
25
6 | setX(x);
26
7 | };
27
8 | return <input value={x.value} onChange={onChange} />;
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-global-reassignment-indirect.expect.md
new
+47
@@ -0,0 +1,47 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { useEffect, useState } from "react";
6
+
7
+let someGlobal = false;
8
+
9
+function Component() {
10
+ const [state, setState] = useState(someGlobal);
11
+
12
+ const setGlobal = () => {
13
+ someGlobal = true;
14
+ };
15
+ const indirectSetGlobal = () => {
16
+ setGlobal();
17
+ };
18
+ indirectSetGlobal();
19
+
20
+ useEffect(() => {
21
+ setState(someGlobal);
22
+ }, [someGlobal]);
23
+
24
+ return <div>{String(state)}</div>;
25
+}
26
+
27
+export const FIXTURE_ENTRYPOINT = {
28
+ fn: Component,
29
+ params: [{}],
30
+};
31
+
32
+```
33
+
34
+
35
+## Error
36
+
37
+```
38
+ 7 |
39
+ 8 | const setGlobal = () => {
40
+> 9 | someGlobal = true;
41
+ | ^^^^^^^^^^ 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) (9:9)
42
+ 10 | };
43
+ 11 | const indirectSetGlobal = () => {
44
+ 12 | setGlobal();
45
+```
46
+
47
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-global-reassignment-indirect.js
new
+26
@@ -0,0 +1,26 @@
1
+import { useEffect, useState } from "react";
2
+
3
+let someGlobal = false;
4
+
5
+function Component() {
6
+ const [state, setState] = useState(someGlobal);
7
+
8
+ const setGlobal = () => {
9
+ someGlobal = true;
10
+ };
11
+ const indirectSetGlobal = () => {
12
+ setGlobal();
13
+ };
14
+ indirectSetGlobal();
15
+
16
+ useEffect(() => {
17
+ setState(someGlobal);
18
+ }, [someGlobal]);
19
+
20
+ return <div>{String(state)}</div>;
21
+}
22
+
23
+export const FIXTURE_ENTRYPOINT = {
24
+ fn: Component,
25
+ params: [{}],
26
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-context-in-callback.expect.md
+1
-1
@@ -27,7 +27,7 @@ function Component(props) {
27
10 | // independently
28
11 | const onClick = () => {
29
> 12 | FooContext.current = true;
30
- | ^^^^^^^^^^^^^^^^^^ InvalidReact: Mutating a value returned from 'useContext()', which should not be mutated. (12:12)
30
+ | ^^^^^^^^^^ InvalidReact: Mutating a value returned from 'useContext()', which should not be mutated.. Found mutation of [object Object] (12:12)
31
13 | };
32
14 | return <div onClick={onClick} />;
33
15 | }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutation-in-closure.expect.md
+4
-4
@@ -16,13 +16,13 @@ function useInvalidMutation(options) {
16
## Error
17
18
```
19
- 1 | function useInvalidMutation(options) {
19
2 | function test() {
21
-> 3 | foo(options.foo); // error should not point on this line
22
- | ^^^^^^^^^^^ InvalidReact: Mutating props or hook arguments is not allowed. Consider using a local variable instead. (3:3)
23
- 4 | options.foo = "bar";
20
+ 3 | foo(options.foo); // error should not point on this line
21
+> 4 | options.foo = "bar";
22
+ | ^^^^^^^ InvalidReact: Mutating props or hook arguments is not allowed. Consider using a local variable instead.. Found mutation of [object Object] (4:4)
23
5 | }
24
6 | return test;
25
+ 7 | }
26
```
27
28
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-prop-mutation-indirect.expect.md
new
+30
@@ -0,0 +1,30 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const f = () => {
7
+ props.value = true;
8
+ };
9
+ const g = () => {
10
+ f();
11
+ };
12
+ g();
13
+}
14
+
15
+```
16
+
17
+
18
+## Error
19
+
20
+```
21
+ 1 | function Component(props) {
22
+ 2 | const f = () => {
23
+> 3 | props.value = true;
24
+ | ^^^^^ InvalidReact: Mutating props or hook arguments is not allowed. Consider using a local variable instead.. Found mutation of [object Object] (3:3)
25
+ 4 | };
26
+ 5 | const g = () => {
27
+ 6 | f();
28
+```
29
+
30
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-prop-mutation-indirect.js
new
+9
@@ -0,0 +1,9 @@
1
+function Component(props) {
2
+ const f = () => {
3
+ props.value = true;
4
+ };
5
+ const g = () => {
6
+ f();
7
+ };
8
+ g();
9
+}