From 677f10c02bf372689b810b3be486aac73ec6fe9b Mon Sep 17 00:00:00 2001 From: Herr Kaste Date: Wed, 6 Apr 2016 01:43:21 +0200 Subject: [PATCH] Refactorings around how computational expressions get their arguments * 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. --- src/lib/bind/effects.html | 22 +++++++++++++++++----- src/standard/notify-path.html | 14 +++++--------- test/unit/notify-path-elements.html | 1 + test/unit/notify-path.html | 10 ++++++++++ 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/lib/bind/effects.html b/src/lib/bind/effects.html index 52ed42ed87..b8d0f4cf50 100644 --- a/src/lib/bind/effects.html +++ b/src/lib/bind/effects.html @@ -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, diff --git a/src/standard/notify-path.html b/src/standard/notify-path.html index e5effe80fb..8933ff1c6a 100644 --- a/src/standard/notify-path.html +++ b/src/standard/notify-path.html @@ -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) { 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 d164e4fa30..6d1e02889e 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