Merge pull request #2227 from knockout/2227-computed-auto-dispose-fix

dispose a computed that depends only on inactive computeds
knockout · Apr 28, 2017 · 5f10dfd · 5f10dfd
2 parents 9798f6d + 6268fc9
commit 5f10dfd
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 9 deletions.
diff --git a/spec/dependentObservableBehaviors.js b/spec/dependentObservableBehaviors.js
@@ -353,7 +353,7 @@ describe('Dependent Observable', function() {
     it('Should describe itself as inactive if subsequent runs of the evaluator result in there being no dependencies', function() {
         var someObservable = ko.observable('initial'),
             shouldHaveDependency = true,
-            computed = ko.computed(function () { return shouldHaveDependency && someObservable(); });
+            computed = ko.computed(function () { shouldHaveDependency && someObservable(); });
         expect(computed.isActive()).toEqual(true);
 
         // Trigger a refresh
@@ -362,6 +362,19 @@ describe('Dependent Observable', function() {
         expect(computed.isActive()).toEqual(false);
     });
 
+    it('Should be inactive if it depends on an inactive computed', function() {
+        var someObservable = ko.observable('initial'),
+            shouldHaveDependency = true,
+            computed1 = ko.computed(function () { shouldHaveDependency && someObservable(); }),
+            computed2 = ko.computed(computed1);
+        expect(computed2.isActive()).toEqual(true);
+
+        // Trigger a refresh
+        shouldHaveDependency = false;
+        someObservable('modified');
+        expect(computed2.isActive()).toEqual(false);
+    });
+
     it('Should advertise that instances *can* have values written to them if you supply a "write" callback', function() {
         var instance = ko.computed({
             read: function() {},

diff --git a/src/subscribables/dependentObservable.js b/src/subscribables/dependentObservable.js
@@ -45,7 +45,9 @@ ko.computed = ko.dependentObservable = function (evaluatorFunctionOrOptions, eva
             return this; // Permits chained assignments
         } else {
             // Reading the value
-            ko.dependencyDetection.registerDependency(computedObservable);
+            if (!state.isDisposed) {
+                ko.dependencyDetection.registerDependency(computedObservable);
+            }
             if (state.isDirty || (state.isSleeping && computedObservable.haveDependenciesChanged())) {
                 computedObservable.evaluateImmediate();
             }
@@ -261,10 +263,6 @@ var computedFn = {
             state.isBeingEvaluated = false;
         }
 
-        if (!state.dependenciesCount) {
-            computedObservable.dispose();
-        }
-
         return changed;
     },
     evaluateImmediate_CallReadWithDependencyDetection: function (notifyChange) {
@@ -297,7 +295,14 @@ var computedFn = {
 
         var newValue = this.evaluateImmediate_CallReadThenEndDependencyDetection(state, dependencyDetectionContext);
 
-        if (computedObservable.isDifferent(state.latestValue, newValue)) {
+        if (!state.dependenciesCount) {
+            computedObservable.dispose();
+            changed = true; // When evaluation causes a disposal, make sure all dependent computeds get notified so they'll see the new state
+        } else {
+            changed = computedObservable.isDifferent(state.latestValue, newValue);
+        }
+
+        if (changed) {
             if (!state.isSleeping) {
                 computedObservable["notifySubscribers"](state.latestValue, "beforeChange");
             } else {
@@ -312,8 +317,6 @@ var computedFn = {
             if (!state.isSleeping && notifyChange) {
                 computedObservable["notifySubscribers"](state.latestValue);
             }
-
-            changed = true;
         }
 
         if (isInitial) {