From 1ad1bb08b17bb9a44be4b8d59053a69a9ca899e6 Mon Sep 17 00:00:00 2001
From: Dominik Lubanski <dominik.lubanski@gmail.com>
Date: Mon, 27 Jan 2020 08:39:26 +0100
Subject: [PATCH] fix(cache): optmize context clean up for observed property

---
 src/cache.js       | 31 +++++++++++++++++--------------
 src/define.js      |  2 +-
 test/spec/cache.js | 16 ++++++++++++++--
 3 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/src/cache.js b/src/cache.js
index 1936d2b4..12dd7b97 100644
--- a/src/cache.js
+++ b/src/cache.js
@@ -49,7 +49,6 @@ export function get(target, key, getter) {
   const entry = getEntry(target, key);
 
   if (contextStack.size && contextStack.has(entry)) {
-    contextStack.clear();
     throw Error(`Circular get invocation of the '${key}' property in '${stringifyElement(target)}'`);
   }
 
@@ -63,21 +62,20 @@ export function get(target, key, getter) {
     }
   });
 
-  contextStack.add(entry);
-
   if (entry.checksum && entry.checksum === calculateChecksum(entry)) {
-    contextStack.delete(entry);
     return entry.value;
   }
 
-  if (entry.deps && entry.deps.size) {
-    entry.deps.forEach((depEntry) => {
-      if (depEntry.contexts) depEntry.contexts.delete(entry);
-    });
-    entry.deps = undefined;
-  }
-
   try {
+    contextStack.add(entry);
+
+    if (entry.observed && entry.deps && entry.deps.size) {
+      entry.deps.forEach((depEntry) => {
+        depEntry.contexts.delete(entry);
+      });
+    }
+
+    entry.deps = undefined;
     const nextValue = getter(target, entry.value);
 
     if (nextValue !== entry.value) {
@@ -90,7 +88,14 @@ export function get(target, key, getter) {
     entry.checksum = calculateChecksum(entry);
     contextStack.delete(entry);
   } catch (e) {
-    contextStack.clear();
+    entry.checksum = 0;
+
+    contextStack.delete(entry);
+    contextStack.forEach((context) => {
+      context.deps.delete(entry);
+      if (context.observed) entry.contexts.delete(context);
+    });
+
     throw e;
   }
 
@@ -99,7 +104,6 @@ export function get(target, key, getter) {
 
 export function set(target, key, setter, value, force) {
   if (contextStack.size && !force) {
-    contextStack.clear();
     throw Error(`Try to set '${key}' of '${stringifyElement(target)}' in get call`);
   }
 
@@ -117,7 +121,6 @@ export function set(target, key, setter, value, force) {
 
 export function invalidate(target, key, clearValue) {
   if (contextStack.size) {
-    contextStack.clear();
     throw Error(`Try to invalidate '${key}' in '${stringifyElement(target)}' get call`);
   }
 
diff --git a/src/define.js b/src/define.js
index 0c044425..f448b21c 100644
--- a/src/define.js
+++ b/src/define.js
@@ -82,7 +82,7 @@ if (process.env.NODE_ENV !== 'production') {
             node.disconnectedCallback();
 
             Object.keys(node.constructor.hybrids).forEach((key) => {
-              cache.invalidate(node, key, node[key] === hybrids[key]);
+              cache.invalidate(node, key, node.constructor.hybrids[key] !== hybrids[key]);
             });
 
             node.connectedCallback();
diff --git a/test/spec/cache.js b/test/spec/cache.js
index 5abad96d..5d2afbf7 100644
--- a/test/spec/cache.js
+++ b/test/spec/cache.js
@@ -152,7 +152,7 @@ describe('cache:', () => {
       });
     });
 
-    it('clean emitter when unobserve', (done) => {
+    it('cleans emitter when unobserve', (done) => {
       const unobserve = observe(target, 'key', _, spy);
 
       requestAnimationFrame(() => {
@@ -165,7 +165,7 @@ describe('cache:', () => {
       });
     });
 
-    it('clean dependencies contexts when unobserve', (done) => {
+    it('cleans dependencies contexts when unobserve', (done) => {
       const getter = () => get(target, 'otherKey', () => get(target, 'deepKey', _));
       const unobserve = observe(target, 'key', getter, spy);
 
@@ -179,5 +179,17 @@ describe('cache:', () => {
         });
       });
     });
+
+    it('cleans contexts when getter throws', (done) => {
+      const getKey = () => get(target, 'key', () => get(target, 'otherKey', () => { throw Error(); }));
+      const unobserve = observe(target, 'key', () => {}, spy);
+
+      expect(() => getKey()).toThrow();
+
+      requestAnimationFrame(() => {
+        expect(() => unobserve()).not.toThrow();
+        done();
+      });
+    });
   });
 });