Skip to content

Commit

Permalink
Merge branch '483-dispose-if-no-dependencies'
Browse files Browse the repository at this point in the history
  • Loading branch information
@SteveSanderson
SteveSanderson committed Sep 7, 2012
2 parents cdd585c + ce18b1c commit cb594d0
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 47 deletions.
45 changes: 45 additions & 0 deletions spec/dependentObservableBehaviors.js
View file
Expand Up @@ -211,6 +211,7 @@ describe('Dependent Observable', {
var computed1 = new ko.dependentObservable(function () { return underlyingObservable() + 1; });
var computed2 = new ko.dependentObservable(function () { return computed1.peek() + 1; });
value_of(computed2()).should_be(3);
value_of(computed2.isActive()).should_be(false);

underlyingObservable(11);
value_of(computed2()).should_be(3); // value wasn't changed
Expand All @@ -237,11 +238,55 @@ describe('Dependent Observable', {
);
value_of(timesEvaluated).should_be(1);
value_of(dependent.getDependenciesCount()).should_be(1);
value_of(dependent.isActive()).should_be(true);

timeToDispose = true;
underlyingObservable(101);
value_of(timesEvaluated).should_be(1);
value_of(dependent.getDependenciesCount()).should_be(0);
value_of(dependent.isActive()).should_be(false);
},

'Should describe itself as active if the evaluator has dependencies on its first run': function() {
var someObservable = ko.observable('initial'),
dependentObservable = new ko.dependentObservable(function () { return someObservable(); });
value_of(dependentObservable.isActive()).should_be(true);
},

'Should describe itself as inactive if the evaluator has no dependencies on its first run': function() {
var dependentObservable = new ko.dependentObservable(function () { return 123; });
value_of(dependentObservable.isActive()).should_be(false);
},

'Should describe itself as inactive if subsequent runs of the evaluator result in there being no dependencies': function() {
var someObservable = ko.observable('initial'),
shouldHaveDependency = true,
dependentObservable = new ko.dependentObservable(function () { return shouldHaveDependency && someObservable(); });
value_of(dependentObservable.isActive()).should_be(true);

// Trigger a refresh
shouldHaveDependency = false;
someObservable('modified');
value_of(dependentObservable.isActive()).should_be(false);
},

'Should register DOM node disposal callback only if active after the initial evaluation': function() {
// Set up an active one
var nodeForActive = document.createElement('DIV'),
observable = ko.observable('initial'),
activeDependentObservable = ko.dependentObservable({ read: function() { return observable(); }, disposeWhenNodeIsRemoved: nodeForActive });
var nodeForInactive = document.createElement('DIV')
inactiveDependentObservable = ko.dependentObservable({ read: function() { return 123; }, disposeWhenNodeIsRemoved: nodeForInactive });

value_of(activeDependentObservable.isActive()).should_be(true);
value_of(inactiveDependentObservable.isActive()).should_be(false);

// Infer existence of disposal callbacks from presence/absence of DOM data. This is really just an implementation detail,
// and so it's unusual to rely on it in a spec. However, the presence/absence of the callback isn't exposed in any other way,
// and if the implementation ever changes, this spec should automatically fail because we're checking for both the positive
// and negative cases.
value_of(ko.utils.domData.clear(nodeForActive)).should_be(true); // There was a callback
value_of(ko.utils.domData.clear(nodeForInactive)).should_be(false); // There was no callback
},

'Should advertise that instances *can* have values written to them if you supply a "write" callback': function() {
Expand Down
2 changes: 1 addition & 1 deletion src/binding/bindingAttributeSyntax.js
View file
Expand Up @@ -144,7 +144,7 @@
}
},
null,
{ 'disposeWhenNodeIsRemoved' : node }
{ disposeWhenNodeIsRemoved : node }
);

