Skip to content

Commit

Permalink
Do the 'dirty' notification after scheduling evaluation so order of e…
Browse files Browse the repository at this point in the history 
…valuation is maintained.
  • Loading branch information
@mbest
mbest committed Feb 23, 2012
1 parent 66aeab3 commit 01dd050
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions knockout-deferred-updates.js
View file
Expand Up @@ -190,9 +190,6 @@ var newComputed = function (evaluatorFunctionOrOptions, evaluatorFunctionTarget,
if (_isBeingEvaluated) if (_isBeingEvaluated)
return; return;
_needsEvaluation = true; _needsEvaluation = true;
dependentObservable["notifySubscribers"](_latestValue, "dirty");
if (!_needsEvaluation) // The notification might have triggered an evaluation
return;
var throttleEvaluationTimeout = dependentObservable['throttleEvaluation']; var throttleEvaluationTimeout = dependentObservable['throttleEvaluation'];
if (throttleEvaluationTimeout && throttleEvaluationTimeout >= 0) { if (throttleEvaluationTimeout && throttleEvaluationTimeout >= 0) {
clearTimeout(evaluationTimeoutInstance); clearTimeout(evaluationTimeoutInstance);
Expand All @@ -201,6 +198,9 @@ var newComputed = function (evaluatorFunctionOrOptions, evaluatorFunctionTarget,
ko.tasks.processDelayed(evaluateImmediate, true, disposeWhenNodeIsRemoved); ko.tasks.processDelayed(evaluateImmediate, true, disposeWhenNodeIsRemoved);
else else
evaluateImmediate(); evaluateImmediate();
dependentObservable["notifySubscribers"](_latestValue, "dirty");

This comment has been minimized.

Sign in to view

Copy link
@scottmessinger

scottmessinger Feb 27, 2012

This line is throwing an error.

// knockout-deferred-updates.js:234
// Uncaught TypeError: Object function
dependentObservable() {
        if (arguments.length > 0) {
            set.apply(dependentObservable, arguments);
        } else {
            return get();
        }
    } has no method 'notifySubscribers'

I'm having a hard time reproducing it in jsfiddle, but I can't figure out why it would break my app. I'm including deferred-updates after knockout-2.0.0 just as I did before this change. Any ideas what's going on?

Here's the full stacktrace from Chrome if you're interested:
https://gist.github.com/1927694

This comment has been minimized.

Sign in to view

Copy link
@scottmessinger

scottmessinger Feb 27, 2012

Even though the stacktrace says the file is knockout-2.0.0.rc3, I updated the content to the released 2.0.0 version and still got the error.

This comment has been minimized.

Sign in to view

Copy link
@mbest

mbest Feb 27, 2012

Author Owner

It'll break if you have a computed observable that triggers re-evaluation of itself during its initial evaluation. I thought I had fixed it, but it got lost somehow.

This comment has been minimized.

Sign in to view

Copy link
@scottmessinger

scottmessinger via email Feb 28, 2012

This comment has been minimized.

Sign in to view

Copy link
@mbest

mbest Feb 28, 2012

Author Owner

I pushed a fix for it. Let me know if it works.
if (!_needsEvaluation && throttleEvaluationTimeout) // The notification might have triggered an evaluation
clearTimeout(evaluationTimeoutInstance);
} }


function addDependency(subscribable) { function addDependency(subscribable) {
Expand Down

0 comments on commit 01dd050

Please sign in to comment.