From c3a882574675e04ca90d69ef43ec2567a17c8df7 Mon Sep 17 00:00:00 2001 From: herr kaste Date: Wed, 17 Feb 2016 00:52:37 +0100 Subject: [PATCH 1/4] Do not set splices property on userland arrays. --- src/lib/bind/effects.html | 12 ++++++++++-- src/standard/notify-path.html | 8 ++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/lib/bind/effects.html b/src/lib/bind/effects.html index 52ed42ed87..9eed075296 100644 --- a/src/lib/bind/effects.html +++ b/src/lib/bind/effects.html @@ -130,10 +130,18 @@ var v; if (arg.literal) { v = arg.value; - } else if (arg.structured) { - v = Polymer.Base._get(name, model); } 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; diff --git a/src/standard/notify-path.html b/src/standard/notify-path.html index 0174974c44..49588d0a21 100644 --- a/src/standard/notify-path.html +++ b/src/standard/notify-path.html @@ -438,14 +438,10 @@ 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); this._notifyPath(path + '.length', array.length); - // Null here to allow potentially large splice records to be GC'ed + // All path notification values are cached on `this.__data__`. + // Null here to allow potentially large splice records to be GC'ed. change.keySplices = null; change.indexSplices = null; }, From bd2d6a0677c849673d0ffd064d70174c2f5b49fb Mon Sep 17 00:00:00 2001 From: herr kaste Date: Fri, 19 Feb 2016 15:06:17 +0100 Subject: [PATCH 2/4] Use value sent from notification if appropriate. --- src/lib/bind/effects.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib/bind/effects.html b/src/lib/bind/effects.html index 9eed075296..7f23e47fa9 100644 --- a/src/lib/bind/effects.html +++ b/src/lib/bind/effects.html @@ -130,6 +130,8 @@ var v; if (arg.literal) { v = arg.value; + } 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 From dc180fb5ddf71850e97637ee7609ceaf588bd725 Mon Sep 17 00:00:00 2001 From: herr kaste Date: Fri, 19 Feb 2016 15:07:10 +0100 Subject: [PATCH 3/4] Don't mutate splices change record sent to userland. --- src/standard/notify-path.html | 6 +++--- test/unit/notify-path-elements.html | 1 + test/unit/notify-path.html | 10 ++++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/standard/notify-path.html b/src/standard/notify-path.html index 49588d0a21..2f212c1554 100644 --- a/src/standard/notify-path.html +++ b/src/standard/notify-path.html @@ -438,12 +438,12 @@ keySplices: Polymer.Collection.applySplices(array, splices), indexSplices: splices }; - this._notifyPath(path + '.splices', change); + var splicesPath = path + '.splices'; + this._notifyPath(splicesPath, change); this._notifyPath(path + '.length', array.length); // All path notification values are cached on `this.__data__`. // Null here to allow potentially large splice records to be GC'ed. - change.keySplices = null; - change.indexSplices = null; + this.__data__[splicesPath] = {keySplices: null, indexSplices: null}; }, _notifySplice: function(array, path, index, added, removed) { diff --git a/test/unit/notify-path-elements.html b/test/unit/notify-path-elements.html index 57b0fb3bdb..363d1af4f6 100644 --- a/test/unit/notify-path-elements.html +++ b/test/unit/notify-path-elements.html @@ -209,6 +209,7 @@ assert.equal(splices.keySplices[0].added.length, 3, 'arrayChanged keySplices incorrect'); } + this.arraySplices = splices; }, arrayChangedDeep: function() { }, arrayNoCollChanged: function(splices) { diff --git a/test/unit/notify-path.html b/test/unit/notify-path.html index e32853b187..4df283d0e4 100644 --- a/test/unit/notify-path.html +++ b/test/unit/notify-path.html @@ -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 From 970d5763f87d21c41c90189c178e0a3882a84862 Mon Sep 17 00:00:00 2001 From: herr kaste Date: Fri, 19 Feb 2016 15:29:42 +0100 Subject: [PATCH 4/4] Simplify conditional expression. --- src/lib/bind/effects.html | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lib/bind/effects.html b/src/lib/bind/effects.html index 7f23e47fa9..b8d0f4cf50 100644 --- a/src/lib/bind/effects.html +++ b/src/lib/bind/effects.html @@ -150,9 +150,11 @@ } 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,