return {
Expand Down
7 changes: 4 additions & 3 deletions src/binding/editDetection/arrayToDomNodeChildren.js
View file
Expand Up @@ -63,8 +63,8 @@
// of which nodes would be deleted if valueToMap was itself later removed
mappedNodes.splice(0, mappedNodes.length);
ko.utils.arrayPushAll(mappedNodes, newMappedNodes);
}, null, { 'disposeWhenNodeIsRemoved': containerNode, 'disposeWhen': function() { return (mappedNodes.length == 0) || !ko.utils.domNodeIsAttachedToDocument(mappedNodes[0]) } });
return { mappedNodes : mappedNodes, dependentObservable : dependentObservable };
}, null, { disposeWhenNodeIsRemoved: containerNode, disposeWhen: function() { return (mappedNodes.length == 0) || !ko.utils.domNodeIsAttachedToDocument(mappedNodes[0]) } });
return { mappedNodes : mappedNodes, dependentObservable : (dependentObservable.isActive() ? dependentObservable : undefined) };
}

var lastMappingResultDomDataKey = "setDomNodeChildrenFromArrayMapping_lastMappingResult";
Expand Down Expand Up @@ -121,7 +121,8 @@
mapData = lastMappingResult[lastMappingResultIndex];

// Stop tracking changes to the mapping for these nodes
mapData.dependentObservable.dispose();
if (mapData.dependentObservable)
mapData.dependentObservable.dispose();

// Queue these nodes for later removal
nodesToDelete.push.apply(nodesToDelete, fixUpNodesToBeMovedOrRemoved(mapData.mappedNodes));
Expand Down
75 changes: 45 additions & 30 deletions src/subscribables/dependentObservable.js
View file
Expand Up @@ -14,41 +14,20 @@ ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunction
if (!readFunction)
readFunction = options["read"];
}
// By here, "options" is always non-null
if (typeof readFunction != "function")
throw new Error("Pass a function that returns the value of the ko.computed");

var writeFunction = options["write"];
if (!evaluatorFunctionTarget)
evaluatorFunctionTarget = options["owner"];
function addSubscriptionToDependency(subscribable) {
_subscriptionsToDependencies.push(subscribable.subscribe(evaluatePossiblyAsync));
}

var _subscriptionsToDependencies = [];
function disposeAllSubscriptionsToDependencies() {
ko.utils.arrayForEach(_subscriptionsToDependencies, function (subscription) {
subscription.dispose();
});
_subscriptionsToDependencies = [];
}
var dispose = disposeAllSubscriptionsToDependencies;

// Build "disposeWhenNodeIsRemoved" and "disposeWhenNodeIsRemovedCallback" option values
// (Note: "disposeWhenNodeIsRemoved" option both proactively disposes as soon as the node is removed using ko.removeNode(),
// plus adds a "disposeWhen" callback that, on each evaluation, disposes if the node was removed by some other means.)
var disposeWhenNodeIsRemoved = (typeof options["disposeWhenNodeIsRemoved"] == "object") ? options["disposeWhenNodeIsRemoved"] : null;
var disposeWhen = options["disposeWhen"] || function() { return false; };
if (disposeWhenNodeIsRemoved) {
dispose = function() {
ko.utils.domNodeDisposal.removeDisposeCallback(disposeWhenNodeIsRemoved, arguments.callee);
disposeAllSubscriptionsToDependencies();
};
ko.utils.domNodeDisposal.addDisposeCallback(disposeWhenNodeIsRemoved, dispose);
var existingDisposeWhenFunction = disposeWhen;
disposeWhen = function () {
return !ko.utils.domNodeIsAttachedToDocument(disposeWhenNodeIsRemoved) || existingDisposeWhenFunction();
}
}

