Skip to content

Commit

Permalink
deferred updates: only notify that a computed changed if it was evalu…
Browse files Browse the repository at this point in the history
…ated
  • Loading branch information
mbest committed Jun 2, 2017
1 parent 89617f7 commit fa17c59
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 5 deletions.
28 changes: 28 additions & 0 deletions spec/asyncBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});
5 changes: 4 additions & 1 deletion src/subscribables/dependentObservable.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,9 @@ var computedFn = {
if (!state.isSleeping && notifyChange) {
computedObservable["notifySubscribers"](state.latestValue);
}
if (computedObservable._recordUpdate) {
computedObservable._recordUpdate();
}
}

if (isInitial) {
Expand Down Expand Up @@ -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 () {
Expand Down
15 changes: 11 additions & 4 deletions src/subscribables/subscribable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
Expand All @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit fa17c59

Please sign in to comment.