Merge pull request knockout#2240 from knockout/2240-deferred-computed…

…-notify-update deferred updates: only notify computed changed if it was evaluated
arrowgr · Jun 3, 2017 · d20958b · d20958b
2 parents e41727d + fa17c59
commit d20958b
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 5 deletions.
diff --git a/spec/asyncBehaviors.js b/spec/asyncBehaviors.js
@@ -1209,5 +1209,33 @@ describe('Deferred', function() {
                 expect(threeNotifications).toEqual([true, false]);
             }
         });
+
+        it('Should only notify changes if computed was evaluated', function() {
+            // See https://github.com/knockout/knockout/issues/2240
+            // Set up a scenario where a computed will be marked as dirty but won't get marked as
+            // stale and so won't be re-evaluated
+            this.restoreAfter(ko.options, 'deferUpdates');
+            ko.options.deferUpdates = true;
+
+            var obs = ko.observable('somevalue'),
+                isTruthy = ko.pureComputed(function() { return !!obs(); }),
+                objIfTruthy = ko.pureComputed(function() { return isTruthy(); }).extend({ notify: 'always' }),
+                notifySpy = jasmine.createSpy('callback'),
+                subscription = objIfTruthy.subscribe(notifySpy);
+
+            obs('someothervalue');
+            jasmine.Clock.tick(1);
+            expect(notifySpy).not.toHaveBeenCalled();
+
+            obs('');
+            jasmine.Clock.tick(1);
+            expect(notifySpy).toHaveBeenCalled();
+            expect(notifySpy.argsForCall).toEqual([[false]]);
+            notifySpy.reset();
+
+            obs(undefined);
+            jasmine.Clock.tick(1);
+            expect(notifySpy).not.toHaveBeenCalled();
+        });
     });
 });
diff --git a/src/subscribables/dependentObservable.js b/src/subscribables/dependentObservable.js
@@ -317,6 +317,9 @@ var computedFn = {
             if (!state.isSleeping && notifyChange) {
                 computedObservable["notifySubscribers"](state.latestValue);
             }
+            if (computedObservable._recordUpdate) {
+                computedObservable._recordUpdate();
+            }
         }
 
         if (isInitial) {
@@ -376,7 +379,7 @@ var computedFn = {
 
             // Pass the observable to the "limit" code, which will evaluate it when
             // it's time to do the notification.
-            this._limitChange(this);
+            this._limitChange(this, !isChange /* isDirty */);
         };
     },
     dispose: function () {

diff --git a/src/subscribables/subscribable.js b/src/subscribables/subscribable.js
@@ -92,7 +92,8 @@ var ko_subscribable_fn = {
 
     limit: function(limitFunction) {
         var self = this, selfIsObservable = ko.isObservable(self),
-            ignoreBeforeChange, notifyNextChange, previousValue, pendingValue, beforeChange = 'beforeChange';
+            ignoreBeforeChange, notifyNextChange, previousValue, pendingValue, didUpdate,
+            beforeChange = 'beforeChange';
 
         if (!self._origNotifySubscribers) {
             self._origNotifySubscribers = self["notifySubscribers"];
@@ -107,16 +108,19 @@ var ko_subscribable_fn = {
             if (selfIsObservable && pendingValue === self) {
                 pendingValue = self._evalIfChanged ? self._evalIfChanged() : self();
             }
-            var shouldNotify = notifyNextChange || self.isDifferent(previousValue, pendingValue);
+            var shouldNotify = notifyNextChange || (didUpdate && self.isDifferent(previousValue, pendingValue));
 
-            notifyNextChange = ignoreBeforeChange = false;
+            didUpdate = notifyNextChange = ignoreBeforeChange = false;
 
             if (shouldNotify) {
                 self._origNotifySubscribers(previousValue = pendingValue);
             }
         });
 
-        self._limitChange = function(value) {
+        self._limitChange = function(value, isDirty) {
+            if (!isDirty || !self._notificationIsPending) {
+                didUpdate = !isDirty;
+            }
             self._changeSubscriptions = self._subscriptions[defaultEvent].slice(0);
             self._notificationIsPending = ignoreBeforeChange = true;
             pendingValue = value;
@@ -128,6 +132,9 @@ var ko_subscribable_fn = {
                 self._origNotifySubscribers(value, beforeChange);
             }
         };
+        self._recordUpdate = function() {
+            didUpdate = true;
+        };
         self._notifyNextChangeIfValueIsDifferent = function() {
             if (self.isDifferent(previousValue, self.peek(true /*evaluate*/))) {
                 notifyNextChange = true;