var evaluationTimeoutInstance = null;
function evaluatePossiblyAsync() {
var throttleEvaluationTimeout = dependentObservable['throttleEvaluation'];
if (throttleEvaluationTimeout && throttleEvaluationTimeout >= 0) {
Expand Down Expand Up @@ -86,7 +65,7 @@ ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunction
if ((inOld = ko.utils.arrayIndexOf(disposalCandidates, subscribable)) >= 0)
disposalCandidates[inOld] = undefined; // Don't want to dispose this subscription, as it's still being used
else
_subscriptionsToDependencies.push(subscribable.subscribe(evaluatePossiblyAsync)); // Brand new subscription - add it
addSubscriptionToDependency(subscribable); // Brand new subscription - add it
});

var newValue = readFunction.call(evaluatorFunctionTarget);
Expand All @@ -107,7 +86,8 @@ ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunction

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

if (!_subscriptionsToDependencies.length)
dispose();
}

function dependentObservable() {
Expand All @@ -128,26 +108,61 @@ ko.dependentObservable = function (evaluatorFunctionOrOptions, evaluatorFunction
}
}

dependentObservable.peek = function () {
function peek() {
if (!_hasBeenEvaluated)
evaluateImmediate();
return _latestValue;
}

function isActive() {
return !_hasBeenEvaluated || _subscriptionsToDependencies.length > 0;
}

// By here, "options" is always non-null
var writeFunction = options["write"],
disposeWhenNodeIsRemoved = options["disposeWhenNodeIsRemoved"] || options.disposeWhenNodeIsRemoved || null,
disposeWhen = options["disposeWhen"] || options.disposeWhen || function() { return false; },
dispose = disposeAllSubscriptionsToDependencies,
_subscriptionsToDependencies = [],
evaluationTimeoutInstance = null;

if (!evaluatorFunctionTarget)
evaluatorFunctionTarget = options["owner"];

dependentObservable.peek = peek;
dependentObservable.getDependenciesCount = function () { return _subscriptionsToDependencies.length; };
dependentObservable.hasWriteFunction = typeof options["write"] === "function";
dependentObservable.dispose = function () { dispose(); };
dependentObservable.isActive = isActive;

ko.subscribable.call(dependentObservable);
ko.utils.extend(dependentObservable, ko.dependentObservable['fn']);

if (options['deferEvaluation'] !== true)
evaluateImmediate();

ko.exportProperty(dependentObservable, 'peek', dependentObservable.peek);
ko.exportProperty(dependentObservable, 'dispose', dependentObservable.dispose);
ko.exportProperty(dependentObservable, 'isActive', dependentObservable.isActive);
ko.exportProperty(dependentObservable, 'getDependenciesCount', dependentObservable.getDependenciesCount);

// Evaluate, unless deferEvaluation is true
if (options['deferEvaluation'] !== true)
evaluateImmediate();

// Build "disposeWhenNodeIsRemoved" and "disposeWhenNodeIsRemovedCallback" option values.
// But skip if isActive is false (there will never be any dependencies to dispose).
// (Note: "disposeWhenNodeIsRemoved" option both proactively disposes as soon as the node is removed using ko.removeNode(),
// plus adds a "disposeWhen" callback that, on each evaluation, disposes if the node was removed by some other means.)
if (disposeWhenNodeIsRemoved && isActive()) {
dispose = function() {
ko.utils.domNodeDisposal.removeDisposeCallback(disposeWhenNodeIsRemoved, arguments.callee);
disposeAllSubscriptionsToDependencies();
};
ko.utils.domNodeDisposal.addDisposeCallback(disposeWhenNodeIsRemoved, dispose);
var existingDisposeWhenFunction = disposeWhen;
disposeWhen = function () {
return !ko.utils.domNodeIsAttachedToDocument(disposeWhenNodeIsRemoved) || existingDisposeWhenFunction();
}
}

return dependentObservable;
};

