fix #597, run depth first over breadth first

solidjs · Aug 17, 2021 · d946cfe · d946cfe
1 parent 0a6772b
commit d946cfe
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 58 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -33,15 +33,15 @@
     "@types/jest": "^26.0.24",
     "@types/shelljs": "^0.8.8",
     "babel-jest": "^26.6.3",
-    "babel-plugin-jsx-dom-expressions": "^0.29.11",
+    "babel-plugin-jsx-dom-expressions": "^0.29.14",
     "coveralls": "^3.1.1",
-    "dom-expressions": "0.29.12",
+    "dom-expressions": "0.29.14",
     "gitly": "^2.1.0",
-    "hyper-dom-expressions": "0.29.12",
+    "hyper-dom-expressions": "0.29.14",
     "jest": "~26.6.3",
     "jest-ts-webcompat-resolver": "^1.0.0",
     "lerna": "^3.22.1",
-    "lit-dom-expressions": "0.29.12",
+    "lit-dom-expressions": "0.29.14",
     "ncp": "2.0.0",
     "npm-run-all": "^4.1.5",
     "rimraf": "^3.0.2",

diff --git a/packages/babel-preset-solid/package.json b/packages/babel-preset-solid/package.json
@@ -14,6 +14,6 @@
     "test": "node test.js"
   },
   "dependencies": {
-    "babel-plugin-jsx-dom-expressions": "^0.29.11"
+    "babel-plugin-jsx-dom-expressions": "^0.29.14"
   }
 }
diff --git a/packages/solid/src/reactive/signal.ts b/packages/solid/src/reactive/signal.ts
@@ -153,7 +153,9 @@ export function createRenderEffect<T>(
   value?: T,
   options?: { name?: string }
 ): void {
-  updateComputation(createComputation(fn, value, false, STALE, "_SOLID_DEV_" ? options : undefined));
+  updateComputation(
+    createComputation(fn, value, false, STALE, "_SOLID_DEV_" ? options : undefined)
+  );
 }
 
 export function createEffect<T>(fn: (v?: T) => T | undefined): void;
@@ -732,16 +734,20 @@ export function writeSignal(node: Signal<any> | Memo<any>, value: any, isComp?:
     }
     if (!TransitionRunning) node.value = value;
   } else node.value = value;
-  if (node.observers && (!Updates || node.observers.length)) {
+  if (node.observers && node.observers.length) {
     runUpdates(() => {
       for (let i = 0; i < node.observers!.length; i += 1) {
         const o = node.observers![i];
         if (TransitionRunning && Transition!.disposed.has(o)) continue;
-        if ((o as Memo<any>).observers && o.state !== PENDING) markUpstream(o as Memo<any>);
-        if (TransitionRunning) o.tState = STALE;
-        else o.state = STALE;
         if (o.pure) Updates!.push(o);
         else Effects!.push(o);
+        if (
+          (o as Memo<any>).observers &&
+          ((TransitionRunning && !o.tState) || (!TransitionRunning && !o.state))
+        )
+          markUpstream(o as Memo<any>);
+        if (TransitionRunning) o.tState = STALE;
+        else o.state = STALE;
       }
       if (Updates!.length > 10e5) {
         Updates = [];
@@ -838,7 +844,8 @@ function createComputation<T>(
 
 function runTop(node: Computation<any>) {
   const runningTransition = Transition && Transition.running;
-  if (node.state !== STALE && !(runningTransition && node.tState)) return;
+  if (!runningTransition && node.state !== STALE) return (node.state = 0);
+  if (runningTransition && node.tState !== STALE) return (node.tState = 0);
   if (node.suspense && untrack(node.suspense.inFallback!))
     return node!.suspense.effects!.push(node!);
   const ancestors = [node];
@@ -988,10 +995,14 @@ function lookDownstream(node: Computation<any>) {
 }
 
 function markUpstream(node: Memo<any>) {
+  const runningTransition = Transition && Transition.running;
   for (let i = 0; i < node.observers!.length; i += 1) {
     const o = node.observers![i];
-    if (!o.state) {
-      o.state = PENDING;
+    if (!o.state || (runningTransition && !o.tState)) {
+      if (runningTransition) o.tState = PENDING;
+      else o.state = PENDING;
+      if (o.pure) Updates!.push(o);
+      else Effects!.push(o);
       (o as Memo<any>).observers && markUpstream(o as Memo<any>);
     }
   }

diff --git a/packages/solid/test/signals.memo.spec.ts b/packages/solid/test/signals.memo.spec.ts
@@ -44,7 +44,7 @@ describe("createMemo", () => {
         expect(order).toBe("t1c1c2");
         order = "";
         set(1);
-        expect(order).toBe("t1c1c2");
+        expect(order).toBe("t1c2c1");
       });
     });
 
@@ -71,7 +71,7 @@ describe("createMemo", () => {
         });
         order = "";
         set(1);
-        expect(order).toBe("t1c1c2c2_1");
+        expect(order).toBe("t1c2c2_1c1");
       });
     });
   });
@@ -213,28 +213,24 @@ describe("createMemo", () => {
 
     // it("evaluates stale computations before dependendees when trackers stay unchanged", () => {
     //   createRoot(() => {
-    //     let [s1, set] = createSignal(1);
+    //     let [s1, set] = createSignal(1, { equals: false });
     //     let order = "";
     //     let t1 = createMemo(
     //       () => {
     //         order += "t1";
     //         return s1() > 2;
-    //       },
-    //       undefined,
-    //       true
+    //       }
     //     );
     //     let t2 = createMemo(
     //       () => {
     //         order += "t2";
     //         return s1() > 2;
-    //       },
-    //       undefined,
-    //       true
+    //       }
     //     );
     //     let c1 = createMemo(() => {
     //       order += "c1";
     //       s1();
-    //     });
+    //     }, undefined, { equals: false });
     //     createComputed(() => {
     //       order += "c2";
     //       t1();