Skip to content

Commit

Permalink
Commit 26.
Browse files Browse the repository at this point in the history 
- Fix for memory leak in jquery.observable.js.
- Some additional unit tests for tag controls.
- A couple of other small bug fixes.
  • Loading branch information
@BorisMoore
BorisMoore committed Jan 19, 2013
1 parent 14bb7ef commit b9f3adc
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 53 deletions.
89 changes: 56 additions & 33 deletions jquery.observable.js
View file
Expand Up @@ -6,7 +6,7 @@
* Copyright 2012, Boris Moore and Brad Olenick
* Released under the MIT License.
*/
// informal pre beta commit counter: 25
// informal pre beta commit counter: 26

// TODO, Array change on leaf. Caching compiled templates.
// TODO later support paths with arrays ~x.y[2].foo, paths with functions on non-leaf tokens: address().street
Expand All @@ -25,16 +25,17 @@

var versionNumber = "v1.0pre",

$viewsSettings = $.views ? $.views.settings : {}, cbBindings, oldLength, _data,
cbBindingsStore = {},
cbBindings, cbBindingsId, oldLength, _data,
$viewsSub = $.views ? $.views.sub: {},
cbBindingKey = 1,
splice = [].splice,
concat = [].concat,
$isArray = $.isArray,
$expando = $.expando,
OBJECT = "object",
propertyChangeStr = $viewsSettings.propChng = $viewsSettings.propChng || "propertyChange",// These two settings can be overridden on settings after loading
arrayChangeStr = $viewsSettings.arrChng = $viewsSettings.arrChng || "arrayChange", // jsRender, and prior to loading jquery.observable.js and/or JsViews
propertyChangeStr = $viewsSub.propChng = $viewsSub.propChng || "propertyChange",// These two settings can be overridden on settings after loading
arrayChangeStr = $viewsSub.arrChng = $viewsSub.arrChng || "arrayChange", // jsRender, and prior to loading jquery.observable.js and/or JsViews
cbBindingsStore = $viewsSub._cbBnds = $viewsSub._cbBnds || {},
observeStr = propertyChangeStr + ".observe",
$isFunction = $.isFunction,
observeObjKey = 1,
Expand Down Expand Up @@ -103,6 +104,18 @@
return out;
}

function removeCbBindings(cbBindings, cbBindingsId) {
var cb, found;

for(cb in cbBindings) {
found = true;
break;
}
if (!found) {
delete cbBindingsStore[cbBindingsId];
}
}

function onObservableChange(ev, eventArgs) {
var ctx = ev.data;
if (ctx.prop === "*" || ctx.prop === eventArgs.path) {
Expand All @@ -116,18 +129,6 @@
}
}

$.event.special.propertyChange = {
// The jQuery 'off' method does not provide the event data from the event(s) that are being unbound, so we register
// a jQuery special 'remove' event, and get the data.cb._bnd from the event here and provide it in the
// cbBindings var to the unobserve handler, so we can immediately remove this object from that cb._bnd collection, after 'unobserving'.
remove: function(evData) {
if ((evData = evData.data) && (evData = evData.cb)) {
// Get the cb._bnd from the ev.data object
cbBindings = cbBindingsStore[evData._bnd];
}
}
};

function $observe() {
// $.observable.observe(root, [1 or more objects, path or path Array params...], callback[, resolveDependenciesCallback][, unobserveOrOrigRoot)
function observeOnOff(namespace, pathStr) {
Expand All @@ -140,7 +141,9 @@
// jQuery off event does not provide the event data, with the callback and we need to remove this object from the cb._bnd collection.
// So we have registered a jQuery special 'remove' event, which stored the cb._bnd in the cbBindings var,
// so we can immediately remove this object from that cb._bnd collection.
delete cbBindings[obIdExpando.obId];
if (cbBindings) {
delete cbBindings[obIdExpando.obId];
}
}
} else {
if (pathStr === "*" && (events = obIdExpando)) {
Expand All @@ -152,10 +155,13 @@
if (data.cb === callback && data.prop !== pathStr) {
$(object).off(namespace + "." + data.prop, onObservableChange);
// We remove this object from that cb._bnd collection (see above).
delete cbBindings[obIdExpando.obId];
if (cbBindings) {
delete cbBindings[obIdExpando.obId];
}
}
}
}
// $(object).on(namespace, null, {linkCtx: linkCtx, root: origRoot, path: pathStr, prop: prop, cb: callback}, onObservableChange);
$($isArray(object) ? [object] : object).on(namespace, null, {path: pathStr, prop: prop, cb: callback}, onObservableChange);
if (bindings) {
// Add object to bindings, and add the counter to the jQuery data on the object
Expand All @@ -165,7 +171,7 @@
}
}

