@samitouri / QOS-React / commits / fd87c9b9e1

[patch] Bug in deriveMinimalDeps

Found when enabling Forget on Webamp

Mofei Zhang committed Oct 6, 2023 at 17:37 UTC fd87c9b9e13d06983194bc19f641a2caf2d12b50
4 files changed +120 -11
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/DeriveMinimalDependencies.ts
+4 -11
@@ -472,17 +472,10 @@ function addSubtreeIntersection(
472 suggestions: null,
473 });
474
475 - otherProperties.forEach((properties) =>
476 - properties.forEach((node, _) =>
477 - CompilerError.invariant(!isUnconditional(node.accessType), {
478 - reason:
479 - "[DeriveMinimalDependencies] Expected otherProperties to only contain unconditional nodes!",
480 - description: null,
481 - loc: null,
482 - suggestions: null,
483 - })
484 - )
485 - );
475 + // otherProperties here may contain unconditional nodes as the result of
476 + // recursively merging exhaustively conditional children with unconditionally
477 + // accessed nodes (e.g. in the test condition itself)
478 + // See `reduce-reactive-cond-deps-cfg-nested-testifelse` fixture for example
479
480 for (const [propertyName, currNode] of currProperties) {
481 const otherNodes = mapNonNull(otherProperties, (properties) =>
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-cfg-nested-testifelse.expect.md new
+72
@@ -0,0 +1,72 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import { setProperty } from "shared-runtime";
6 +
7 +function useFoo({
8 + o,
9 + branchCheck,
10 +}: {
11 + o: { value: number };
12 + branchCheck: boolean;
13 +}) {
14 + let x = {};
15 + if (branchCheck) {
16 + setProperty(x, o.value);
17 + } else {
18 + if (o.value) {
19 + setProperty(x, o.value);
20 + } else {
21 + setProperty(x, o.value);
22 + }
23 + }
24 + return x;
25 +}
26 +
27 +export const FIXTURE_ENTRYPOINT = {
28 + fn: useFoo,
29 + params: [{ o: { value: 2 }, branchCheck: false }],
30 +};
31 +
32 +```
33 +
34 +## Code
35 +
36 +```javascript
37 +import { unstable_useMemoCache as useMemoCache } from "react";
38 +import { setProperty } from "shared-runtime";
39 +
40 +function useFoo(t27) {
41 + const $ = useMemoCache(3);
42 + const { o, branchCheck } = t27;
43 + const c_0 = $[0] !== branchCheck;
44 + const c_1 = $[1] !== o.value;
45 + let x;
46 + if (c_0 || c_1) {
47 + x = {};
48 + if (branchCheck) {
49 + setProperty(x, o.value);
50 + } else {
51 + if (o.value) {
52 + setProperty(x, o.value);
53 + } else {
54 + setProperty(x, o.value);
55 + }
56 + }
57 + $[0] = branchCheck;
58 + $[1] = o.value;
59 + $[2] = x;
60 + } else {
61 + x = $[2];
62 + }
63 + return x;
64 +}
65 +
66 +export const FIXTURE_ENTRYPOINT = {
67 + fn: useFoo,
68 + params: [{ o: { value: 2 }, branchCheck: false }],
69 +};
70 +
71 +```
72 +
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-cond-deps-cfg-nested-testifelse.ts new
+26
@@ -0,0 +1,26 @@
1 +import { setProperty } from "shared-runtime";
2 +
3 +function useFoo({
4 + o,
5 + branchCheck,
6 +}: {
7 + o: { value: number };
8 + branchCheck: boolean;
9 +}) {
10 + let x = {};
11 + if (branchCheck) {
12 + setProperty(x, o.value);
13 + } else {
14 + if (o.value) {
15 + setProperty(x, o.value);
16 + } else {
17 + setProperty(x, o.value);
18 + }
19 + }
20 + return x;
21 +}
22 +
23 +export const FIXTURE_ENTRYPOINT = {
24 + fn: useFoo,
25 + params: [{ o: { value: 2 }, branchCheck: false }],
26 +};
compiler/packages/sprout/src/shared-runtime.ts
+18
@@ -52,6 +52,24 @@ export function mutate(arg: any): void {
52 }
53 }
54
55 +export function setProperty(arg: any, property: any): void {
56 + // don't mutate primitive
57 + if (typeof arg === null || typeof arg !== "object") {
58 + return;
59 + }
60 +
61 + let count: number = 0;
62 + let key;
63 + while (true) {
64 + key = "wat" + count;
65 + if (!Object.hasOwn(arg, key)) {
66 + arg[key] = property;
67 + return;
68 + }
69 + count++;
70 + }
71 +}
72 +
73 export function graphql(value: string): string {
74 return value;
75 }