Skip to content

Commit

Permalink
Merge branch '387-recursive-computed-eval'
Browse files Browse the repository at this point in the history
  • Loading branch information
unknown authored and unknown committed Mar 20, 2012
2 parents 440a933 + e89e12a commit 5d9c406
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
11 changes: 11 additions & 0 deletions spec/dependentObservableBehaviors.js
Expand Up @@ -226,5 +226,16 @@ describe('Dependent Observable', {
value_of(timesEvaluated).should_be(0);
value_of(instance()).should_be(123);
value_of(timesEvaluated).should_be(1);
},

'Should prevent recursive calling of read function': function() {
var observable = ko.observable(0),
computed = ko.dependentObservable(function() {
// this both reads and writes to the observable
// will result in errors like "Maximum call stack size exceeded" (chrome)
// or "Out of stack space" (IE) or "too much recursion" (Firefox) if recursion
// isn't prevented
observable(observable() + 1);
});
}
})
14 changes: 13 additions & 1 deletion src/subscribables/dependentObservable.js
@@ -1,6 +1,7 @@
ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunctionTarget, options) {
var _latestValue,
_hasBeenEvaluated = false,
_isBeingEvaluated = false,
readFunction = evaluatorFunctionOrOptions;

if (readFunction && typeof readFunction == "object") {
Expand Down Expand Up @@ -58,6 +59,14 @@ ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunction
}

function evaluateImmediate() {
if (_isBeingEvaluated) {
// If the evaluation of a ko.computed causes side effects, it's possible that it will trigger its own re-evaluation.
// This is not desirable (it's hard for a developer to realise a chain of dependencies might cause this, and they almost
// certainly didn't intend infinite re-evaluations). So, for predictability, we simply prevent ko.computeds from causing
// their own re-evaluation. Further discussion at https://github.com/SteveSanderson/knockout/pull/387
return;
}

// Don't dispose on first evaluation, because the "disposeWhen" callback might
// e.g., dispose when the associated DOM element isn't in the doc, and it's not
// going to be in the doc until *after* the first evaluation
Expand All @@ -66,6 +75,7 @@ ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunction
return;
}

_isBeingEvaluated = true;
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.
Expand All @@ -86,6 +96,7 @@ ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunction
if (disposalCandidates[i])
_subscriptionsToDependencies.splice(i, 1)[0].dispose();
}
_hasBeenEvaluated = true;

dependentObservable["notifySubscribers"](_latestValue, "beforeChange");
_latestValue = newValue;
Expand All @@ -95,7 +106,8 @@ ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunction
}

dependentObservable["notifySubscribers"](_latestValue);
_hasBeenEvaluated = true;
_isBeingEvaluated = false;

}

function dependentObservable() {
Expand Down

0 comments on commit 5d9c406

Please sign in to comment.