@samitouri / QOS-React / commits / 0ae37f1568

Treat setState as non-reactive

Updates PruneNonReactiveDependencies to treat setState functions as non-reactive, since we know they have a stable identity. This is based on type inference and our recently added definitions for useState and its return type, so it's conservative and will only work when our inference can prove that the scope dependency has the SetState type. Note that this approach is simple and has limitations, notably the fact that the setState is non-reactive doesn't propagate. But it's simple, trivially correct, and already improves codegen somewhat, so i figured it's worth landing for now. ## Test Plan Tested on internal app

Joe Savona committed May 31, 2023 at 14:29 UTC 0ae37f156815d656d9b3835bab1900d889dc03ce
10 files changed +86 -83
compiler/forget/src/HIR/HIR.ts
+8
@@ -987,6 +987,14 @@ export function isUseRefType(id: Identifier): boolean {
987 return id.type.kind === "Object" && id.type.shapeId === "BuiltInUseRefId";
988 }
989
990 +export function isUseStateType(id: Identifier): boolean {
991 + return id.type.kind === "Object" && id.type.shapeId === "BuiltInUseState";
992 +}
993 +
994 +export function isSetStateType(id: Identifier): boolean {
995 + return id.type.kind === "Function" && id.type.shapeId === "BuiltInSetState";
996 +}
997 +
998 export function getHookKind(env: Environment, id: Identifier): HookKind | null {
999 const idType = id.type;
1000 if (idType.kind === "Function") {
compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts
+4 -1
@@ -18,6 +18,7 @@ import {
18 printIdentifier,
19 printInstructionValue,
20 printPlace,
21 + printType,
22 } from "../HIR/PrintHIR";
23 import { assertExhaustive } from "../Utils/utils";
24
@@ -59,7 +60,9 @@ export function printReactiveBlock(
60 }
61
62 function printDependency(dependency: ReactiveScopeDependency): string {
62 - const identifier = printIdentifier(dependency.identifier);
63 + const identifier =
64 + printIdentifier(dependency.identifier) +
65 + printType(dependency.identifier.type);
66 return `${identifier}${dependency.path.map((prop) => `.${prop}`).join("")}`;
67 }
68
compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts
+8 -2
@@ -5,7 +5,12 @@
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 + ReactiveScopeBlock,
12 + isSetStateType,
13 +} from "../HIR";
14 import { inferReactiveIdentifiers } from "./InferReactiveIdentifiers";
15 import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
16
@@ -26,7 +31,8 @@ class Visitor extends ReactiveFunctionVisitor<State> {
31 override visitScope(scope: ReactiveScopeBlock, state: State): void {
32 this.traverseScope(scope, state);
33 for (const dep of scope.scope.dependencies) {
29 - const isReactive = state.has(dep.identifier.id);
34 + const isReactive =
35 + state.has(dep.identifier.id) && !isSetStateType(dep.identifier);
36 if (!isReactive) {
37 scope.scope.dependencies.delete(dep);
38 }
compiler/forget/src/__tests__/fixtures/compiler/concise-arrow-expr.expect.md
+9 -11
@@ -15,26 +15,24 @@ function component() {
15 ```javascript
16 import { unstable_useMemoCache as useMemoCache } from "react";
17 function component() {
18 - const $ = useMemoCache(4);
18 + const $ = useMemoCache(3);
19 const [x, setX] = useState(0);
20 - const c_0 = $[0] !== setX;
20 let t0;
22 - if (c_0) {
21 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
22 t0 = (v) => setX(v);
24 - $[0] = setX;
25 - $[1] = t0;
23 + $[0] = t0;
24 } else {
27 - t0 = $[1];
25 + t0 = $[0];
26 }
27 const handler = t0;
30 - const c_2 = $[2] !== handler;
28 + const c_1 = $[1] !== handler;
29 let t1;
32 - if (c_2) {
30 + if (c_1) {
31 t1 = <Foo handler={handler} />;
34 - $[2] = handler;
35 - $[3] = t1;
32 + $[1] = handler;
33 + $[2] = t1;
34 } else {
37 - t1 = $[3];
35 + t1 = $[2];
36 }
37 return t1;
38 }
compiler/forget/src/__tests__/fixtures/compiler/controlled-input.expect.md
+11 -13
@@ -15,28 +15,26 @@ function component() {
15 ```javascript
16 import { unstable_useMemoCache as useMemoCache } from "react";
17 function component() {
18 - const $ = useMemoCache(5);
18 + const $ = useMemoCache(4);
19 const [x, setX] = useState(0);
20 - const c_0 = $[0] !== setX;
20 let t0;
22 - if (c_0) {
21 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
22 t0 = (event) => setX(event.target.value);
24 - $[0] = setX;
25 - $[1] = t0;
23 + $[0] = t0;
24 } else {
27 - t0 = $[1];
25 + t0 = $[0];
26 }
27 const handler = t0;
30 - const c_2 = $[2] !== handler;
31 - const c_3 = $[3] !== x;
28 + const c_1 = $[1] !== handler;
29 + const c_2 = $[2] !== x;
30 let t1;
33 - if (c_2 || c_3) {
31 + if (c_1 || c_2) {
32 t1 = <input onChange={handler} value={x} />;
35 - $[2] = handler;
36 - $[3] = x;
37 - $[4] = t1;
33 + $[1] = handler;
34 + $[2] = x;
35 + $[3] = t1;
36 } else {
39 - t1 = $[4];
37 + t1 = $[3];
38 }
39 return t1;
40 }
compiler/forget/src/__tests__/fixtures/compiler/disable-jsx-memoization.expect.md
+4 -6
@@ -22,18 +22,16 @@ function Component(props) {
22 ```javascript
23 import { unstable_useMemoCache as useMemoCache } from "react"; // @memoizeJsxElements false
24 function Component(props) {
25 - const $ = useMemoCache(2);
25 + const $ = useMemoCache(1);
26 const [name, setName] = useState(null);
27 - const c_0 = $[0] !== setName;
27 let t0;
29 - if (c_0) {
28 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
29 t0 = function (e) {
30 setName(e.target.value);
31 };
33 - $[0] = setName;
34 - $[1] = t0;
32 + $[0] = t0;
33 } else {
36 - t0 = $[1];
34 + t0 = $[0];
35 }
36 const onChange = t0;
37 return (
compiler/forget/src/__tests__/fixtures/compiler/inadvertent-mutability-readonly-lambda.expect.md
+9 -11
@@ -23,29 +23,27 @@ function Component(props) {
23 ```javascript
24 import { unstable_useMemoCache as useMemoCache } from "react";
25 function Component(props) {
26 - const $ = useMemoCache(4);
26 + const $ = useMemoCache(3);
27 const [value, setValue] = useState(null);
28 - const c_0 = $[0] !== setValue;
28 let t0;
30 - if (c_0) {
29 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
30 t0 = (e) => setValue((value_0) => value_0 + e.target.value);
32 - $[0] = setValue;
33 - $[1] = t0;
31 + $[0] = t0;
32 } else {
35 - t0 = $[1];
33 + t0 = $[0];
34 }
35 const onChange = t0;
36
37 useOtherHook();
40 - const c_2 = $[2] !== onChange;
38 + const c_1 = $[1] !== onChange;
39 let x;
42 - if (c_2) {
40 + if (c_1) {
41 x = {};
42 foo(x, onChange);
45 - $[2] = onChange;
46 - $[3] = x;
43 + $[1] = onChange;
44 + $[2] = x;
45 } else {
48 - x = $[3];
46 + x = $[2];
47 }
48 return x;
49 }
compiler/forget/src/__tests__/fixtures/compiler/invalid-freeze-mutable-lambda.expect.md
+11 -13
@@ -19,7 +19,7 @@ function Component(props) {
19 ```javascript
20 import { unstable_useMemoCache as useMemoCache } from "react";
21 function Component(props) {
22 - const $ = useMemoCache(7);
22 + const $ = useMemoCache(6);
23 let t0;
24 if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
25 t0 = { value: "" };
@@ -29,31 +29,29 @@ function Component(props) {
29 }
30 const [x, setX] = useState(t0);
31 const c_1 = $[1] !== x;
32 - const c_2 = $[2] !== setX;
32 let t1;
34 - if (c_1 || c_2) {
33 + if (c_1) {
34 t1 = (e) => {
35 // INVALID! should use copy-on-write and pass the new value
36 x.value = e.target.value;
37 setX(x);
38 };
39 $[1] = x;
41 - $[2] = setX;
42 - $[3] = t1;
40 + $[2] = t1;
41 } else {
44 - t1 = $[3];
42 + t1 = $[2];
43 }
44 const onChange = t1;
47 - const c_4 = $[4] !== x.value;
48 - const c_5 = $[5] !== onChange;
45 + const c_3 = $[3] !== x.value;
46 + const c_4 = $[4] !== onChange;
47 let t2;
50 - if (c_4 || c_5) {
48 + if (c_3 || c_4) {
49 t2 = <input value={x.value} onChange={onChange} />;
52 - $[4] = x.value;
53 - $[5] = onChange;
54 - $[6] = t2;
50 + $[3] = x.value;
51 + $[4] = onChange;
52 + $[5] = t2;
53 } else {
56 - t2 = $[6];
54 + t2 = $[5];
55 }
56 return t2;
57 }
compiler/forget/src/__tests__/fixtures/compiler/nested-function-shadowed-identifiers.expect.md
+11 -13
@@ -20,31 +20,29 @@ function Component(props) {
20 ```javascript
21 import { unstable_useMemoCache as useMemoCache } from "react";
22 function Component(props) {
23 - const $ = useMemoCache(5);
23 + const $ = useMemoCache(4);
24 const [x, setX] = useState(null);
25 - const c_0 = $[0] !== setX;
25 let t0;
27 - if (c_0) {
26 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
27 t0 = (e) => {
28 let x_0 = null; // intentionally shadow the original x
29 setX((currentX) => currentX + x_0); // intentionally refer to shadowed x
30 };
32 - $[0] = setX;
33 - $[1] = t0;
31 + $[0] = t0;
32 } else {
35 - t0 = $[1];
33 + t0 = $[0];
34 }
35 const onChange = t0;
38 - const c_2 = $[2] !== x;
39 - const c_3 = $[3] !== onChange;
36 + const c_1 = $[1] !== x;
37 + const c_2 = $[2] !== onChange;
38 let t1;
41 - if (c_2 || c_3) {
39 + if (c_1 || c_2) {
40 t1 = <input value={x} onChange={onChange} />;
43 - $[2] = x;
44 - $[3] = onChange;
45 - $[4] = t1;
41 + $[1] = x;
42 + $[2] = onChange;
43 + $[3] = t1;
44 } else {
47 - t1 = $[4];
45 + t1 = $[3];
46 }
47 return t1;
48 }
compiler/forget/src/__tests__/fixtures/compiler/use-callback-simple.expect.md
+11 -13
@@ -16,28 +16,26 @@ function component() {
16 ```javascript
17 import { unstable_useMemoCache as useMemoCache } from "react";
18 function component() {
19 - const $ = useMemoCache(5);
19 + const $ = useMemoCache(4);
20 const [count, setCount] = useState(0);
21 - const c_0 = $[0] !== setCount;
22 - const c_1 = $[1] !== count;
21 + const c_0 = $[0] !== count;
22 let t0;
24 - if (c_0 || c_1) {
23 + if (c_0) {
24 t0 = () => setCount(count + 1);
26 - $[0] = setCount;
27 - $[1] = count;
28 - $[2] = t0;
25 + $[0] = count;
26 + $[1] = t0;
27 } else {
30 - t0 = $[2];
28 + t0 = $[1];
29 }
30 const increment = t0;
33 - const c_3 = $[3] !== increment;
31 + const c_2 = $[2] !== increment;
32 let t1;
35 - if (c_3) {
33 + if (c_2) {
34 t1 = <Foo onClick={increment} />;
37 - $[3] = increment;
38 - $[4] = t1;
35 + $[2] = increment;
36 + $[3] = t1;
37 } else {
40 - t1 = $[4];
38 + t1 = $[3];
39 }
40 return t1;
41 }