var i, parts, prop, path, dep, object, unobserve, callback, cbNs, el, data, events, filter, items, bindings, obIdExpando, depth,
var i, parts, prop, path, dep, object, unobserve, callback, cbId, el, data, events, filter, items, bindings, obIdExpando, depth,
topLevel = 1,
ns = observeStr,
paths = concat.apply([], arguments), // flatten the arguments
Expand Down Expand Up @@ -197,25 +203,20 @@
l--;
}

// Use a unique namespace, cbNs, (e.g. obs7) associated with each observe() callback to allow unobserve to
// Use a unique namespace (e.g. obs7) associated with each observe() callback to allow unobserve to
// remove onObservableChange handlers that wrap that callback
ns += unobserve
? (callback ? "." + callback.cbNs : "")
: "." + (callback.cbNs = callback.cbNs || "obs" + observeCbKey++);
cbNs = callback && callback.cbNs;
? (callback ? ".obs" + callback._bnd: "")
: ".obs" + (cbId = callback._bnd = callback._bnd || observeCbKey++);
// : ".obs" + (cbId = callback._bnd = observeCbKey++);

if (unobserve && l === 0 && root) {
// unobserve(object) TODO: What if there is a callback specified?
$(root).off(observeStr, onObservableChange);
}
if (callback) {
bindings = callback._bnd = callback._bnd ||
// This will be a top-level call for a new callback
cbBindingKey++;

bindings = cbBindingsStore[bindings] = cbBindingsStore[bindings] || {};
if (!unobserve) {
bindings = cbBindingsStore[cbId] = cbBindingsStore[cbId] || {};
}

depth = 0;
for (i = 0; i < l; i++) {
path = paths[i];
Expand Down Expand Up @@ -299,8 +300,12 @@
object = prop;
}
}
// Return the bindings to the top-level caller, along with the cbNs
return { cbNs: cbNs, bnd: bindings };
if (cbId) {
removeCbBindings(bindings, cbId);
}

// Return the bindings to the top-level caller, along with the cbId
return { cbId: cbId, bnd: bindings };
}

function $unobserve() {
Expand Down Expand Up @@ -478,4 +483,22 @@
$data.triggerHandler(propertyChangeStr, {path: "length", value: _data.length, oldValue: oldLength});
}
};

$.event.special.propertyChange = {
// The jQuery 'off' method does not provide the event data from the event(s) that are being unbound, so we register
// a jQuery special 'remove' event, and get the data.cb._bnd from the event here and provide it in the
// cbBindings var to the unobserve handler, so we can immediately remove this object from that cb._bnd collection, after 'unobserving'.
remove: function(evData) {
if ((evData = evData.data) && (evData = evData.cb)) {
// Get the cb._bnd from the ev.data object
cbBindings = cbBindingsStore[cbBindingsId = evData._bnd];
}
},
teardown: function(namespaces) {
if (cbBindings) {
delete cbBindings[this[$expando].obId];
removeCbBindings(cbBindings, cbBindingsId);
}
}
};
})(this, this.jQuery || this.jsviews);
36 changes: 20 additions & 16 deletions jquery.views.js
View file
Expand Up @@ -7,7 +7,7 @@
* Copyright 2012, Boris Moore
* Released under the MIT License.
*/
// informal pre beta commit counter: 25
// informal pre beta commit counter: 26

