Propagate reactivity to other operands accounting for mutable ranges
Previously if any operand was reactive, we transferred that reactivity to other operands that had a mutable effect (capture, conditionally mutate, mutate, or store). But a value can be captured without ever being modified again. This PR updates the logic to only transfer reactivity among operands that are actually mutable at the given instruction, based on the mutable range. This is strictly more precise.
Joe Savona committed
Nov 1, 2023 at 17:13 UTC
3e157bbc27d5c8a4121cb99c3dd7d1af90becf03
7 files changed
+150
-15
compiler/packages/babel-plugin-react-forget/src/Inference/InferReactivePlaces.ts
+7
-4
@@ -23,6 +23,7 @@ import {
23
eachTerminalOperand,
24
} from "../HIR/visitors";
25
import { hasBackEdge } from "../Optimization/DeadCodeElimination";
26
+import { isMutable } from "../ReactiveScopes/InferReactiveScopeVariables";
27
import { assertExhaustive } from "../Utils/utils";
28
29
/**
@@ -195,11 +196,13 @@ export function inferReactivePlaces(fn: HIRFunction): void {
196
case Effect.Store:
197
case Effect.ConditionallyMutate:
198
case Effect.Mutate: {
198
- const resolvedId = identifierMapping.get(operand.identifier);
199
- if (resolvedId !== undefined) {
200
- reactiveIdentifiers.markReactiveIdentifier(resolvedId);
199
+ if (isMutable(instruction, operand)) {
200
+ const resolvedId = identifierMapping.get(operand.identifier);
201
+ if (resolvedId !== undefined) {
202
+ reactiveIdentifiers.markReactiveIdentifier(resolvedId);
203
+ }
204
+ reactiveIdentifiers.markReactive(operand);
205
}
202
- reactiveIdentifiers.markReactive(operand);
206
break;
207
}
208
case Effect.Freeze:
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts
+1
-1
@@ -212,7 +212,7 @@ export function inferReactiveScopeVariables(fn: HIRFunction): void {
212
}
213
214
// Is the operand mutable at this given instruction
215
-function isMutable({ id }: Instruction, place: Place): boolean {
215
+export function isMutable({ id }: Instruction, place: Place): boolean {
216
const range = place.identifier.mutableRange;
217
return id >= range.start && id < range.end;
218
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/phi-type-inference-property-store.expect.md
+8
-10
@@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = {
32
```javascript
33
import { unstable_useMemoCache as useMemoCache } from "react"; // @debug
34
function Component(props) {
35
- const $ = useMemoCache(7);
35
+ const $ = useMemoCache(5);
36
let t0;
37
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
38
t0 = {};
@@ -42,7 +42,7 @@ function Component(props) {
42
}
43
const x = t0;
44
let y;
45
- if ($[1] !== props || $[2] !== x) {
45
+ if ($[1] !== props) {
46
if (props.cond) {
47
y = {};
48
} else {
@@ -51,19 +51,17 @@ function Component(props) {
51
52
y.x = x;
53
$[1] = props;
54
- $[2] = x;
55
- $[3] = y;
54
+ $[2] = y;
55
} else {
57
- y = $[3];
56
+ y = $[2];
57
}
58
let t1;
60
- if ($[4] !== x || $[5] !== y) {
59
+ if ($[3] !== y) {
60
t1 = [x, y];
62
- $[4] = x;
63
- $[5] = y;
64
- $[6] = t1;
61
+ $[3] = y;
62
+ $[4] = t1;
63
} else {
66
- t1 = $[6];
64
+ t1 = $[4];
65
}
66
return t1;
67
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-dependency-nonreactive-captured-with-reactive.expect.md
new
+50
@@ -0,0 +1,50 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const x = {};
7
+ const y = props.y;
8
+ return [x, y]; // x is captured here along with a reactive value. this shouldn't make `x` reactive!
9
+}
10
+
11
+export const FIXTURE_ENTRYPOINT = {
12
+ fn: Component,
13
+ params: [{ y: 42 }],
14
+};
15
+
16
+```
17
+
18
+## Code
19
+
20
+```javascript
21
+import { unstable_useMemoCache as useMemoCache } from "react";
22
+function Component(props) {
23
+ const $ = useMemoCache(3);
24
+ let t0;
25
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
26
+ t0 = {};
27
+ $[0] = t0;
28
+ } else {
29
+ t0 = $[0];
30
+ }
31
+ const x = t0;
32
+ const y = props.y;
33
+ let t1;
34
+ if ($[1] !== y) {
35
+ t1 = [x, y];
36
+ $[1] = y;
37
+ $[2] = t1;
38
+ } else {
39
+ t1 = $[2];
40
+ }
41
+ return t1;
42
+}
43
+
44
+export const FIXTURE_ENTRYPOINT = {
45
+ fn: Component,
46
+ params: [{ y: 42 }],
47
+};
48
+
49
+```
50
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-dependency-nonreactive-captured-with-reactive.js
new
+10
@@ -0,0 +1,10 @@
1
+function Component(props) {
2
+ const x = {};
3
+ const y = props.y;
4
+ return [x, y]; // x is captured here along with a reactive value. this shouldn't make `x` reactive!
5
+}
6
+
7
+export const FIXTURE_ENTRYPOINT = {
8
+ fn: Component,
9
+ params: [{ y: 42 }],
10
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-dependency-object-captured-with-reactive-mutated.expect.md
new
+59
@@ -0,0 +1,59 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+const { mutate } = require("shared-runtime");
6
+
7
+function Component(props) {
8
+ const x = {};
9
+ const y = props.y;
10
+ const z = [x, y];
11
+ mutate(z);
12
+ // x's object identity can change bc it co-mutates with z, which is reactive via props.y
13
+ return [x];
14
+}
15
+
16
+export const FIXTURE_ENTRYPOINT = {
17
+ fn: Component,
18
+ params: [{ y: 42 }],
19
+};
20
+
21
+```
22
+
23
+## Code
24
+
25
+```javascript
26
+import { unstable_useMemoCache as useMemoCache } from "react";
27
+const { mutate } = require("shared-runtime");
28
+
29
+function Component(props) {
30
+ const $ = useMemoCache(4);
31
+ let x;
32
+ if ($[0] !== props.y) {
33
+ x = {};
34
+ const y = props.y;
35
+ const z = [x, y];
36
+ mutate(z);
37
+ $[0] = props.y;
38
+ $[1] = x;
39
+ } else {
40
+ x = $[1];
41
+ }
42
+ let t0;
43
+ if ($[2] !== x) {
44
+ t0 = [x];
45
+ $[2] = x;
46
+ $[3] = t0;
47
+ } else {
48
+ t0 = $[3];
49
+ }
50
+ return t0;
51
+}
52
+
53
+export const FIXTURE_ENTRYPOINT = {
54
+ fn: Component,
55
+ params: [{ y: 42 }],
56
+};
57
+
58
+```
59
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-dependency-object-captured-with-reactive-mutated.js
new
+15
@@ -0,0 +1,15 @@
1
+const { mutate } = require("shared-runtime");
2
+
3
+function Component(props) {
4
+ const x = {};
5
+ const y = props.y;
6
+ const z = [x, y];
7
+ mutate(z);
8
+ // x's object identity can change bc it co-mutates with z, which is reactive via props.y
9
+ return [x];
10
+}
11
+
12
+export const FIXTURE_ENTRYPOINT = {
13
+ fn: Component,
14
+ params: [{ y: 42 }],
15
+};