Skip to content

Commit

Permalink
Merge branch longer-subscriber-chains into master
Browse files Browse the repository at this point in the history
Conflicts:
	spec/dependentObservableBehaviors.js
  • Loading branch information
SteveSanderson committed Sep 8, 2013
2 parents 8dab4bf + 688a567 commit da39a30
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 6 deletions.
18 changes: 18 additions & 0 deletions spec/dependentObservableBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -408,4 +408,22 @@ describe('Dependent Observable', function() {
observable.valueHasMutated();
expect(notifiedValues).toEqual([1]);
});

// Borrowed from haberman/knockout (see knockout/knockout#359)
it('Should allow long chains without overflowing the stack', function() {
// maximum with previous code (when running this test only): Chrome 28: 1310, IE 10: 2200; FF 23: 103
// maximum with changed code: Chrome 28: 2620, +100%, IE 10: 4900, +122%; FF 23: 267, +160%
var depth = 200;
var first = ko.observable(0);
var last = first;
for (var i = 0; i < depth; i++) {
(function() {
var l = last;
last = ko.computed(function() { return l() + 1; });
})();
}
var all = ko.computed(function() { return last() + first(); });
first(1);
expect(all()).toEqual(depth+2);
});
});
2 changes: 1 addition & 1 deletion src/subscribables/dependencyDetection.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ ko.dependencyDetection = (function () {

return {
begin: function (callback) {
_frames.push({ callback: callback, distinctDependencies:[] });
_frames.push(callback && { callback: callback, distinctDependencies:[] });
},

end: function () {
Expand Down
2 changes: 1 addition & 1 deletion src/subscribables/dependentObservable.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunction
addSubscriptionToDependency(subscribable); // Brand new subscription - add it
});

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

// For each subscription no longer being used, remove it from the active subscriptions list and dispose it
for (var i = disposalCandidates.length - 1; i >= 0; i--) {
Expand Down
11 changes: 7 additions & 4 deletions src/subscribables/subscribable.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,17 @@ ko.subscribable['fn'] = {
"notifySubscribers": function (valueToNotify, event) {
event = event || defaultEvent;
if (this.hasSubscriptionsForEvent(event)) {
ko.dependencyDetection.ignore(function() {
ko.utils.arrayForEach(this._subscriptions[event].slice(0), function (subscription) {
try {
ko.dependencyDetection.begin();
for (var a = this._subscriptions[event].slice(0), i = 0, subscription; subscription = a[i]; ++i) {
// In case a subscription was disposed during the arrayForEach cycle, check
// for isDisposed on each subscription before invoking its callback
if (subscription && (subscription.isDisposed !== true))
subscription.callback(valueToNotify);
});
}, this);
}
} finally {
ko.dependencyDetection.end();
}
}
},

Expand Down

0 comments on commit da39a30

Please sign in to comment.