Skip to content

Commit

Permalink
Don't expose isSleeping; auto-dispose pure computed if it has no depe…
Browse files Browse the repository at this point in the history 
…ndencies; make copy of parameter object
  • Loading branch information
@mbest
mbest committed May 8, 2014
1 parent 5532e46 commit df33ca2
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 95 deletions.
5 changes: 0 additions & 5 deletions spec/dependentObservableBehaviors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,11 +320,6 @@ describe('Dependent Observable', function() {
expect(ko.isWriteableObservable(instance)).toEqual(true);
});

it('Should describe itself as not sleeping', function () {
var computed = ko.computed(function () { });
expect(computed.isSleeping()).toEqual(false);
});

it('Should allow deferring of evaluation (and hence dependency detection)', function () {
var timesEvaluated = 0;
var instance = ko.computed({
Expand Down
62 changes: 26 additions & 36 deletions spec/pureComputedBehaviors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,28 @@ describe('Pure Computed', function() {
expect(invokedWriteWithValue).toEqual("some value");
});

it('Should describe itself as sleeping initially', function () {
it('Should describe itself as active initially', function() {
var computed = ko.pureComputed(function () { });
expect(computed.isSleeping()).toEqual(true);
expect(computed.isActive()).toEqual(true);
});

it('Should describe itself as active initially (always "active" while sleeping)', function() {
it('Should describe itself as inactive if the evaluator has no dependencies on its first run', function() {
var computed = ko.pureComputed(function () { });
computed(); // access the computed to evaluate it
expect(computed.isActive()).toEqual(false);
});

it('Should describe itself as active if the evaluator has dependencies on its first run', function() {
var observable = ko.observable('initial'),
computed = ko.computed(observable);
computed(); // access the computed to evaluate it
expect(computed.isActive()).toEqual(true);
});

it('Should evaluate on each access while sleeping', function () {
it('Should evaluate on each access while sleeping if not disposed', function () {
var timesEvaluated = 0,
computed = ko.pureComputed(function () { return ++timesEvaluated; });
data = ko.observable(),
computed = ko.pureComputed(function () { data(); return ++timesEvaluated; });

expect(timesEvaluated).toEqual(0);

Expand Down Expand Up @@ -92,11 +101,8 @@ describe('Pure Computed', function() {
computed = ko.pureComputed(data),
notifiedValues = [];

expect(computed.isSleeping()).toEqual(true);

// Subscribe to computed; the dependency should now be tracked
computed.subscribe(function (value) { notifiedValues.push(value); });
expect(computed.isSleeping()).toEqual(false);
expect(data.getSubscriptionsCount()).toEqual(1);
expect(computed.getDependenciesCount()).toEqual(1);

Expand All @@ -113,12 +119,12 @@ describe('Pure Computed', function() {
computed = ko.pureComputed(data),
subscription = computed.subscribe(function () {});

expect(computed.isSleeping()).toEqual(false);
expect(data.getSubscriptionsCount()).toEqual(1);
expect(computed.getDependenciesCount()).toEqual(1);

// Dispose the subscription to the computed
subscription.dispose();
expect(computed.isSleeping()).toEqual(true);
// It goes to sleep, disposing its subscription to the observable
expect(data.getSubscriptionsCount()).toEqual(0);
expect(computed.getDependenciesCount()).toEqual(1);
});
Expand All @@ -139,28 +145,27 @@ describe('Pure Computed', function() {
expect(timesEvaluated).toEqual(2);

// Double check that disposing subscriptions puts the pure computed to sleep
expect(pureComputed.isSleeping()).toEqual(false);
computed.dispose();
expect(pureComputed.isSleeping()).toEqual(true);
expect(data.getSubscriptionsCount()).toEqual(0);
});

it('Should be able to re-evaluate a sleeping computed that previously threw an exception', function() {
var shouldThrow = false, value = 1,
var shouldThrow = ko.observable(false), observableValue = ko.observable(1),
computed = ko.pureComputed(function() {
if (shouldThrow) {
if (shouldThrow()) {
throw Error("Error during computed evaluation");
} else {
return value;
return observableValue();
}
});

expect(computed()).toEqual(1);

value = 2;
shouldThrow = true;
observableValue(2);
shouldThrow(true);
expect(computed).toThrow("Error during computed evaluation");

shouldThrow = false;
shouldThrow(false);
expect(computed()).toEqual(2);
});

Expand All @@ -184,9 +189,11 @@ describe('Pure Computed', function() {

describe('Context', function() {
it('Should not define initial evaluation', function() {
var evaluationCount = 0,
var observable = ko.observable(1),
evaluationCount = 0,
computed = ko.pureComputed(function() {
++evaluationCount;
observable(); // for dependency
return ko.computedContext.isInitial();
});

Expand Down Expand Up @@ -225,22 +232,5 @@ describe('Pure Computed', function() {
expect(computed()).toEqual(2); // second evaluation
expect(computed.getDependenciesCount()).toEqual(2); // matches value from context
});

it('Should accurately report when computed is sleeping', function() {
var computed = ko.pureComputed(function() {
return ko.computedContext.isSleeping();
});

expect(computed()).toEqual(true); // compueted is sleeping

var subscription = computed.subscribe(function () {});
expect(computed()).toEqual(false); // compueted is not sleeping

subscription.dispose();
expect(computed()).toEqual(true); // compueted is sleeping again

// value outside of computed is undefined
expect(ko.computedContext.isSleeping()).toBeUndefined();
});
});
});
5 changes: 0 additions & 5 deletions src/subscribables/dependencyDetection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ ko.computedContext = ko.dependencyDetection = (function () {
isInitial: function() {
if (currentFrame)
return currentFrame.isInitial;
},

isSleeping: function () {
if (currentFrame)
return currentFrame.computed.isSleeping();
}
};
})();
Expand Down
96 changes: 47 additions & 49 deletions src/subscribables/dependentObservable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,66 +104,65 @@ ko.computed = ko.dependentObservable = function (evaluatorFunctionOrOptions, eva
ko.dependencyDetection.end();
_isBeingEvaluated = false;
}
return;
}

try {
// Initially, we assume that none of the subscriptions are still being used (i.e., all are candidates for disposal).
// Then, during evaluation, we cross off any that are in fact still being used.
var disposalCandidates = _subscriptionsToDependencies, disposalCount = _dependenciesCount;
ko.dependencyDetection.begin({
callback: function(subscribable, id) {
if (!_isDisposed) {
if (disposalCount && disposalCandidates[id]) {
// Don't want to dispose this subscription, as it's still being used
_subscriptionsToDependencies[id] = disposalCandidates[id];
++_dependenciesCount;
delete disposalCandidates[id];
--disposalCount;
} else {
// Brand new subscription - add it
addSubscriptionToDependency(subscribable, id);
} else {
try {
// Initially, we assume that none of the subscriptions are still being used (i.e., all are candidates for disposal).
// Then, during evaluation, we cross off any that are in fact still being used.
var disposalCandidates = _subscriptionsToDependencies, disposalCount = _dependenciesCount;
ko.dependencyDetection.begin({
callback: function(subscribable, id) {
if (!_isDisposed) {
if (disposalCount && disposalCandidates[id]) {
// Don't want to dispose this subscription, as it's still being used
_subscriptionsToDependencies[id] = disposalCandidates[id];
++_dependenciesCount;
delete disposalCandidates[id];
--disposalCount;
} else {
// Brand new subscription - add it
addSubscriptionToDependency(subscribable, id);
}
}
}
},
computed: dependentObservable,
isInitial: pure ? undefined : !_dependenciesCount // If we're evaluating when there are no previous dependencies, it must be the first time
});
},
computed: dependentObservable,
isInitial: pure ? undefined : !_dependenciesCount // If we're evaluating when there are no previous dependencies, it must be the first time
});

_subscriptionsToDependencies = {};
_dependenciesCount = 0;
_subscriptionsToDependencies = {};
_dependenciesCount = 0;

try {
var newValue = evaluatorFunctionTarget ? readFunction.call(evaluatorFunctionTarget) : readFunction();
try {
var newValue = evaluatorFunctionTarget ? readFunction.call(evaluatorFunctionTarget) : readFunction();

} finally {
ko.dependencyDetection.end();
} finally {
ko.dependencyDetection.end();

// For each subscription no longer being used, remove it from the active subscriptions list and dispose it
if (disposalCount) {
ko.utils.objectForEach(disposalCandidates, function(id, toDispose) {
toDispose.dispose();
});
}
// For each subscription no longer being used, remove it from the active subscriptions list and dispose it
if (disposalCount) {
ko.utils.objectForEach(disposalCandidates, function(id, toDispose) {
toDispose.dispose();
});
}

_needsEvaluation = false;
}
_needsEvaluation = false;
}

if (dependentObservable.isDifferent(_latestValue, newValue)) {
dependentObservable["notifySubscribers"](_latestValue, "beforeChange");
if (dependentObservable.isDifferent(_latestValue, newValue)) {
dependentObservable["notifySubscribers"](_latestValue, "beforeChange");

_latestValue = newValue;
if (DEBUG) dependentObservable._latestValue = _latestValue;
_latestValue = newValue;
if (DEBUG) dependentObservable._latestValue = _latestValue;

if (suppressChangeNotification !== true) { // Check for strict true value since setTimeout in Firefox passes a numeric value to the function
dependentObservable["notifySubscribers"](_latestValue);
if (suppressChangeNotification !== true) { // Check for strict true value since setTimeout in Firefox passes a numeric value to the function
dependentObservable["notifySubscribers"](_latestValue);
}
}
} finally {
_isBeingEvaluated = false;
}
} finally {
_isBeingEvaluated = false;
}

if (!pure && !_dependenciesCount)
if (!_dependenciesCount)
dispose();
}

Expand Down Expand Up @@ -218,7 +217,6 @@ ko.computed = ko.dependentObservable = function (evaluatorFunctionOrOptions, eva
dependentObservable.hasWriteFunction = typeof options["write"] === "function";
dependentObservable.dispose = function () { dispose(); };
dependentObservable.isActive = isActive;
dependentObservable.isSleeping = function () { return isSleeping; };

// Replace the limit function with one that delays evaluation as well.
var originalLimit = dependentObservable.limit;
Expand Down Expand Up @@ -263,7 +261,6 @@ ko.computed = ko.dependentObservable = function (evaluatorFunctionOrOptions, eva
ko.exportProperty(dependentObservable, 'dispose', dependentObservable.dispose);
ko.exportProperty(dependentObservable, 'isActive', dependentObservable.isActive);
ko.exportProperty(dependentObservable, 'getDependenciesCount', dependentObservable.getDependenciesCount);
ko.exportProperty(dependentObservable, 'isSleeping', dependentObservable.isSleeping);

// Add a "disposeWhen" callback that, on each evaluation, disposes if the node was removed without using ko.removeNode.
if (disposeWhenNodeIsRemoved) {
Expand Down Expand Up @@ -327,6 +324,7 @@ ko.pureComputed = function (evaluatorFunctionOrOptions, evaluatorFunctionTarget)
if (typeof evaluatorFunctionOrOptions === 'function') {
return ko.computed(evaluatorFunctionOrOptions, evaluatorFunctionTarget, {'pure':true});
} else {
evaluatorFunctionOrOptions = ko.utils.extend({}, evaluatorFunctionOrOptions); // make a copy of the parameter object
evaluatorFunctionOrOptions['pure'] = true;
return ko.computed(evaluatorFunctionOrOptions, evaluatorFunctionTarget);
}
Expand Down

0 comments on commit df33ca2

Please sign in to comment.