Expand Down
26 changes: 13 additions & 13 deletions src/templating/templating.js
View file
Expand Up @@ -107,7 +107,7 @@
}
},
null,
{ 'disposeWhen': whenToDispose, 'disposeWhenNodeIsRemoved': activelyDisposeWhenNodeIsRemoved }
{ disposeWhen: whenToDispose, disposeWhenNodeIsRemoved: activelyDisposeWhenNodeIsRemoved }
);
} else {
// We don't yet have a DOM node to evaluate, so use a memo and render the template later when there is a DOM node
Expand Down Expand Up @@ -150,15 +150,15 @@

ko.utils.setDomNodeChildrenFromArrayMapping(targetNode, filteredArray, executeTemplateForArrayItem, options, activateBindingsCallback);

}, null, { 'disposeWhenNodeIsRemoved': targetNode });
}, null, { disposeWhenNodeIsRemoved: targetNode });
};

var templateSubscriptionDomDataKey = '__ko__templateSubscriptionDomDataKey__';
function disposeOldSubscriptionAndStoreNewOne(element, newSubscription) {
var oldSubscription = ko.utils.domData.get(element, templateSubscriptionDomDataKey);
if (oldSubscription && (typeof(oldSubscription.dispose) == 'function'))
oldSubscription.dispose();
ko.utils.domData.set(element, templateSubscriptionDomDataKey, newSubscription);
var templateComputedDomDataKey = '__ko__templateComputedDomDataKey__';
function disposeOldComputedAndStoreNewOne(element, newComputed) {
var oldComputed = ko.utils.domData.get(element, templateComputedDomDataKey);
if (oldComputed && (typeof(oldComputed.dispose) == 'function'))
oldComputed.dispose();
ko.utils.domData.set(element, templateComputedDomDataKey, (newComputed && newComputed.isActive()) ? newComputed : undefined);
}

ko.bindingHandlers['template'] = {
Expand Down Expand Up @@ -190,25 +190,25 @@
shouldDisplay = shouldDisplay && !ko.utils.unwrapObservable(bindingValue['ifnot']);
}

var templateSubscription = null;
var templateComputed = null;

if ((typeof bindingValue === 'object') && ('foreach' in bindingValue)) { // Note: can't use 'in' operator on strings
// Render once for each data point (treating data set as empty if shouldDisplay==false)
var dataArray = (shouldDisplay && bindingValue['foreach']) || [];
templateSubscription = ko.renderTemplateForEach(templateName || element, dataArray, /* options: */ bindingValue, element, bindingContext);
templateComputed = ko.renderTemplateForEach(templateName || element, dataArray, /* options: */ bindingValue, element, bindingContext);
} else {
if (shouldDisplay) {
// Render once for this single data point (or use the viewModel if no data was provided)
var innerBindingContext = (typeof bindingValue == 'object') && ('data' in bindingValue)
? bindingContext['createChildContext'](ko.utils.unwrapObservable(bindingValue['data']), bindingValue['as']) // Given an explitit 'data' value, we create a child binding context for it
: bindingContext; // Given no explicit 'data' value, we retain the same binding context
templateSubscription = ko.renderTemplate(templateName || element, innerBindingContext, /* options: */ bindingValue, element);
templateComputed = ko.renderTemplate(templateName || element, innerBindingContext, /* options: */ bindingValue, element);
} else
ko.virtualElements.emptyNode(element);
}

// It only makes sense to have a single template subscription per element (otherwise which one should have its output displayed?)
disposeOldSubscriptionAndStoreNewOne(element, templateSubscription);
// It only makes sense to have a single template computed per element (otherwise which one should have its output displayed?)
disposeOldComputedAndStoreNewOne(element, templateComputed);
}
};

Expand Down
2 changes: 2 additions & 0 deletions src/utils.domData.js
View file
Expand Up @@ -33,7 +33,9 @@ ko.utils.domData = new (function () {
if (dataStoreKey) {
delete dataStore[dataStoreKey];
node[dataStoreKeyExpandoPropertyName] = null;
return true; // Exposing "did clean" flag purely so specs can infer whether things have been cleaned up as intended
}
return false;
}
}
})();
Expand Down

0 comments on commit cb594d0

Please sign in to comment.