optimism - npm Package Compare versions

Comparing version 0.2.3 to 0.3.0

lib/entry.js

@@ -22,2 +22,5 @@ "use strict";
   this.dirtyChildren = null;
+
+  this.subscribe = null;
+  this.unsubscribe = null;
 }
@@ -30,8 +33,16 @@
 Ep.setDirty = function setDirty() {
+  if (this.dirty) return;
   this.dirty = true;
-  this.parents.forEach(function (parent) {
-    parent.reportDirtyChild(this);
-  }, this);
+  this.parents.forEach(reportDirty, this);
 };
 
+function reportDirty(parent) {
+  parent.reportDirtyChild(this);
+}
+
+function reportClean(parent) {
+  parent.reportCleanChild(this);
+}
+
+// Let a parent Entry know that one of its children may be dirty.
 Ep.reportDirtyChild = function reportDirtyChild(child) {
@@ -53,13 +64,17 @@ assert(this.childValues.has(child));
   this.dirtyChildren.add(child);
-
-  this.parents.forEach(function (parent) {
-    parent.reportDirtyChild(this);
-  }, this);
+  this.parents.forEach(reportDirty, this);
 };
 
+// Let a parent Entry know that one of its children is no longer dirty.
 Ep.reportCleanChild = function reportCleanChild(child) {
   assert(! child.dirty);
-
   this.childValues.set(child, child.value);
+  this.removeDirtyChild(child);
+};
 
+// Often we are removing a child because it is no longer dirty, so
+// child.dirty is not a precondition for this method. Also note that the
+// child may remain in this.childValues, so we definitely do not want to
+// call child.parents.delete(this) here.
+Ep.removeDirtyChild = function removeDirtyChild(child) {
   var dc = this.dirtyChildren;
@@ -74,5 +89,10 @@ if (dc) {
 
-  if (this.dirty || dc) {
-    // This Entry is not clean, either because it's explicitly dirty or
-    // because it still has dirty children, so we can't report it as
+  this.maybeReportClean();
+};
+
+Ep.maybeReportClean = function maybeReportClean() {
+  if (this.dirty || (this.dirtyChildren &&
+                     this.dirtyChildren.size > 0)) {
+    // This Entry is still not clean, either because it's explicitly dirty
+    // or because it still has dirty children, so we can't report it as
     // clean to its parents.
@@ -82,5 +102,3 @@ return;
 
-  this.parents.forEach(function (parent) {
-    parent.reportCleanChild(this);
-  }, this);
+  this.parents.forEach(reportClean, this);
 };
@@ -105,5 +123,9 @@
     this.dirtyChildren.forEach(function (child) {
+      assert(this.childValues.has(child));
+
       var oldValue = this.childValues.get(child);
-      if (child.recomputeIfDirty() !== oldValue) {
-        this.dirty = true;
+      var newValue = child.recomputeIfDirty();
+
+      if (newValue !== oldValue) {
+        this.setDirty();
       }
@@ -133,9 +155,4 @@ }, this);
 
-  // Set this.dirty = true so that we can tell if the try block threw an
-  // exception below, even though we aren't catching the exception.
-  this.dirty = true;
-
   try {
     this.value = this.fn.apply(null, this.args);
-    this.dirty = false;
 
@@ -146,14 +163,26 @@ } finally {
 
-    if (! this.dirty) {
-      if (oldParentEntry) {
-        this.parents.add(oldParentEntry);
-      }
+    if (oldParentEntry) {
+      this.parents.add(oldParentEntry);
+    }
+  }
 
-      this.parents.forEach(function (parent) {
-        parent.reportCleanChild(this);
-      }, this);
+  if (! subscribe(this)) {
+    // Mark this entry dirty if we fail to resubscribe. This is important
+    // because, if we have a subscribe function and it failed, then we're
+    // going to miss important notifications about the potential dirtiness
+    // of this.value.
+    this.setDirty();
 
-      return this.value;
-    }
+  } else {
+    // If we successfully recomputed this.value and did not fail to
+    // (re)subscribe, then this Entry is no longer explicitly dirty.
+    this.dirty = false;
+
+    // However, this Entry may still have dirty children, so even though
+    // there's a chance we can let our parents know we're clean, it's best
+    // to let this.maybeReportClean() decide.
+    this.maybeReportClean();
   }
+
+  return this.value;
 };
@@ -163,13 +192,51 @@
   this.childValues.forEach(function (value, child) {
-    child.parents.delete(this);
+    this.forgetChild(child);
   }, this);
 
-  this.childValues.clear();
+  // After we forget all our children, this.dirtyChildren must be empty
+  // and thus have been reset to null.
+  assert.strictEqual(this.dirtyChildren, null);
+};
 
-  var dc = this.dirtyChildren;
-  if (dc) {
-    dc.clear();
-    emptySetPool.push(dc);
-    this.dirtyChildren = null;
+Ep.forgetChild = function forgetChild(child) {
+  child.parents.delete(this);
+  this.childValues.delete(child);
+  this.removeDirtyChild(child);
+};
+
+Ep.dispose = function dispose() {
+  // If we're no longer going to be subscribed to changes affecting this
+  // Entry, then we'd better inform its parents that it needs to be
+  // recomputed.
+  this.setDirty();
+  this.forgetChildren();
+  unsubscribe(this);
+};
+
+function subscribe(entry) {
+  if (typeof entry.subscribe === "function") {
+    try {
+      unsubscribe(entry); // Prevent double subscriptions.
+      entry.unsubscribe = entry.subscribe.apply(null, entry.args);
+    } catch (e) {
+      // If this Entry has a subscribe function and it threw an exception
+      // (or an unsubscribe function it previously returned now throws),
+      // return false to indicate that we were not able to subscribe (or
+      // unsubscribe), and this Entry should remain dirty.
+      entry.setDirty();
+      return false;
+    }
   }
-};
+
+  // Returning true indicates either that there was no entry.subscribe
+  // function or that it succeeded.
+  return true;
+}
+
+function unsubscribe(entry) {
+  var unsub = entry.unsubscribe;
+  if (typeof unsub === "function") {
+    entry.unsubscribe = null;
+    unsub();
+  }
+}

lib/index.js

@@ -31,10 +31,3 @@ "use strict";
     dispose: function (key, entry) {
-      // If we're no longer going to be subscribed to changes affecting
-      // this entry, then we'd better inform its parents that it needs to
-      // be recomputed.
-      entry.setDirty();
-
-      if (typeof entry.unsubscribe === "function") {
-        entry.unsubscribe.call(optimistic);
-      }
+      entry.dispose();
     }
@@ -55,5 +48,3 @@ });
       cache.set(key, entry = new Entry(fn, key, args));
-      if (typeof options.subscribe === "function") {
-        entry.unsubscribe = options.subscribe.apply(optimistic, args);
-      }
+      entry.subscribe = options.subscribe;
     }
@@ -60,0 +51,0 @@

package.json

 {
   "name": "optimism",
-  "version": "0.2.3",
+  "version": "0.3.0",
   "author": "Ben Newman <[e-mail]>",
@@ -5,0 +5,0 @@ "description": "Composable reactive caching with efficient invalidation.",

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

11281

increased by22.05%

Lines of code

266

increased by23.15%

No dependency changes