From c1885a6a94a2592dd6dc33dd834a9934f8724cea Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Tue, 6 Feb 2018 14:42:03 -0800 Subject: [PATCH 1/3] [PropertiesChanged]: adds _shouldPropertiesChange Adds `_shouldPropertiesChange` method which can be overridden to customize when the `_propertiesChanged` method is called. This method gets the current, pending, and old property values and by default returns truthy... --- lib/mixins/properties-changed.html | 26 +++++++++++++++++++---- test/unit/properties-changed.html | 33 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/lib/mixins/properties-changed.html b/lib/mixins/properties-changed.html index aca31674af..ac5481b131 100644 --- a/lib/mixins/properties-changed.html +++ b/lib/mixins/properties-changed.html @@ -143,7 +143,7 @@ /* eslint-disable valid-jsdoc */ /** @this {PropertiesChanged} */ get() { - return this.__data[property]; + return this._getProperty(property); }, /** @this {PropertiesChanged} */ set: readOnly ? function () {} : function (value) { @@ -332,13 +332,31 @@ * @protected */ _flushProperties() { - if (this.__dataPending && this.__dataOld) { - let changedProps = this.__dataPending; + const props = this.__data; + const changedProps = this.__dataPending; + const old = this.__dataOld; + if (this._shouldPropertiesChange(props, changedProps, old)) { this.__dataPending = null; - this._propertiesChanged(this.__data, changedProps, this.__dataOld); + this._propertiesChanged(props, changedProps, old); } } + /** + * Called in `_flushProperties` to determine if `_propertiesChanged` + * should be called. The default implementation returns true if + * properties are pending. Override to customize when + * `_propertiesChanged` is called. + * @param {!Object} currentProps Bag of all current accessor values + * @param {!Object} changedProps Bag of properties changed since the last + * call to `_propertiesChanged` + * @param {!Object} oldProps Bag of previous values for each property + * in `changedProps` + * @return {boolean} true if changedProps is truthy + */ + _shouldPropertiesChange(props, changedProps, old) { + return changedProps; + } + /** * Callback called when any properties with accessors created via * `_createPropertyAccessor` have been set. diff --git a/test/unit/properties-changed.html b/test/unit/properties-changed.html index dc835d7ad8..eebb662706 100644 --- a/test/unit/properties-changed.html +++ b/test/unit/properties-changed.html @@ -56,6 +56,24 @@ } window.XFoo = XFoo; customElements.define('x-foo', XFoo); + + class ShouldPropertiesChange extends Polymer.PropertiesChanged(HTMLElement) { + constructor() { + super(); + this._propertiesChanged = sinon.spy(); + } + + connectedCallback() { + this._enableProperties(); + //this._flushProperties(); + } + + _shouldPropertiesChange() { + return true; + } + } + + customElements.define('should-properties-change', ShouldPropertiesChange); }); @@ -129,6 +147,21 @@ }); }); + test('shouldPropertiesChange', function(done) { + const el = document.createElement('should-properties-change'); + document.body.appendChild(el); + assert.isTrue(el._propertiesChanged.calledOnce); + el._invalidateProperties(); + setTimeout(function() { + assert.isTrue(el._propertiesChanged.calledTwice); + el._setProperty('foo', true); + setTimeout(function() { + assert.isTrue(el._propertiesChanged.calledThrice); + done(); + }); + }); + }) + }); From 5607a2d8c317eb381f51b85bcfb26e5d0c8ab576 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Tue, 6 Feb 2018 14:48:36 -0800 Subject: [PATCH 2/3] Lint and type fixes --- externs/closure-types.js | 9 +++++++++ lib/mixins/properties-changed.html | 2 +- test/unit/properties-changed.html | 3 +-- types/lib/mixins/properties-changed.d.ts | 15 +++++++++++++++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/externs/closure-types.js b/externs/closure-types.js index 7db4746320..0cabb51658 100644 --- a/externs/closure-types.js +++ b/externs/closure-types.js @@ -78,6 +78,15 @@ Polymer_PropertiesChanged.prototype._enableProperties = function(){}; Polymer_PropertiesChanged.prototype._flushProperties = function(){}; /** * @param {!Object} currentProps Bag of all current accessor values +* @param {!Object} changedProps Bag of properties changed since the last + call to `_propertiesChanged` +* @param {!Object} oldProps Bag of previous values for each property + in `changedProps` +* @return {boolean} +*/ +Polymer_PropertiesChanged.prototype._shouldPropertiesChange = function(currentProps, changedProps, oldProps){}; +/** +* @param {!Object} currentProps Bag of all current accessor values * @param {!Object} changedProps Bag of properties changed since the last call to `_propertiesChanged` * @param {!Object} oldProps Bag of previous values for each property diff --git a/lib/mixins/properties-changed.html b/lib/mixins/properties-changed.html index ac5481b131..203874d523 100644 --- a/lib/mixins/properties-changed.html +++ b/lib/mixins/properties-changed.html @@ -353,7 +353,7 @@ * in `changedProps` * @return {boolean} true if changedProps is truthy */ - _shouldPropertiesChange(props, changedProps, old) { + _shouldPropertiesChange(currentProps, changedProps, oldProps) { // eslint-disable-line no-unused-vars return changedProps; } diff --git a/test/unit/properties-changed.html b/test/unit/properties-changed.html index eebb662706..9ee8417f35 100644 --- a/test/unit/properties-changed.html +++ b/test/unit/properties-changed.html @@ -160,8 +160,7 @@ done(); }); }); - }) - + }); }); diff --git a/types/lib/mixins/properties-changed.d.ts b/types/lib/mixins/properties-changed.d.ts index e7ecd9c5e1..02555dcb96 100644 --- a/types/lib/mixins/properties-changed.d.ts +++ b/types/lib/mixins/properties-changed.d.ts @@ -182,6 +182,21 @@ declare namespace Polymer { */ _flushProperties(): void; + /** + * Called in `_flushProperties` to determine if `_propertiesChanged` + * should be called. The default implementation returns true if + * properties are pending. Override to customize when + * `_propertiesChanged` is called. + * + * @param currentProps Bag of all current accessor values + * @param changedProps Bag of properties changed since the last + * call to `_propertiesChanged` + * @param oldProps Bag of previous values for each property + * in `changedProps` + * @returns true if changedProps is truthy + */ + _shouldPropertiesChange(currentProps: object, changedProps: object, oldProps: object): boolean; + /** * Callback called when any properties with accessors created via * `_createPropertyAccessor` have been set. From 74907b9a10d0ce2fe2bea0da455d553b87df0566 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Wed, 7 Feb 2018 17:15:30 -0800 Subject: [PATCH 3/3] [PropertiesChanged]: allow old data to be gc'd after `_propertiesChanged` --- lib/mixins/properties-changed.html | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/mixins/properties-changed.html b/lib/mixins/properties-changed.html index 203874d523..bd47e08c73 100644 --- a/lib/mixins/properties-changed.html +++ b/lib/mixins/properties-changed.html @@ -337,6 +337,7 @@ const old = this.__dataOld; if (this._shouldPropertiesChange(props, changedProps, old)) { this.__dataPending = null; + this.__dataOld = null; this._propertiesChanged(props, changedProps, old); } }