Skip to content

Commit

Permalink
Refactorings around how computational expressions get their arguments
Browse files Browse the repository at this point in the history
* Do not set splices property on userland arrays.

* Use value sent from notification if appropriate.

* Don't mutate splices change record sent to userland.

* Simplify conditional expression.
  • Loading branch information
kaste authored and dfreedm committed Apr 5, 2016
1 parent b30f962 commit 677f10c
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 14 deletions.
22 changes: 17 additions & 5 deletions src/lib/bind/effects.html
Original file line number Diff line number Diff line change
Expand Up @@ -130,19 +130,31 @@
var v;
if (arg.literal) {
v = arg.value;
} else if (arg.structured) {
v = Polymer.Base._get(name, model);
} else if (path === name) {
v = value;
} else {
// Take advantage of the fact that all values sent through the path
// notifications system are cached on the model. Trivially, all the
// user properties can be looked up there as well.
v = model[name];
if (v === undefined && arg.structured) {
// All initial values and base assignments don't go through the
// notifications API, so we must construct/evaluate the correct
// value. (E.g. you assign a new object to `this.foo`, but your
// observer actually listens to `foo.bar.quux`)
v = Polymer.Base._get(name, model);
}
}
if (bailoutEarly && v === undefined) {
return;
}
if (arg.wildcard) {
// Only send the actual path changed info if the change that
// caused the observer to run matched the wildcard
var baseChanged = (name.indexOf(path + '.') === 0);
var matches = (effect.trigger.name.indexOf(name) === 0 && !baseChanged);
// caused the observer to run matches this arg. Note that this holds
// also true when `path === name`. We can skip this check b/c then
// `name` and `v` already have the values we want.
// Read the following as: `head(path) === name`.
var matches = path.indexOf(name + '.') === 0;
values[i] = {
path: matches ? path : name,
value: matches ? value : v,
Expand Down
14 changes: 5 additions & 9 deletions src/standard/notify-path.html
Original file line number Diff line number Diff line change
Expand Up @@ -435,16 +435,12 @@
keySplices: Polymer.Collection.applySplices(array, splices),
indexSplices: splices
};
if (!array.hasOwnProperty('splices')) {
Object.defineProperty(array, 'splices',
{configurable: true, writable: true});
}
array.splices = change;
this._notifyPath(path + '.splices', change);
var splicesPath = path + '.splices';
this._notifyPath(splicesPath, change);
this._notifyPath(path + '.length', array.length);
// Null here to allow potentially large splice records to be GC'ed
change.keySplices = null;
change.indexSplices = null;
// All path notification values are cached on `this.__data__`.
// Null here to allow potentially large splice records to be GC'ed.
this.__data__[splicesPath] = {keySplices: null, indexSplices: null};
},

_notifySplice: function(array, path, index, added, removed) {
Expand Down
1 change: 1 addition & 0 deletions test/unit/notify-path-elements.html
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@
assert.equal(splices.keySplices[0].added.length, 3,
'arrayChanged keySplices incorrect');
}
this.arraySplices = splices;
},
arrayChangedDeep: function() { },
arrayNoCollChanged: function(splices) {
Expand Down
10 changes: 10 additions & 0 deletions test/unit/notify-path.html
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,16 @@
assert.equal(Polymer.Collection.get(el.array).getKeys().length, 6);
});

test('ensure splices sent into userland dont get nulled', function(){
el.array = [];
// Ensure keySplices are generated for test purposes
Polymer.Collection.get(el.array);

el.push('array', 1, 2, 3);
assert.notEqual(el.arraySplices.indexSplices, null);
assert.notEqual(el.arraySplices.keySplices, null);
});

test('array.splices notified, multiple args, prop first', function() {
el.array = [];
// Ensure keySplices are generated for test purposes
Expand Down

0 comments on commit 677f10c

Please sign in to comment.