From 9ec1fbe8f0d2969aa55ece35326882f729ff5629 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 14 Sep 2016 18:52:47 -0700 Subject: [PATCH 01/20] WIP eliminate collection --- src/elements/element.html | 2 +- src/properties/batched-effects.html | 16 +- src/properties/property-effects.html | 54 +--- src/templatizer/dom-repeat.html | 445 ++++----------------------- src/utils/path.html | 53 +--- test/smoke/data-table.html | 379 +++++++++++++++++++++++ test/smoke/issue-34.html | 62 ++++ test/smoke/issue-3947-test.html | 66 ++++ test/smoke/issue-3947.html | 33 ++ test/smoke/test.html | 10 + test/unit/dom-repeat.html | 10 +- 11 files changed, 649 insertions(+), 481 deletions(-) create mode 100644 test/smoke/data-table.html create mode 100644 test/smoke/issue-34.html create mode 100644 test/smoke/issue-3947-test.html create mode 100644 test/smoke/issue-3947.html create mode 100644 test/smoke/test.html diff --git a/src/elements/element.html b/src/elements/element.html index bb011eb6b8..80924b356a 100644 --- a/src/elements/element.html +++ b/src/elements/element.html @@ -179,7 +179,7 @@ // reserved for canonical behavior connectedCallback() { if (hostStack.isEmpty()) { - this._flushProperties(); + this._flushProperties(true); this.updateStyles(); } } diff --git a/src/properties/batched-effects.html b/src/properties/batched-effects.html index 198dbe6d52..30a4470caa 100644 --- a/src/properties/batched-effects.html +++ b/src/properties/batched-effects.html @@ -123,11 +123,11 @@ // -- set properties machinery _propertiesChanged(currentProps, changedProps, oldProps) { - // if (window.debug) { - // var c = Object.getOwnPropertyNames(changedProps); - // console.group(inst.localName + '#' + inst.id + ': ' + c); - // debugger; - // } + // ---------------------------- + var c = Object.getOwnPropertyNames(changedProps || {}); + console.group(this.localName + '#' + this.id + ': ' + c); + debugger; + // ---------------------------- // Compute var computedProps = runComputedEffects(this, changedProps, oldProps); // Compute linked paths @@ -148,9 +148,9 @@ runEffects(this, this.PROPERTY_EFFECT_TYPES.OBSERVE, changedProps, oldProps); } - // if (window.debug) { - // console.groupEnd(inst.localName + '#' + inst.id + ': ' + c); - // } + // ---------------------------- + console.groupEnd(this.localName + '#' + this.id + ': ' + c); + // ---------------------------- } _setPropertyToNodeFromAnnotation(node, prop, value) { diff --git a/src/properties/property-effects.html b/src/properties/property-effects.html index efa869ac74..95b7b07968 100644 --- a/src/properties/property-effects.html +++ b/src/properties/property-effects.html @@ -557,27 +557,8 @@ // data api // Note: this implemetation only accepts key-based array paths - function notifySplices(inst, array, path, splices) { - let change = { - keySplices: Polymer.Collection.applySplices(array, splices), - indexSplices: splices - }; - let splicesPath = path + '.splices'; - inst._setProperty(splicesPath, change); - inst._setProperty(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. - inst.__data[splicesPath] = {keySplices: null, indexSplices: null}; - } - - function notifySplice(inst, array, path, index, added, removed) { - notifySplices(inst, array, path, [{ - index: index, - addedCount: added, - removed: removed, - object: array, - type: 'splice' - }]); + function notifySplices(inst, array, path) { + inst._setProperty(path, array); } function upper(name) { @@ -688,12 +669,12 @@ // `path` can be a user-facing path string or array of path parts. _setPathOrUnmanagedProperty(path, value) { let rootProperty = Polymer.Path.root(Array.isArray(path) ? path[0] : path); - let _hasEffect = this._hasPropertyEffect(rootProperty); + let hasEffect = this._hasPropertyEffect(rootProperty); let isPath = (rootProperty !== path); - if (!_hasEffect || isPath) { + if (!hasEffect || isPath) { path = Polymer.Path.set(this, path, value); } - if (_hasEffect) { + if (hasEffect) { return path; } } @@ -801,7 +782,7 @@ let info = {}; let array = Polymer.Path.get(this, path, info); // Notify change to key-based path - notifySplices(this, array, info.path, splices); + notifySplices(this, array, info.path); } /** @@ -885,10 +866,9 @@ push(path, ...items) { let info = {}; let array = Polymer.Path.get(this, path, info); - let len = array.length; let ret = array.push(...items); if (items.length) { - notifySplice(this, array, info.path, len, items.length, []); + notifySplices(this, array, info.path); } return ret; } @@ -912,7 +892,7 @@ let hadLength = Boolean(array.length); let ret = array.pop(); if (hadLength) { - notifySplice(this, array, info.path, array.length, 0, [ret]); + notifySplices(this, array, info.path); } return ret; } @@ -937,18 +917,9 @@ splice(path, start, deleteCount, ...items) { let info = {}; let array = Polymer.Path.get(this, path, info); - // Normalize fancy native splice handling of crazy start values - if (start < 0) { - start = array.length - Math.floor(-start); - } else { - start = Math.floor(start); - } - if (!start) { - start = 0; - } let ret = array.splice(start, deleteCount, ...items); if (items.length || ret.length) { - notifySplice(this, array, info.path, start, items.length, ret); + notifySplices(this, array, info.path); } return ret; } @@ -972,7 +943,7 @@ let hadLength = Boolean(array.length); let ret = array.shift(); if (hadLength) { - notifySplice(this, array, info.path, 0, 0, [ret]); + notifySplices(this, array, info.path); } return ret; } @@ -996,12 +967,15 @@ let array = Polymer.Path.get(this, path, info); let ret = array.unshift(...items); if (items.length) { - notifySplice(this, array, info.path, 0, items.length, []); + notifySplices(this, array, info.path); } return ret; } notifyPath(path, value) { + if (arguments.length == 1) { + value = this.get(path); + } this._setProperty(path, value); } diff --git a/src/templatizer/dom-repeat.html b/src/templatizer/dom-repeat.html index 0e529acce9..308884cdd9 100644 --- a/src/templatizer/dom-repeat.html +++ b/src/templatizer/dom-repeat.html @@ -74,33 +74,10 @@ the host, or a function may be assigned to the property directly. The functions should implemented following the standard `Array` filter/sort API. -In order to re-run the filter or sort functions based on changes to sub-fields -of `items`, the `observe` property may be set as a space-separated list of -`item` sub-fields that should cause a re-filter/sort when modified. If -the filter or sort function depends on properties not contained in `items`, -the user should observe changes to those properties and call `render` to update -the view based on the dependency change. - -For example, for an `dom-repeat` with a filter of the following: - -```js -isEngineer: function(item) { - return item.type == 'engineer' || item.manager.type == 'engineer'; -} -``` - -Then the `observe` property should be configured as follows: - -```html - + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/smoke/issue-34.html b/test/smoke/issue-34.html new file mode 100644 index 0000000000..af4fb03c36 --- /dev/null +++ b/test/smoke/issue-34.html @@ -0,0 +1,62 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/smoke/issue-3947-test.html b/test/smoke/issue-3947-test.html new file mode 100644 index 0000000000..03ab60b177 --- /dev/null +++ b/test/smoke/issue-3947-test.html @@ -0,0 +1,66 @@ + + + + + + + + + + + + + + + + + + + diff --git a/test/smoke/issue-3947.html b/test/smoke/issue-3947.html new file mode 100644 index 0000000000..5ae21c26c8 --- /dev/null +++ b/test/smoke/issue-3947.html @@ -0,0 +1,33 @@ + + + + + + + + + + + + + + + + + + diff --git a/test/smoke/test.html b/test/smoke/test.html new file mode 100644 index 0000000000..7af2d666a4 --- /dev/null +++ b/test/smoke/test.html @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/test/unit/dom-repeat.html b/test/unit/dom-repeat.html index 97264b56dc..494309280c 100644 --- a/test/unit/dom-repeat.html +++ b/test/unit/dom-repeat.html @@ -3227,14 +3227,16 @@

