diff --git a/spec/asyncBehaviors.js b/spec/asyncBehaviors.js index f6201aa6d..f2dc534eb 100644 --- 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 index 058a308e2..955696a0f 100644 --- 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 index a22c3c5f7..27f71faff 100644 --- 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;