(function(global, $, undefined) {
// global is the this object, which is window when running in the usual browser environment.
Expand Down Expand Up @@ -50,7 +50,8 @@
jsvAttrStr = "data-jsv",
$viewsLinkAttr = $viewsSettings.linkAttr || "data-link", // Allows override on settings prior to loading jquery.views.js
propertyChangeStr = $viewsSettings.propChng = $viewsSettings.propChng || "propertyChange",// These two settings can be overridden on settings after loading
arrayChangeStr = $viewsSettings.arrChng = $viewsSettings.arrChng || "arrayChange", // jsRender, and prior to loading jquery.observable.js and/or JsViews
arrayChangeStr = $viewsSub.arrChng = $viewsSub.arrChng || "arrayChange", // jsRender, and prior to loading jquery.observable.js and/or JsViews
cbBindingsStore = $viewsSub._cbBnds = $viewsSub._cbBnds || {},
elementChangeStr = "change.jsv",
onBeforeChangeStr = "onBeforeChange",
onAfterChangeStr = "onAfterChange",
Expand Down Expand Up @@ -277,12 +278,14 @@
setter = fnSetters[attr];

if (setter) {
if (changed = $target[setter]() !== sourceValue) {
// TODO support for testing whether {^{: or {^{tag have changed or not
if (changed = tag || $target[setter]() !== sourceValue) {
// TODO support for testing whether {^{: or {^{tag have changed or not. Currently always true, since sourceValue has not been converted yet by convertMarkers
if (attr === "html") {
if (tag) {
elCnt = tag._elCnt;
if (tag._.inline) {
if (!tag.flow && !tag.render && !tag.template) {
targetElem = target;
} else if (tag._.inline) {
var nodesToRemove = tag.nodes(TRUE);

if (elCnt && prevNode && prevNode !== nextNode) {
Expand Down Expand Up @@ -1104,8 +1107,9 @@
linkInfo = elem[1];
elem = elem[0];
if (linkInfo) {
tag = bindingStore[linkInfo.id]; // The tag was stored temporarily on the bindingStore
tag = bindingStore[linkInfo.id];
tag = tag.linkCtx ? tag.linkCtx.tag : tag;
// The tag may have been stored temporarily on the bindingStore - or may have already been replaced by the actual binding
if (linkInfo.open) {
// This is an 'open bound tag' script marker node for a data-bound tag {^{...}}
tag.parentElem = elem.parentNode;
Expand Down Expand Up @@ -1136,8 +1140,9 @@

if (boundTagId) {
// {^{...}} bound tag. So only one linkTag in linkMarkup
tag = bindingStore[boundTagId]; // The tag was stored temporarily on the viewStore
tag = bindingStore[boundTagId];
tag = tag.linkCtx ? tag.linkCtx.tag : tag;
// The tag may have been stored temporarily on the bindingStore - or may have already been replaced by the actual binding
linkMarkup = delimOpenChar1 + tag.tagName + " " + tag.tagCtx.params + delimCloseChar0;
}
if (linkMarkup && node) {
Expand Down Expand Up @@ -1368,7 +1373,7 @@
? (prevNode === self._nxt
? self.parentElem.lastSibling
: prevNode)
: (self._inline === FALSE
: (self._.inline === FALSE
? prevNode || self.linkCtx.elem.firstChild
: prevNode && prevNode.nextSibling);

Expand Down Expand Up @@ -1463,7 +1468,7 @@
binding = bindingStore[bindId];
if (binding) {
for (objId in binding.bnd) {
$($.isArray(object = binding.bnd[objId]) ? [object] : object).off(propertyChangeStr + "." + binding.cbNs);
$($.isArray(object = binding.bnd[objId]) ? [object] : object).off(propertyChangeStr + ".obs" + binding.cbId);
delete binding.bnd[objId];
}

Expand All @@ -1478,7 +1483,8 @@
}
}
delete linkCtx.view._.bnd[bindId];
delete bindingStore[bindId];
delete bindingStore[bindId]
delete $viewsSub._cbBnds[binding.cbId];
}
}

Expand Down Expand Up @@ -1621,7 +1627,7 @@
//===============

$viewsSub.viewInfos = viewInfos;
// Expose as public helper
// Expose viewInfos() as public helper method

//====================================
// Additional members for linked views
Expand Down Expand Up @@ -1650,11 +1656,9 @@
}
}
if (!emptyView) {
if (vwInfo.ch === "_") {
viewOrTag = viewStore[viewId];
} else {
viewOrTag = bindingStore[viewId].linkCtx.tag;
}
viewOrTag = vwInfo.ch === "_"
? viewStore[viewId]
: bindingStore[viewId].linkCtx.tag;
if (vwInfo.open) {
// A "#m" token
viewOrTag._prv = nextNode;
Expand Down
4 changes: 2 additions & 2 deletions jsrender.js
View file
Expand Up @@ -6,7 +6,7 @@
* Copyright 2012, Boris Moore
* Released under the MIT License.
*/
// informal pre beta commit counter: 25
// informal pre beta commit counter: 26

(function(global, jQuery, undefined) {
// global is the this object, which is window when running in the usual browser environment.
Expand Down Expand Up @@ -416,7 +416,7 @@

if (linkCtx) {
linkCtx.tag = tag;
tag._.linkCtx = linkCtx;
tag.linkCtx = linkCtx;
}
if (render = render || tag.render) {
ret = render.apply(tag, args);
Expand Down

0 comments on commit b9f3adc

Please sign in to comment.