Skip to content

Commit

Permalink
Improvements to path API. Fixes #2509.
Browse files Browse the repository at this point in the history
* Allows `set` to take paths with array #keys
* Allows `notifyPath` to take paths with array indices
* Exposes public notifySplices API
  • Loading branch information
kevinpschaaf committed Oct 8, 2015
1 parent 85c23e1 commit 10021cc
Show file tree
Hide file tree
Showing 9 changed files with 178 additions and 40 deletions.
2 changes: 1 addition & 1 deletion src/lib/bind/accessors.html
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@
return function(e, target) {
if (!bogusTest(e, target)) {
if (e.detail && e.detail.path) {
this.notifyPath(this._fixPath(path, property, e.detail.path),
this._notifyPath(this._fixPath(path, property, e.detail.path),
e.detail.value);
} else {
var value = target[property];
Expand Down
8 changes: 6 additions & 2 deletions src/lib/collection.html
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,14 @@
},

getKey: function(item) {
var key;
if (item && typeof item == 'object') {
return '#' + this.omap.get(item);
key = this.omap.get(item);
} else {
return '#' + this.pmap[item];
key = this.pmap[item];
}
if (key != undefined) {
return '#' + key;
}
},

Expand Down
1 change: 1 addition & 0 deletions src/lib/template/array-selector.html
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@
}
} else {
this.unlinkPaths('selected');
this.unlinkPaths('selectedItem');
}
// Initialize selection
if (this.multi) {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/template/dom-if.html
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
// notifying parent.<path> path change on each row
_forwardParentPath: function(path, value) {
if (this._instance) {
this._instance.notifyPath(path, value, true);
this._instance._notifyPath(path, value, true);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/lib/template/dom-repeat.html
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@
// for notifying items.<key-for-inst>.<path> change up to host
_forwardInstancePath: function(inst, path, value) {
if (path.indexOf(this.as + '.') === 0) {
this.notifyPath('items.' + inst.__key__ + '.' +
this._notifyPath('items.' + inst.__key__ + '.' +
path.slice(this.as.length + 1), value);
}
},
Expand All @@ -624,7 +624,7 @@
// notifying parent path change on each inst
_forwardParentPath: function(path, value) {
this._instances.forEach(function(inst) {
inst.notifyPath(path, value, true);
inst._notifyPath(path, value, true);
}, this);
},

Expand All @@ -639,7 +639,7 @@
if (inst) {
if (dot >= 0) {
path = this.as + '.' + path.substring(dot+1);
inst.notifyPath(path, value, true);
inst._notifyPath(path, value, true);
} else {
inst.__setProperty(this.as, value, true);
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/template/templatizer.html
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@
archetype._prepBindings();

// boilerplate code
archetype._notifyPath = this._notifyPathImpl;
archetype._notifyPathUp = this._notifyPathUpImpl;
archetype._scopeElementClass = this._scopeElementClassImpl;
archetype.listen = this._listenImpl;
archetype._showHideChildren = this._showHideChildrenImpl;
Expand Down Expand Up @@ -292,7 +292,7 @@
// _forwardParentPath: function(path, value) { },
// _forwardParentProp: function(prop, value) { },

_notifyPathImpl: function(path, value) {
_notifyPathUpImpl: function(path, value) {
var dataHost = this.dataHost;
var dot = path.indexOf('.');
var root = dot < 0 ? path : path.slice(0, dot);
Expand Down
139 changes: 113 additions & 26 deletions src/standard/notify-path.html
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,12 @@

Polymer.Base._addFeature({
/**
* Notify that a path has changed. For example:
* Notify that a path has changed.
*
* this.item.user.name = 'Bob';
* this.notifyPath('item.user.name', this.item.user.name);
* Example:
*
* this.item.user.name = 'Bob';
* this.notifyPath('item.user.name', this.item.user.name);
*
* @param {string} path Path that should be notified.
* @param {*} value Value of the specified path.
Expand All @@ -79,6 +81,15 @@
* based on a dirty check of whether the new value was already known.
*/
notifyPath: function(path, value, fromAbove) {
// Convert any array indicies to keys before notifying path
var info = {};
path = this.get(path, this, info);
// Notify change to key-based path
this._notifyPath(info.path, value, fromAbove);
},

// Note: this implemetation only accepts key-based array paths
_notifyPath: function(path, value, fromAbove) {
var old = this._propertySetter(path, value);
// manual dirty checking for now...
// NaN is always not equal to itself,
Expand All @@ -94,7 +105,7 @@
// is coming from above already (avoid wasted event dispatch)
if (!fromAbove) {
// TODO(sorvell): should only notify if notify: true?
this._notifyPath(path, value);
this._notifyPathUp(path, value);
}
// console.groupEnd((this.localName || this.dataHost.id + '-' + this.dataHost.dataHost.index) + '#' + (this.id || this.index) + ' ' + path, value);
return true;
Expand Down Expand Up @@ -146,29 +157,56 @@
var array;
var last = parts[parts.length-1];
if (parts.length > 1) {
// Loop over path parts[0..n-2] and dereference
for (var i=0; i<parts.length-1; i++) {
var part = parts[i];
prop = prop[part];
if (array && (parseInt(part) == part)) {
parts[i] = Polymer.Collection.get(array).getKey(prop);
if (array && part[0] == '#') {
// Part was key; lookup item in collection
prop = Polymer.Collection.get(array).getItem(part);
} else {
// Get item from simple property dereference
prop = prop[part];
if (array && (parseInt(part) == part)) {
// Translate array indicies to collection keys for path notificaiton
parts[i] = Polymer.Collection.get(array).getKey(prop);
}
}
if (!prop) {
return;
}
// Cache previous part if it is an array
array = Array.isArray(prop) ? prop : null;
}
if (array && (parseInt(last) == last)) {
// Special handling when last part is a array item: need to replace
// item in collection associated with key for that item
if (array) {
var coll = Polymer.Collection.get(array);
var old = prop[last];
var key = coll.getKey(old);
parts[i] = key;
coll.setItem(key, value);
if (parseInt(last) == last) {
// Dereference index & lookup collection key
var old = prop[last];
var key = coll.getKey(old);
// Translate array indicies to collection keys for path notificaiton
parts[i] = key;
// Replace item associated with key in collection
coll.setItem(key, value);
} else if (last[0] == '#') {
// Part was key; lookup item in collection
var key = last;
var old = coll.getItem(key);
// Update last part from key to index: O(n) lookup unavoidable
last = array.indexOf(old);
// Replace item associated with key in collection
coll.setItem(key, value);
}
}
// Set value to object at end of path
prop[last] = value;
// Notify observers of path change
if (!root) {
this.notifyPath(parts.join('.'), value);
this._notifyPath(parts.join('.'), value);
}
} else {
// Simple property set
prop[path] = value;
}
},
Expand All @@ -189,25 +227,38 @@
* indicies, the index may be used as a dotted part directly
* (e.g. `users.12.name` or `['users', 12, 'name']`).
* @param {Object=} root Root object from which the path is evaluated.
* @param {Object=} info Info object; if supplied, a `path` property
* will be added containing path with array indicies converted to keys
* @return {*} Value at the path, or `undefined` if any part of the path
* is undefined.
*/
get: function(path, root) {
get: function(path, root, info) {
var prop = root || this;
var parts = this._getPathParts(path);
var array;
// Loop over path parts[0..n-1] and dereference
for (var i=0; i<parts.length; i++) {
if (!prop) {
return;
}
var part = parts[i];
if (array && part[0] == '#') {
// Part was key; lookup item in collection
prop = Polymer.Collection.get(array).getItem(part);
} else {
// Part was index; simple dereference
prop = prop[part];
if (info && array && (parseInt(part) == part)) {
// Translate array indicies to collection keys for path notificaiton
parts[i] = Polymer.Collection.get(array).getKey(prop);
}
}
// Cache previous part if it is an array
array = Array.isArray(prop) ? prop : null;
}
if (info) {
info.path = parts.join('.');
}
return prop;
},

Expand Down Expand Up @@ -319,7 +370,7 @@
return property + path.slice(root.length);
},

_notifyPath: function(path, value) {
_notifyPathUp: function(path, value) {
var rootName = this._modelForPath(path);
var dashCaseName = Polymer.CaseMap.camelToDashCase(rootName);
var eventName = dashCaseName + this._EVENT_CHANGED;
Expand All @@ -336,14 +387,42 @@

_EVENT_CHANGED: '-changed',

_notifySplice: function(array, path, index, added, removed) {
var splices = [{
index: index,
addedCount: added,
removed: removed,
object: array,
type: 'splice'
}];
/**
* Notify that an array has changed.
*
* Example:
*
* this.items = [ {name: 'Jim'}, {name: 'Todd'}, {name: 'Bill'} ];
* ...
* this.items.splice(1, 1, {name: 'Sam'});
* this.items.push({name: 'Bob'});
* this.notifySplices('items', [
* { index: 1, removed: [{name: 'Todd'}], addedCount: 1, obect: this.items, type: 'splice' },
* { index: 3, removed: [], addedCount: 1, object: this.items, type: 'splice'}
* ]);
*
* @param {string} path Path that should be notified.
* @param {Array} splices Array of splice records indicating ordered
* changes that occurred to the array. Each record should have the
* following fields:
* * index: index at which the change occurred
* * removed: array of items that were removed from this index
* * addedCount: number of new items added at this index
* * object: a reference to the array in question
* * type: the string literal 'splice'
*
* Note that splice records _must_ be normalized such that they are
* reported in index order (raw results from `Object.observe` are not
* ordered and must be normalized/merged before notifying).
*/
notifySplices: function(path, splices) {
var info = {};
var array = this.get(path, this, info);
// Notify change to key-based path
this._notifySplices(array, info.path, splices);
},

_notifySplices: function(array, path, splices) {
var change = {
keySplices: Polymer.Collection.applySplices(array, splices),
indexSplices: splices
Expand All @@ -353,14 +432,22 @@
{configurable: true, writable: true});
}
this.set(path + '.splices', change);
if (added != removed.length) {
this.notifyPath(path + '.length', array.length);
}
this._notifyPath(path + '.length', array.length);
// Null here to allow potentially large splice records to be GC'ed
change.keySplices = null;
change.indexSplices = null;
},

_notifySplice: function(array, path, index, added, removed) {
this._notifySplices(array, path, [{
index: index,
addedCount: added,
removed: removed,
object: array,
type: 'splice'
}]);
},

/**
* Adds items onto the end of the array at the path specified.
*
Expand Down
10 changes: 5 additions & 5 deletions test/unit/array-selector.html
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
assert.equal(info.path, 'multiSelected.splices');
assert.equal(info.value.keySplices.length, 1);
assert.equal(info.value.keySplices[0].added.length, 1);
assert.equal(info.value.keySplices[0].added[0], 0);
assert.equal(info.value.keySplices[0].added[0], '#0');
assert.equal(info.value.keySplices[0].removed.length, 0);
assert.sameMembers(bind.$.observer.multiSelected, [bind.items[2]]);
}
Expand All @@ -203,7 +203,7 @@
assert.equal(info.path, 'multiSelected.splices');
assert.equal(info.value.keySplices.length, 1);
assert.equal(info.value.keySplices[0].added.length, 1);
assert.equal(info.value.keySplices[0].added[0], 1);
assert.equal(info.value.keySplices[0].added[0], '#1');
assert.equal(info.value.keySplices[0].removed.length, 0);
assert.sameMembers(bind.$.observer.multiSelected, [bind.items[2], bind.items[0]]);
}
Expand All @@ -217,14 +217,14 @@
var called = false;
observer.multiChanged = function(info) {
called = true;
assert.equal(info.path, 'multiSelected.0.name');
assert.equal(info.path, 'multiSelected.#0.name');
assert.equal(info.value, 'test');
assert.equal(bind.$.observer.multiSelected[0].name, 'test');
};
bind.set(['items', 2, 'name'], 'test');
observer.multiChanged = function(info) {
called = true;
assert.equal(info.path, 'multiSelected.1.name');
assert.equal(info.path, 'multiSelected.#1.name');
assert.equal(info.value, 'test2');
assert.equal(bind.$.observer.multiSelected[1].name, 'test2');
};
Expand All @@ -238,7 +238,7 @@
var called = false;
observer.multiChanged = function(info) {
called = true;
assert.equal(info.path, 'multiSelected.1.name');
assert.equal(info.path, 'multiSelected.#1.name');
assert.equal(info.value, 'test3');
assert.equal(bind.$.observer.multiSelected[1].name, 'test3');
};
Expand Down
Loading

0 comments on commit 10021cc

Please sign in to comment.