diff --git a/externs/closure-types.js b/externs/closure-types.js index ed4da846b8..d8d4531337 100644 --- a/externs/closure-types.js +++ b/externs/closure-types.js @@ -45,12 +45,22 @@ Polymer_PropertyAccessors.prototype.__dataOld; /** @type {Object} */ Polymer_PropertyAccessors.prototype.__dataProto; -/** @type {Object} */ +/** @type {Object} */ Polymer_PropertyAccessors.prototype.__dataHasAccessor; /** @type {Object} */ Polymer_PropertyAccessors.prototype.__dataInstanceProps; +/** + * @const + * @type {{ + * NONE: number, + * SYNTHETIC: number, + * PRE_DEFINED: number + * }} + */ +Polymer_PropertyAccessors.prototype.ACCESSOR_TYPE; + /** * @param {string} name Name of attribute that changed * @param {?string} old Old attribute value @@ -317,6 +327,19 @@ Polymer_PropertyEffects.prototype.__readOnly; /** @type {!TemplateInfo} */ Polymer_PropertyEffects.prototype.__templateInfo; +/** + * @const + * @type {{ + * COMPUTE: string, + * REFLECT: string, + * NOTIFY: string, + * PROPAGATE: string, + * OBSERVE: string, + * READ_ONLY: string + * }} + */ +Polymer_PropertyEffects.prototype.PROPERTY_EFFECT_TYPES; + /** * @override * @param {!HTMLTemplateElement} template Template to stamp diff --git a/lib/mixins/property-accessors.html b/lib/mixins/property-accessors.html index c1977997cd..bd1a421e82 100644 --- a/lib/mixins/property-accessors.html +++ b/lib/mixins/property-accessors.html @@ -70,6 +70,13 @@ } } + /** @enum */ + const AccessorType = { + NONE: 0, + SYNTHETIC: 1, + PRE_DEFINED: 2 + }; + /** * Element class mixin that provides basic meta-programming for creating one * or more property accessors (getter/setter pair) that enqueue an async @@ -138,13 +145,17 @@ this.__dataOld; /** @type {Object} */ this.__dataProto; - /** @type {Object} */ + /** @type {Object} */ this.__dataHasAccessor; /** @type {Object} */ this.__dataInstanceProps; this._initializeProperties(); } + get ACCESSOR_TYPE() { + return AccessorType; + } + /** * Implements native Custom Elements `attributeChangedCallback` to * set an attribute value to a property via `_attributeToProperty`. @@ -413,21 +424,51 @@ if (!this.hasOwnProperty('__dataHasAccessor')) { this.__dataHasAccessor = Object.assign({}, this.__dataHasAccessor); } - if (!this.__dataHasAccessor[property]) { - this.__dataHasAccessor[property] = true; + if ((this.__dataHasAccessor[property] || AccessorType.NONE) === AccessorType.NONE) { + this.__dataHasAccessor[property] = AccessorType.SYNTHETIC; saveAccessorValue(this, property); - Object.defineProperty(this, property, { - /* eslint-disable valid-jsdoc */ + + const existingDescriptor = Object.getOwnPropertyDescriptor(this, property) || {}; + + /** @type {Object} */ + const propertyDefinition = { + enumerable: existingDescriptor.enumerable || false + }; + + // If the property was pre-defined with an es6 getter, + // we just use that and don't need to define our own getter + if (existingDescriptor.get) { + propertyDefinition.get = existingDescriptor.get; + this.__dataHasAccessor[property] = AccessorType.PRE_DEFINED; + } else { + // eslint-disable-next-line valid-jsdoc /** @this {PropertyAccessors} */ - get: function() { + propertyDefinition.get = function() { return this.__data[property]; - }, + }; + } + + // If the property was pre-defined with an es6 setter, + // use that, but also call our own _setProperty function + // so that change notifications are propagated + if (existingDescriptor.set) { + const existingSetter = /** @type {{set: function(*)}} */ (existingDescriptor).set.bind(this); + // eslint-disable-next-line valid-jsdoc /** @this {PropertyAccessors} */ - set: readOnly ? function() {} : function(value) { + propertyDefinition.set = function(value) { + // Call the predefined setter + existingSetter(value); + // Invalidate properties if changed - includes dirty checking this._setProperty(property, value); - } - /* eslint-enable */ - }); + }; + } else if (this.__dataHasAccessor[property] !== AccessorType.PRE_DEFINED) { + // eslint-disable-next-line valid-jsdoc + propertyDefinition.set = readOnly ? function() {} : /** @this {PropertyAccessors} */ function(value) { + this._setProperty(property, value); + }; + } + + Object.defineProperty(this, property, propertyDefinition); } } @@ -438,7 +479,8 @@ * @return {boolean} True if an accessor was created */ _hasAccessor(property) { - return this.__dataHasAccessor && this.__dataHasAccessor[property]; + return this.__dataHasAccessor !== undefined && + (this.__dataHasAccessor[property] || AccessorType.NONE) !== AccessorType.NONE; } /** diff --git a/lib/mixins/property-effects.html b/lib/mixins/property-effects.html index be7118c239..6ac0f93474 100644 --- a/lib/mixins/property-effects.html +++ b/lib/mixins/property-effects.html @@ -1359,7 +1359,7 @@ return true; } } else { - if (this.__dataHasAccessor && this.__dataHasAccessor[path]) { + if (this.__dataHasAccessor && this.__dataHasAccessor[/** @type {string} */ (path)]) { return this._setPendingProperty(/**@type{string}*/(path), value, shouldNotify); } else { this[path] = value; @@ -2260,6 +2260,15 @@ if (!wasPreBound) { for (let prop in templateInfo.propertyEffects) { this._createPropertyAccessor(prop); + + // For any predefined getters without setters, add a read only effect + // so that change notifications don't attempt to update the data cache + if (this.ACCESSOR_TYPE && this.__dataHasAccessor[prop] === this.ACCESSOR_TYPE.PRE_DEFINED) { + const descriptor = Object.getOwnPropertyDescriptor(this, prop); + if (descriptor && !descriptor.set) { + this._createReadOnlyProperty(prop, false); + } + } } } if (instanceBinding) { diff --git a/test/unit/property-accessors.html b/test/unit/property-accessors.html index 9ceebfbbdb..946c269753 100644 --- a/test/unit/property-accessors.html +++ b/test/unit/property-accessors.html @@ -16,14 +16,17 @@ - + @@ -47,10 +52,25 @@ assert.ok(Object.getOwnPropertyDescriptor(window.XFoo.prototype, 'prop2')); }); + test('createPropertiesForAttributes preserves existing getters', function() { + assert.ok(Object.getOwnPropertyDescriptor(window.XFoo.prototype, 'prop3') + .get.toString().indexOf('() { return this.prop3_; }') > 0); + assert.ok(Object.getOwnPropertyDescriptor(window.XFoo.prototype, 'prop4') + .get.toString().indexOf("() { return 'prop4' + (new Date()).getTime(); }") > 0); + }); + + test('createPropertiesForAttributes properly handles class setters', function() { + assert.ok(Object.getOwnPropertyDescriptor(window.XFoo.prototype, 'prop3').set); + assert.ok(Object.getOwnPropertyDescriptor(window.XFoo.prototype, 'prop3') + .set.toString().indexOf('(value) { this.prop3_ = value; }') < 0); + assert.equal(Object.getOwnPropertyDescriptor(window.XFoo.prototype, 'prop4').set, undefined); + }); + test('attributes reflected to properties via upgrade', function() { var el = document.querySelector('x-foo'); assert.equal(el.prop1, 'a'); assert.equal(el.prop2, 'b'); + assert.equal(el.prop3, 'c'); }); test('setting properties results in _propertiesChanged', function(done) { @@ -58,22 +78,28 @@ document.body.appendChild(el); el.prop1 = 'a'; el.prop2 = 'b'; + el.prop3 = 'c'; assert.equal(el._propertiesChanged.callCount, 0, '_propertiesChanged is not async'); setTimeout(function() { assert.isTrue(el._propertiesChanged.calledOnce); assert.equal(el._propertiesChanged.getCall(0).args[0].prop1, 'a'); assert.equal(el._propertiesChanged.getCall(0).args[0].prop2, 'b'); + assert.equal(el._propertiesChanged.getCall(0).args[0].prop3, 'c'); assert.equal(el._propertiesChanged.getCall(0).args[1].prop1, 'a'); assert.equal(el._propertiesChanged.getCall(0).args[1].prop2, 'b'); + assert.equal(el._propertiesChanged.getCall(0).args[1].prop3, 'c'); assert.equal(el._propertiesChanged.getCall(0).args[2].prop1, undefined); assert.equal(el._propertiesChanged.getCall(0).args[2].prop2, undefined); + assert.equal(el._propertiesChanged.getCall(0).args[2].prop3, undefined); assert.equal(el.prop1, 'a'); assert.equal(el.prop2, 'b'); + assert.equal(el.prop3, 'c'); el.prop1 = 'aa'; setTimeout(function() { assert.isTrue(el._propertiesChanged.calledTwice); assert.equal(el._propertiesChanged.getCall(1).args[0].prop1, 'aa'); assert.equal(el._propertiesChanged.getCall(1).args[0].prop2, 'b'); + assert.equal(el._propertiesChanged.getCall(1).args[0].prop3, 'c'); assert.equal(el._propertiesChanged.getCall(1).args[1].prop1, 'aa'); assert.equal(el._propertiesChanged.getCall(1).args[2].prop1, 'a'); assert.isFalse('prop2' in el._propertiesChanged.getCall(1).args[1]); @@ -88,25 +114,33 @@ document.body.appendChild(el); el.setAttribute('prop1', 'a'); el.setAttribute('prop2', 'b'); + el.setAttribute('prop3', 'c'); setTimeout(function() { assert.isTrue(el._propertiesChanged.calledOnce); assert.equal(el._propertiesChanged.getCall(0).args[0].prop1, 'a'); assert.equal(el._propertiesChanged.getCall(0).args[0].prop2, 'b'); + assert.equal(el._propertiesChanged.getCall(0).args[0].prop3, 'c'); assert.equal(el._propertiesChanged.getCall(0).args[1].prop1, 'a'); assert.equal(el._propertiesChanged.getCall(0).args[1].prop2, 'b'); + assert.equal(el._propertiesChanged.getCall(0).args[1].prop3, 'c'); assert.equal(el._propertiesChanged.getCall(0).args[2].prop1, undefined); assert.equal(el._propertiesChanged.getCall(0).args[2].prop2, undefined); + assert.equal(el._propertiesChanged.getCall(0).args[2].prop3, undefined); assert.equal(el.prop1, 'a'); assert.equal(el.prop2, 'b'); + assert.equal(el.prop3, 'c'); el.setAttribute('prop1', 'aa'); setTimeout(function() { assert.isTrue(el._propertiesChanged.calledTwice); assert.equal(el._propertiesChanged.getCall(1).args[0].prop1, 'aa'); assert.equal(el._propertiesChanged.getCall(1).args[0].prop2, 'b'); + assert.equal(el._propertiesChanged.getCall(1).args[0].prop3, 'c'); assert.equal(el._propertiesChanged.getCall(1).args[1].prop1, 'aa'); assert.equal(el._propertiesChanged.getCall(1).args[2].prop1, 'a'); assert.isFalse('prop2' in el._propertiesChanged.getCall(1).args[1]); assert.isFalse('prop2' in el._propertiesChanged.getCall(1).args[2]); + assert.isFalse('prop3' in el._propertiesChanged.getCall(1).args[1]); + assert.isFalse('prop3' in el._propertiesChanged.getCall(1).args[2]); done(); }); diff --git a/test/unit/property-effects-elements.html b/test/unit/property-effects-elements.html index c44cae2435..81008f8512 100644 --- a/test/unit/property-effects-elements.html +++ b/test/unit/property-effects-elements.html @@ -1025,4 +1025,26 @@ this.xChanged = sinon.spy(); } }); - \ No newline at end of file + + + + + \ No newline at end of file diff --git a/test/unit/property-effects.html b/test/unit/property-effects.html index bae8126a4d..e60dac6072 100644 --- a/test/unit/property-effects.html +++ b/test/unit/property-effects.html @@ -1760,6 +1760,51 @@ }); +suite('ES6 getters/setters', function() { + test('notify path bypasses internal cache to update template', function(done) { + let el = document.createElement('x-getters-setters'); + el.prop1 = 'prop1'; + document.body.appendChild(el); + setTimeout(() => { + let originalProp2 = el.$.prop2.innerText; + assert.equal(el.$.prop1.innerText, 'prop1'); + assert.equal(originalProp2.indexOf('prop2'), 0); + assert.equal(el.$.addProps.innerText.indexOf('prop1 prop2'), 0); + assert.equal(el.$.prop1TwoWay.value, 'prop1'); + let originalProp2TwoWay = el.$.prop2TwoWay.value; + assert.equal(originalProp2TwoWay.indexOf('prop2'), 0); + + el.internal_prop1_ = 'prop1changed'; + el.notifyPath('prop1'); + el.notifyPath('prop2'); + setTimeout(() => { + assert.equal(el.$.prop1.innerText, 'prop1changed'); + assert.equal(el.$.addProps.innerText.indexOf('prop1changed prop2'), 0); + assert.equal(el.$.prop1TwoWay.value, 'prop1changed'); + assert.notEqual(el.$.prop2.innerText, originalProp2); + assert.notEqual(el.$.prop2TwoWay.value, originalProp2TwoWay); + done(); + }); + }); + }); + + test('setters update template', function(done) { + let el = document.createElement('x-getters-setters'); + el.prop1 = 'prop1'; + document.body.appendChild(el); + setTimeout(() => { + assert.equal(el.$.prop1.innerText, 'prop1'); + assert.equal(el.$.addProps.innerText.indexOf('prop1 prop2'), 0); + + el.prop1 = 'prop1changed'; + setTimeout(() => { + assert.equal(el.$.prop1.innerText, 'prop1changed'); + assert.equal(el.$.addProps.innerText.indexOf('prop1changed prop2'), 0); + done(); + }); + }); + }); +});