@samitouri / QOS-React-1 / commits / 4d84bee172

Propagate reactive scope dependencies transitively

During PruneNonReactiveDependencies, we sometimes need to promote a value from non-reactive to reactive if it ended up being grouped in the same reactive scope as some other reactive value. This generally happens due to interleaving mutations. In this case all downstream usage of the promoted value need to also be considered reactive. Fully propagating the reactivity requires re-running InferReactivePlaces, to account for things like control reactivity. We can't yet reuse that pass here though, because we haven't unified the pipeline on HIR yet. For now, we propagate the reactivity through local variables and downstream reactive scopes. See test fixtures for some examples that now correctly propagate reactivity and some that need the full reactivity inference to run correctly. The latter cases are handled in the next PR.

Joe Savona committed Jan 22, 2024 at 15:35 UTC 4d84bee1723ad024058dda7e9a47774146912c77
4 files changed +197 -18
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts
+83 -13
@@ -5,7 +5,14 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 -import { IdentifierId, ReactiveFunction, ReactiveScopeBlock } from "../HIR";
8 +import {
9 + IdentifierId,
10 + ReactiveFunction,
11 + ReactiveInstruction,
12 + ReactiveScopeBlock,
13 + isSetStateType,
14 +} from "../HIR";
15 +import { eachPatternOperand } from "../HIR/visitors";
16 import { collectReactiveIdentifiers } from "./CollectReactiveIdentifiers";
17 import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
18
@@ -23,27 +30,90 @@ export function pruneNonReactiveDependencies(fn: ReactiveFunction): void {
30 type ReactiveIdentifiers = Set<IdentifierId>;
31
32 class Visitor extends ReactiveFunctionVisitor<ReactiveIdentifiers> {
33 + override visitInstruction(
34 + instruction: ReactiveInstruction,
35 + state: ReactiveIdentifiers
36 + ): void {
37 + this.traverseInstruction(instruction, state);
38 +
39 + const { lvalue, value } = instruction;
40 + switch (value.kind) {
41 + case "LoadLocal": {
42 + if (lvalue !== null && state.has(value.place.identifier.id)) {
43 + state.add(lvalue.identifier.id);
44 + }
45 + break;
46 + }
47 + case "StoreLocal": {
48 + if (state.has(value.value.identifier.id)) {
49 + state.add(value.lvalue.place.identifier.id);
50 + if (lvalue !== null) {
51 + state.add(lvalue.identifier.id);
52 + }
53 + }
54 + break;
55 + }
56 + case "Destructure": {
57 + if (state.has(value.value.identifier.id)) {
58 + for (const lvalue of eachPatternOperand(value.lvalue.pattern)) {
59 + if (isSetStateType(lvalue.identifier)) {
60 + continue;
61 + }
62 + state.add(lvalue.identifier.id);
63 + }
64 + if (lvalue !== null) {
65 + state.add(lvalue.identifier.id);
66 + }
67 + }
68 + break;
69 + }
70 + case "PropertyLoad": {
71 + if (
72 + lvalue !== null &&
73 + state.has(value.object.identifier.id) &&
74 + !isSetStateType(lvalue.identifier)
75 + ) {
76 + state.add(lvalue.identifier.id);
77 + }
78 + break;
79 + }
80 + case "ComputedLoad": {
81 + if (
82 + lvalue !== null &&
83 + (state.has(value.object.identifier.id) ||
84 + state.has(value.property.identifier.id))
85 + ) {
86 + state.add(lvalue.identifier.id);
87 + }
88 + break;
89 + }
90 + }
91 + }
92 +
93 override visitScope(
27 - scope: ReactiveScopeBlock,
94 + scopeBlock: ReactiveScopeBlock,
95 state: ReactiveIdentifiers
96 ): void {
30 - this.traverseScope(scope, state);
31 - for (const dep of scope.scope.dependencies) {
97 + this.traverseScope(scopeBlock, state);
98 + for (const dep of scopeBlock.scope.dependencies) {
99 const isReactive = state.has(dep.identifier.id);
100 if (!isReactive) {
34 - scope.scope.dependencies.delete(dep);
101 + scopeBlock.scope.dependencies.delete(dep);
102 }
103 }
37 - if (scope.scope.dependencies.size === 0) {
38 - // If a scope has no dependencies, then its declarations are all non-reactive
39 - for (const [, declaration] of scope.scope.declarations) {
40 - state.delete(declaration.identifier.id);
41 - }
42 - } else {
43 - // otherwise, all the scope's declarations are reactive
44 - for (const [, declaration] of scope.scope.declarations) {
104 + if (scopeBlock.scope.dependencies.size !== 0) {
105 + /**
106 + * If any of a scope's dependencies are reactive, then all of its
107 + * outputs will re-evaluate whenever those dependencies change.
108 + * Mark all of the outputs as reactive to reflect the fact that
109 + * they may change in practice based on a reactive input.
110 + */
111 + for (const [, declaration] of scopeBlock.scope.declarations) {
112 state.add(declaration.identifier.id);
113 }
114 + for (const reassignment of scopeBlock.scope.reassignments) {
115 + state.add(reassignment.id);
116 + }
117 }
118 }
119 }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-if.expect.md new
+77
@@ -0,0 +1,77 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + // a and b are independent but their mutations are interleaved, so
7 + // they get grouped in a reactive scope. this means that a becomes
8 + // reactive since it will effectively re-evaluate based on a reactive
9 + // input
10 + const a = [];
11 + const b = [];
12 + b.push(props.cond);
13 + a.push(null);
14 +
15 + // Downstream consumer of a, which initially seems non-reactive except
16 + // that a becomes reactive, per above
17 + const c = [a];
18 +
19 + let x;
20 + if (c[0]) {
21 + x = 1;
22 + } else {
23 + x = 2;
24 + }
25 + // The values assigned to `x` are non-reactive, but the value of `x`
26 + // depends on the "control" value `c[0]` which becomes reactive via
27 + // being interleaved with `b`.
28 + // Therefore x should be treated as reactive too.
29 + return [x];
30 +}
31 +
32 +export const FIXTURE_ENTRYPOINT = {
33 + fn: Component,
34 + params: [{ cond: true }],
35 +};
36 +
37 +```
38 +
39 +## Code
40 +
41 +```javascript
42 +import { unstable_useMemoCache as useMemoCache } from "react";
43 +function Component(props) {
44 + const $ = useMemoCache(1);
45 +
46 + const a = [];
47 + const b = [];
48 + b.push(props.cond);
49 + a.push(null);
50 +
51 + const c = [a];
52 +
53 + let x;
54 + if (c[0]) {
55 + x = 1;
56 + } else {
57 + x = 2;
58 + }
59 + let t0;
60 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
61 + t0 = [x];
62 + $[0] = t0;
63 + } else {
64 + t0 = $[0];
65 + }
66 + return t0;
67 +}
68 +
69 +export const FIXTURE_ENTRYPOINT = {
70 + fn: Component,
71 + params: [{ cond: true }],
72 +};
73 +
74 +```
75 +
76 +### Eval output
77 +(kind: ok) [1]
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-if.js new
+31
@@ -0,0 +1,31 @@
1 +function Component(props) {
2 + // a and b are independent but their mutations are interleaved, so
3 + // they get grouped in a reactive scope. this means that a becomes
4 + // reactive since it will effectively re-evaluate based on a reactive
5 + // input
6 + const a = [];
7 + const b = [];
8 + b.push(props.cond);
9 + a.push(null);
10 +
11 + // Downstream consumer of a, which initially seems non-reactive except
12 + // that a becomes reactive, per above
13 + const c = [a];
14 +
15 + let x;
16 + if (c[0]) {
17 + x = 1;
18 + } else {
19 + x = 2;
20 + }
21 + // The values assigned to `x` are non-reactive, but the value of `x`
22 + // depends on the "control" value `c[0]` which becomes reactive via
23 + // being interleaved with `b`.
24 + // Therefore x should be treated as reactive too.
25 + return [x];
26 +}
27 +
28 +export const FIXTURE_ENTRYPOINT = {
29 + fn: Component,
30 + params: [{ cond: true }],
31 +};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactivity-analysis-interleaved-reactivity.expect.md
+6 -5
@@ -35,7 +35,7 @@ export const FIXTURE_ENTRYPOINT = {
35 ```javascript
36 import { unstable_useMemoCache as useMemoCache } from "react";
37 function Component(props) {
38 - const $ = useMemoCache(6);
38 + const $ = useMemoCache(7);
39 let a;
40 if ($[0] !== props.b) {
41 a = {};
@@ -57,12 +57,13 @@ function Component(props) {
57 }
58 const c = t0;
59 let t1;
60 - if ($[4] !== a) {
60 + if ($[4] !== c || $[5] !== a) {
61 t1 = [c, a];
62 - $[4] = a;
63 - $[5] = t1;
62 + $[4] = c;
63 + $[5] = a;
64 + $[6] = t1;
65 } else {
65 - t1 = $[5];
66 + t1 = $[6];
67 }
68 return t1;
69 }