@samitouri / QOS-React-2 / commits / f838ebb34e

Simpler fix for missed reactivity bug

Fixes the issue @mofeiZ identified, where mutations of an object via a propertyload were not tracking reactivity.

Joe Savona committed Mar 17, 2023 at 15:07 UTC f838ebb34e7915672b6f9b0c6ec39061ad2e4d54
7 files changed +228
compiler/forget/src/ReactiveScopes/InferReactiveIdentifiers.ts
+8
@@ -105,6 +105,14 @@ class Visitor extends ReactiveFunctionVisitor<State> {
105 instr.lvalue.identifier.id,
106 instr.value.place.identifier.id
107 );
108 + } else if (
109 + instr.value.kind === "PropertyLoad" ||
110 + instr.value.kind === "ComputedLoad"
111 + ) {
112 + const resolvedId =
113 + state.temporaries.get(instr.value.object.identifier.id) ??
114 + instr.value.object.identifier.id;
115 + state.temporaries.set(instr.lvalue.identifier.id, resolvedId);
116 }
117 }
118 }
compiler/forget/src/__tests__/fixtures/compiler/reactivity-analysis-interleaved-reactivity.expect.md new
+67
@@ -0,0 +1,67 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + // a and b are technically independent, but their mutation is interleaved
7 + // so they are grouped in a single reactive scope. a does not have any
8 + // reactive inputs, but b does. therefore, we have to treat a as reactive,
9 + // since it will be recreated based on a reactive input.
10 + const a = {};
11 + const b = [];
12 + b.push(props.b);
13 + a.a = null;
14 +
15 + // because a may recreate when b does, it becomes reactive. we have to recreate
16 + // c if a changes.
17 + const c = [a];
18 +
19 + // Example usage that could fail if we didn't treat a as reactive:
20 + // const [c, a] = Component({b: ...});
21 + // assert(c[0] === a);
22 + return [c, a];
23 +}
24 +
25 +```
26 +
27 +## Code
28 +
29 +```javascript
30 +function Component(props) {
31 + const $ = React.unstable_useMemoCache(6);
32 + const c_0 = $[0] !== props.b;
33 + let a;
34 + if (c_0) {
35 + a = {};
36 + const b = [];
37 + b.push(props.b);
38 + a.a = null;
39 + $[0] = props.b;
40 + $[1] = a;
41 + } else {
42 + a = $[1];
43 + }
44 + const c_2 = $[2] !== a;
45 + let t0;
46 + if (c_2) {
47 + t0 = [a];
48 + $[2] = a;
49 + $[3] = t0;
50 + } else {
51 + t0 = $[3];
52 + }
53 + const c = t0;
54 + const c_4 = $[4] !== a;
55 + let t1;
56 + if (c_4) {
57 + t1 = [c, a];
58 + $[4] = a;
59 + $[5] = t1;
60 + } else {
61 + t1 = $[5];
62 + }
63 + return t1;
64 +}
65 +
66 +```
67 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/reactivity-analysis-interleaved-reactivity.js new
+19
@@ -0,0 +1,19 @@
1 +function Component(props) {
2 + // a and b are technically independent, but their mutation is interleaved
3 + // so they are grouped in a single reactive scope. a does not have any
4 + // reactive inputs, but b does. therefore, we have to treat a as reactive,
5 + // since it will be recreated based on a reactive input.
6 + const a = {};
7 + const b = [];
8 + b.push(props.b);
9 + a.a = null;
10 +
11 + // because a may recreate when b does, it becomes reactive. we have to recreate
12 + // c if a changes.
13 + const c = [a];
14 +
15 + // Example usage that could fail if we didn't treat a as reactive:
16 + // const [c, a] = Component({b: ...});
17 + // assert(c[0] === a);
18 + return [c, a];
19 +}
compiler/forget/src/__tests__/fixtures/compiler/reactivity-analysis-reactive-via-mutation-of-computed-load.expect.md new
+60
@@ -0,0 +1,60 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + const items = bar();
7 + mutate(items[props.key], props.a);
8 +
9 + const count = foo(items.length + 1);
10 +
11 + return { items, count };
12 +}
13 +
14 +```
15 +
16 +## Code
17 +
18 +```javascript
19 +function Component(props) {
20 + const $ = React.unstable_useMemoCache(8);
21 + const c_0 = $[0] !== props.key;
22 + const c_1 = $[1] !== props.a;
23 + let items;
24 + if (c_0 || c_1) {
25 + items = bar();
26 + mutate(items[props.key], props.a);
27 + $[0] = props.key;
28 + $[1] = props.a;
29 + $[2] = items;
30 + } else {
31 + items = $[2];
32 + }
33 +
34 + const t0 = items.length + 1;
35 + const c_3 = $[3] !== t0;
36 + let t1;
37 + if (c_3) {
38 + t1 = foo(t0);
39 + $[3] = t0;
40 + $[4] = t1;
41 + } else {
42 + t1 = $[4];
43 + }
44 + const count = t1;
45 + const c_5 = $[5] !== items;
46 + const c_6 = $[6] !== count;
47 + let t2;
48 + if (c_5 || c_6) {
49 + t2 = { items, count };
50 + $[5] = items;
51 + $[6] = count;
52 + $[7] = t2;
53 + } else {
54 + t2 = $[7];
55 + }
56 + return t2;
57 +}
58 +
59 +```
60 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/reactivity-analysis-reactive-via-mutation-of-computed-load.js new
+8
@@ -0,0 +1,8 @@
1 +function Component(props) {
2 + const items = bar();
3 + mutate(items[props.key], props.a);
4 +
5 + const count = foo(items.length + 1);
6 +
7 + return { items, count };
8 +}
compiler/forget/src/__tests__/fixtures/compiler/reactivity-analysis-reactive-via-mutation-of-property-load.expect.md new
+58
@@ -0,0 +1,58 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + const items = bar();
7 + mutate(items.a, props.a);
8 +
9 + const count = foo(items.length + 1);
10 +
11 + return { items, count };
12 +}
13 +
14 +```
15 +
16 +## Code
17 +
18 +```javascript
19 +function Component(props) {
20 + const $ = React.unstable_useMemoCache(7);
21 + const c_0 = $[0] !== props.a;
22 + let items;
23 + if (c_0) {
24 + items = bar();
25 + mutate(items.a, props.a);
26 + $[0] = props.a;
27 + $[1] = items;
28 + } else {
29 + items = $[1];
30 + }
31 +
32 + const t0 = items.length + 1;
33 + const c_2 = $[2] !== t0;
34 + let t1;
35 + if (c_2) {
36 + t1 = foo(t0);
37 + $[2] = t0;
38 + $[3] = t1;
39 + } else {
40 + t1 = $[3];
41 + }
42 + const count = t1;
43 + const c_4 = $[4] !== items;
44 + const c_5 = $[5] !== count;
45 + let t2;
46 + if (c_4 || c_5) {
47 + t2 = { items, count };
48 + $[4] = items;
49 + $[5] = count;
50 + $[6] = t2;
51 + } else {
52 + t2 = $[6];
53 + }
54 + return t2;
55 +}
56 +
57 +```
58 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/reactivity-analysis-reactive-via-mutation-of-property-load.js new
+8
@@ -0,0 +1,8 @@
1 +function Component(props) {
2 + const items = bar();
3 + mutate(items.a, props.a);
4 +
5 + const count = foo(items.length + 1);
6 +
7 + return { items, count };
8 +}