Skip to content

Commit

Permalink
when using deferred updates, only evaluate a computed if one of its d…
Browse files Browse the repository at this point in the history
…ependencies actually changes (rather than just being "dirty")
  • Loading branch information
mbest committed Dec 29, 2016
1 parent 87e8a59 commit 495a969
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 10 deletions.
20 changes: 20 additions & 0 deletions spec/asyncBehaviors.js
Expand Up @@ -975,6 +975,26 @@ describe('Deferred', function() {
expect(notifySpy.argsForCall).toEqual([['i(x,h(cx,g(ex,fx),d(bx,cx)),bx,fx)']]); // only one evaluation and notification
});

it('Should minimize evaluation when dependent computed doesn\'t actually change', function() {
// From https://github.com/knockout/knockout/issues/2174
this.restoreAfter(ko.options, 'deferUpdates');
ko.options.deferUpdates = true;

var source = ko.observable({ key: 'value' }),
c1 = ko.computed(function () {
return source()['key'];
}),
countEval = 0,
c2 = ko.computed(function () {
countEval++;
return c1();
});

source({ key: 'value' });
jasmine.Clock.tick(1);
expect(countEval).toEqual(1);
});

it('Should ignore recursive dirty events', function() {
// From https://github.com/knockout/knockout/issues/1943
this.restoreAfter(ko.options, 'deferUpdates');
Expand Down
34 changes: 25 additions & 9 deletions src/subscribables/dependentObservable.js
Expand Up @@ -18,6 +18,7 @@ ko.computed = ko.dependentObservable = function (evaluatorFunctionOrOptions, eva
var state = {
latestValue: undefined,
isStale: true,
isDirty: false,
isBeingEvaluated: false,
suppressDisposalUntilDisposeWhenReturnsFalse: false,
isDisposed: false,
Expand Down Expand Up @@ -45,7 +46,7 @@ ko.computed = ko.dependentObservable = function (evaluatorFunctionOrOptions, eva
} else {
// Reading the value
ko.dependencyDetection.registerDependency(computedObservable);
if (state.isStale || (state.isSleeping && computedObservable.haveDependenciesChanged())) {
if (state.isStale || state.isDirty || (state.isSleeping && computedObservable.haveDependenciesChanged())) {
computedObservable.evaluateImmediate();
}
return state.latestValue;
Expand Down Expand Up @@ -166,16 +167,19 @@ var computedFn = {
markDirty: function () {
// Process "dirty" events if we can handle delayed notifications
if (this._evalDelayed && !this[computedState].isBeingEvaluated) {
this._evalDelayed();
this._evalDelayed(false /*isChange*/);
}
},
isActive: function () {
return this[computedState].isStale || this[computedState].dependenciesCount > 0;
var state = this[computedState];
return state.isStale || state.isDirty || state.dependenciesCount > 0;
},
respondToChange: function () {
// Ignore "change" events if we've already scheduled a delayed notification
if (!this._notificationIsPending) {
this.evaluatePossiblyAsync();
} else if (this[computedState].isDirty) {
this[computedState].isStale = true;
}
},
subscribeToDependency: function (target) {
Expand All @@ -202,7 +206,7 @@ var computedFn = {
computedObservable.evaluateImmediate(true /*notifyChange*/);
}, throttleEvaluationTimeout);
} else if (computedObservable._evalDelayed) {
computedObservable._evalDelayed();
computedObservable._evalDelayed(true /*isChange*/);
} else {
computedObservable.evaluateImmediate(true /*notifyChange*/);
}
Expand Down Expand Up @@ -322,7 +326,7 @@ var computedFn = {
ko.utils.objectForEach(dependencyDetectionContext.disposalCandidates, computedDisposeDependencyCallback);
}

state.isStale = false;
state.isStale = state.isDirty = false;
}
},
peek: function () {
Expand All @@ -336,15 +340,26 @@ var computedFn = {
limit: function (limitFunction) {
// Override the limit function with one that delays evaluation as well
ko.subscribable['fn'].limit.call(this, limitFunction);
this._evalDelayed = function () {
this._evalIfChanged = function () {
if (this[computedState].isStale) {
this.evaluateImmediate();
}
return this[computedState].latestValue;
};
this._evalDelayed = function (isChange) {
this._limitBeforeChange(this[computedState].latestValue);

this[computedState].isStale = true; // Mark as dirty
// Mark as dirty
if (isChange) {
this[computedState].isStale = true;
} else {
this[computedState].isDirty = true;
}

// Pass the observable to the "limit" code, which will access it when
// Pass the observable to the "limit" code, which will evaluate it when
// it's time to do the notification.
this._limitChange(this);
}
};
},
dispose: function () {
var state = this[computedState];
Expand All @@ -361,6 +376,7 @@ var computedFn = {
state.dependenciesCount = 0;
state.isDisposed = true;
state.isStale = false;
state.isDirty = false;
state.isSleeping = false;
state.disposeWhenNodeIsRemoved = undefined;
state.disposeWhen = undefined;
Expand Down
2 changes: 1 addition & 1 deletion src/subscribables/subscribable.js
Expand Up @@ -104,7 +104,7 @@ var ko_subscribable_fn = {
// If an observable provided a reference to itself, access it to get the latest value.
// This allows computed observables to delay calculating their value until needed.
if (selfIsObservable && pendingValue === self) {
pendingValue = self();
pendingValue = self._evalIfChanged ? self._evalIfChanged() : self();
}
ignoreBeforeChange = false;
if (self.isDifferent(previousValue, pendingValue)) {
Expand Down

0 comments on commit 495a969

Please sign in to comment.