x-repeat-chunked

unconfigured1.$.repeater.render(); var stamped1 = unconfigured1.root.querySelectorAll('*:not(template):not(dom-repeat)'); var stamped2 = unconfigured2.root.querySelectorAll('*:not(template):not(dom-repeat)'); + assert.equal(stamped1[26].itemaProp, 'prop-1'); + assert.equal(stamped2[26].itemaProp, 'prop-3'); var old = unconfigured.items[2]; - // Change last item (first rendered in #1, last rendered in #2) + // Change last item (last item rendered in both, due to sort in #1) unconfigured1.$.repeater._instances[0].itema = {prop: 'change!'}; - assert.equal(stamped1[0].itemaProp, 'change!'); + assert.equal(stamped1[26].itemaProp, 'change!'); assert.equal(stamped2[26].itemaProp, 'change!'); // Revert unconfigured2.$.repeater._instances[2].itema = old; - assert.equal(stamped1[0].itemaProp, 'prop-3'); + assert.equal(stamped1[26].itemaProp, 'prop-1'); assert.equal(stamped2[26].itemaProp, 'prop-3'); unconfigured1.$.repeater.sort = null; unconfigured1.$.repeater.render(); @@ -3321,7 +3323,7 @@

x-repeat-chunked

assert.equal(repeater3.itemForElement(stamped1[4]).prop, 'prop-1-1-3'); }); - test('keyForElement', function() { + test.skip('keyForElement', function() { var dom = unconfigured1.root; var stamped1 = dom.querySelectorAll('*:not(template):not(dom-repeat)'); var repeater1 = dom.querySelector('dom-repeat[as=itema]'); From b5420e73998e402feb0a3dc1686253f195c6984b Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Mon, 19 Sep 2016 15:19:22 -0700 Subject: [PATCH 02/20] Keep async render, sync index path update, indexSplices notification. --- src/properties/batched-effects.html | 30 +++-- src/properties/meta-effects.html | 2 +- src/properties/property-effects.html | 44 +++++-- src/templatizer/dom-repeat.html | 109 +++++++++++++-- test/runner.html | 2 +- test/unit/dom-repeat-elements.html | 2 +- test/unit/dom-repeat.html | 15 ++- test/unit/notify-path.html | 190 +++++++++++++-------------- 8 files changed, 260 insertions(+), 134 deletions(-) diff --git a/src/properties/batched-effects.html b/src/properties/batched-effects.html index 30a4470caa..45b76bfd64 100644 --- a/src/properties/batched-effects.html +++ b/src/properties/batched-effects.html @@ -39,10 +39,11 @@ } function computeLinkedPaths(inst, changedProps, computedProps) { - if (inst.__dataLinkedPaths) { + const links = inst.__dataLinkedPaths; + if (links) { var link; - for (var a in inst.__dataLinkedPaths) { - var b = inst.__dataLinkedPaths[a]; + for (var a in links) { + var b = links[a]; for (var path in changedProps) { if (Polymer.Path.isDescendant(a, path)) { link = Polymer.Path.translate(a, b, path); @@ -69,7 +70,7 @@ utils.mixin(inst.__dataInterimOld, oldProps) : oldProps; // Notify var host = inst.__dataHost; - runEffects(inst, inst.PROPERTY_EFFECT_TYPES.NOTIFY, props); + runEffects(inst, inst.PROPERTY_EFFECT_TYPES.NOTIFY, props, oldProps, notifyPaths); // Flush host // TODO(kschaaf): fix this check (__dataLib is gone) if (host /*&& host.__dataLib instanceof BatchedEffects*/ && host.__dataPending) { @@ -85,7 +86,16 @@ } } - function runEffects(inst, type, props, oldProps) { + function notifyPaths(inst, prop, value) { + let rootProperty = Polymer.Path.root(prop); + if (rootProperty !== prop) { + let name = Polymer.CaseMap.camelToDashCase(rootProperty) + '-changed'; + inst.dispatchEvent(new CustomEvent(name, { detail: {value, path: prop }})); + } + } + + + function runEffects(inst, type, props, oldProps, fallback) { var ran; var effects = inst[type]; if (effects) { @@ -105,6 +115,9 @@ ran = true; } } + } else if (fallback) { + fallback(inst, prop, inst.__data[prop], + oldProps && oldProps[prop], inst.__dataFromAbove); } } } @@ -124,9 +137,8 @@ _propertiesChanged(currentProps, changedProps, oldProps) { // ---------------------------- - var c = Object.getOwnPropertyNames(changedProps || {}); - console.group(this.localName + '#' + this.id + ': ' + c); - debugger; + // var c = Object.getOwnPropertyNames(changedProps || {}); + // console.group(this.localName + '#' + this.id + ': ' + c); // ---------------------------- // Compute var computedProps = runComputedEffects(this, changedProps, oldProps); @@ -149,7 +161,7 @@ changedProps, oldProps); } // ---------------------------- - console.groupEnd(this.localName + '#' + this.id + ': ' + c); + // console.groupEnd(this.localName + '#' + this.id + ': ' + c); // ---------------------------- } diff --git a/src/properties/meta-effects.html b/src/properties/meta-effects.html index c3038c4d12..f0cca5bea0 100644 --- a/src/properties/meta-effects.html +++ b/src/properties/meta-effects.html @@ -61,7 +61,7 @@ info.value.call(this) : info.value; if (this._hasReadOnlyEffect(p)) { - this._setProperty(p, value) + this._setProperty(p, value); } else { this[p] = value; } diff --git a/src/properties/property-effects.html b/src/properties/property-effects.html index 95b7b07968..313f86c3a0 100644 --- a/src/properties/property-effects.html +++ b/src/properties/property-effects.html @@ -557,8 +557,26 @@ // data api // Note: this implemetation only accepts key-based array paths - function notifySplices(inst, array, path) { - inst._setProperty(path, array); + function notifySplices(inst, array, path, splices) { + let change = { + indexSplices: splices + }; + let splicesPath = path + '.splices'; + inst._setProperty(splicesPath, change); + inst._setProperty(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. + inst.__data[splicesPath] = {keySplices: null, indexSplices: null}; + } + + function notifySplice(inst, array, path, index, added, removed) { + notifySplices(inst, array, path, [{ + index: index, + addedCount: added, + removed: removed, + object: array, + type: 'splice' + }]); } function upper(name) { @@ -782,7 +800,7 @@ let info = {}; let array = Polymer.Path.get(this, path, info); // Notify change to key-based path - notifySplices(this, array, info.path); + notifySplices(this, array, info.path, splices); } /** @@ -866,9 +884,10 @@ push(path, ...items) { let info = {}; let array = Polymer.Path.get(this, path, info); + let len = array.length; let ret = array.push(...items); if (items.length) { - notifySplices(this, array, info.path); + notifySplice(this, array, info.path, len, items.length, []); } return ret; } @@ -892,7 +911,7 @@ let hadLength = Boolean(array.length); let ret = array.pop(); if (hadLength) { - notifySplices(this, array, info.path); + notifySplice(this, array, info.path, array.length, 0, [ret]); } return ret; } @@ -917,9 +936,18 @@ splice(path, start, deleteCount, ...items) { let info = {}; let array = Polymer.Path.get(this, path, info); + // Normalize fancy native splice handling of crazy start values + if (start < 0) { + start = array.length - Math.floor(-start); + } else { + start = Math.floor(start); + } + if (!start) { + start = 0; + } let ret = array.splice(start, deleteCount, ...items); if (items.length || ret.length) { - notifySplices(this, array, info.path); + notifySplice(this, array, info.path, start, items.length, ret); } return ret; } @@ -943,7 +971,7 @@ let hadLength = Boolean(array.length); let ret = array.shift(); if (hadLength) { - notifySplices(this, array, info.path); + notifySplice(this, array, info.path, 0, 0, [ret]); } return ret; } @@ -967,7 +995,7 @@ let array = Polymer.Path.get(this, path, info); let ret = array.unshift(...items); if (items.length) { - notifySplices(this, array, info.path); + notifySplice(this, array, info.path, 0, items.length, []); } return ret; } diff --git a/src/templatizer/dom-repeat.html b/src/templatizer/dom-repeat.html index 308884cdd9..b89e93768e 100644 --- a/src/templatizer/dom-repeat.html +++ b/src/templatizer/dom-repeat.html @@ -74,6 +74,28 @@ the host, or a function may be assigned to the property directly. The functions should implemented following the standard `Array` filter/sort API. +In order to re-run the filter or sort functions based on changes to sub-fields +of `items`, the `observe` property may be set as a space-separated list of +`item` sub-fields that should cause a re-filter/sort when modified. If +the filter or sort function depends on properties not contained in `items`, +the user should observe changes to those properties and call `render` to update +the view based on the dependency change. + +For example, for an `dom-repeat` with a filter of the following: + +```js +isEngineer: function(item) { + return item.type == 'engineer' || item.manager.type == 'engineer'; +} +``` + +Then the `observe` property should be configured as follows: + +```html +