fix(cache): clear entries references with greater precision

Author: smalluban

src/cache.js

@@ -2,6 +2,7 @@ import * as emitter from "./emitter.js";
 
 const entries = new WeakMap();
 const suspense = new WeakSet();
+const values = new WeakMap();
 
 export function getEntry(target, key) {
   let targetMap = entries.get(target);
@@ -24,6 +25,14 @@ export function getEntry(target, key) {
       resolved: false,
     };
     targetMap.set(key, entry);
+  } else if (entry.contexts.size) {
+    entry.contexts.forEach(contextEntry => {
+      if (suspense.has(contextEntry.target)) {
+        entry.contexts.delete(contextEntry);
+        contextEntry.depState = 0;
+        contextEntry.resolved = false;
+      }
+    });
   }
 
   return entry;
@@ -43,7 +52,10 @@ export function getEntries(target) {
 function dispatchDeep(entry) {
   entry.resolved = false;
 
-  emitter.dispatch(entry);
+  if (!suspense.has(entry.target)) {
+    emitter.dispatch(entry);
+  }
+
   entry.contexts.forEach(dispatchDeep);
 }
 
@@ -52,10 +64,6 @@ const contexts = new Set();
 export function get(target, key, getter, validate) {
   const entry = getEntry(target, key);
 
-  if (contexts.has(entry)) {
-    throw Error(`Circular get invocation is forbidden: '${key}'`);
-  }
-
   if (context && !suspense.has(context.target)) {
     context.deps.add(entry);
     entry.contexts.add(context);
@@ -88,6 +96,10 @@ export function get(target, key, getter, validate) {
     }
   }
 
+  if (contexts.has(entry)) {
+    throw Error(`Circular get invocation is forbidden: '${key}'`);
+  }
+
   const lastContext = context;
 
   try {
@@ -143,6 +155,7 @@ export function set(target, key, setter, value) {
   if (newValue !== entry.value) {
     entry.value = newValue;
     entry.state += 1;
+    entry.depState = 0;
 
     dispatchDeep(entry);
   }
@@ -209,54 +222,24 @@ export function invalidateAll(target, clearValue, deleteValue) {
   }
 }
 
-const values = new WeakMap();
 export function observe(target, key, getter, fn) {
   const entry = getEntry(target, key);
 
   return emitter.subscribe(entry, () => {
-    if (!suspense.has(target)) {
-      const lastValue = values.get(entry);
-      const value = get(target, key, getter);
+    const lastValue = values.get(entry);
+    const value = get(target, key, getter);
 
-      if (value !== lastValue) {
-        fn(target, value, lastValue);
-        values.set(entry, value);
-      }
+    if (value !== lastValue) {
+      fn(target, value, lastValue);
+      values.set(entry, value);
     }
   });
 }
 
-const clearTargets = new Set();
-export function clear(target) {
-  if (clearTargets.size === 0) {
-    requestAnimationFrame(() => {
-      clearTargets.forEach(t => {
-        const targetMap = entries.get(t);
-        if (targetMap) {
-          targetMap.forEach(entry => {
-            entry.resolved = false;
-
-            entry.deps.forEach(depEntry => {
-              depEntry.contexts.delete(entry);
-            });
-
-            entry.deps.clear();
-            entry.contexts.clear();
-          });
-        }
-      });
-
-      clearTargets.clear();
-    });
-  }
-  clearTargets.add(target);
-}
-
 export function suspend(target) {
   suspense.add(target);
 }
 
 export function unsuspend(target) {
   suspense.delete(target);
-  clearTargets.delete(target);
 }

src/define.js

@@ -191,7 +191,6 @@ export function defineElement(tagName, hybrids) {
       }
 
       cache.suspend(this);
-      cache.clear(this);
     }
   }
 

test/spec/cache.js

@@ -8,7 +8,6 @@ import {
   suspend,
   unsuspend,
   getEntry,
-  clear,
 } from "../../src/cache.js";
 
 describe("cache:", () => {
@@ -339,34 +338,6 @@ describe("cache:", () => {
     });
   });
 
-  describe("clear()", () => {
-    it("removes references from deps", done => {
-      const dep = {};
-      get(target, "key", () => get(dep, "value", () => "value"));
-      const hasTarget = () =>
-        [...getEntry(dep, "value").contexts].some(
-          entry => entry.target === target,
-        );
-      expect(hasTarget()).toBe(true);
-
-      clear(target);
-
-      requestAnimationFrame(() => {
-        expect(hasTarget()).toBe(false);
-        done();
-      });
-    });
-
-    it("skip when already added", done => {
-      clear(target);
-      clear(target);
-
-      requestAnimationFrame(() => {
-        done();
-      });
-    });
-  });
-
   describe("suspend()", () => {
     it("disables cache hits", () => {
       get(target, "key", spy);
@@ -395,7 +366,30 @@ describe("cache:", () => {
       observe(target, "key", (_, v) => v, spy);
 
       requestAnimationFrame(() => {
-        expect(spy).not.toHaveBeenCalled();
+        expect(spy).toHaveBeenCalledTimes(1);
+
+        set(target, "key", () => "new value");
+        requestAnimationFrame(() => {
+          expect(spy).toHaveBeenCalledTimes(1);
+          done();
+        });
+      });
+    });
+
+    it("removes references from deps when dep is called", done => {
+      const dep = {};
+      get(target, "key", () => get(dep, "value", () => "value"));
+      const hasTarget = () =>
+        [...getEntry(dep, "value").contexts].some(
+          entry => entry.target === target,
+        );
+      expect(hasTarget()).toBe(true);
+
+      suspend(target);
+      get(dep, "value", () => "value");
+
+      requestAnimationFrame(() => {
+        expect(hasTarget()).toBe(false);
         done();
       });
     });