From cd6dd8a6a432b44451f4e3629fff4be294814ce8 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Tue, 12 Mar 2019 16:06:59 -0700 Subject: [PATCH 1/2] Ensure attribute reflected from property is correct Fixes #592. Ensures a reflecting property set immediately after a corresponding attribute reflects correctly. --- CHANGELOG.md | 2 ++ src/lib/updating-element.ts | 17 ++++++++++------ src/test/lib/updating-element_test.ts | 28 +++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abcd3508..a90ae761 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). * `LitElement.renderRoot` is now `public readonly` instead of `protected`. ### Fixed +* A reflecting property set immediately after a corresponding attribute +now reflects properly ([#592](https://github.com/Polymer/lit-element/issues/592)). * Properties annotated with the `@query` and `@queryAll` decorators will now survive property renaming optimizations when used with tsickle and Closure JS Compiler. diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index abb2fd93..73f9cb14 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -579,16 +579,21 @@ export abstract class UpdatingElement extends HTMLElement { */ requestUpdate(name?: PropertyKey, oldValue?: unknown) { let shouldRequestUpdate = true; - // if we have a property key, perform property update steps. - if (name !== undefined && !this._changedProperties.has(name)) { + // If we have a property key, perform property update steps. + if (name !== undefined) { const ctor = this.constructor as typeof UpdatingElement; const options = ctor._classProperties!.get(name) || defaultPropertyDeclaration; if (ctor._valueHasChanged( this[name as keyof this], oldValue, options.hasChanged)) { - // track old value when changing. - this._changedProperties.set(name, oldValue); - // add to reflecting properties set + // Track old value when changing. + if (!this._changedProperties.has(name)) { + this._changedProperties.set(name, oldValue); + } + // Add to reflecting properties set. + // Note, it's important that every change has a chance to add the + // property to `_reflectingProperties`. This ensures setting + // attribute + property reflects correctly. if (options.reflect === true && !(this._updateState & STATE_IS_REFLECTING_TO_PROPERTY)) { if (this._reflectingProperties === undefined) { @@ -596,7 +601,7 @@ export abstract class UpdatingElement extends HTMLElement { } this._reflectingProperties.set(name, options); } - // abort the request if the property should not be considered changed. + // Abort the request if the property should not be considered changed. } else { shouldRequestUpdate = false; } diff --git a/src/test/lib/updating-element_test.ts b/src/test/lib/updating-element_test.ts index 8daa277f..d10d3b0e 100644 --- a/src/test/lib/updating-element_test.ts +++ b/src/test/lib/updating-element_test.ts @@ -2219,4 +2219,32 @@ suite('UpdatingElement', () => { await a.updateComplete; assert.equal(a.updatedCalledCount, 1); }); + + test('property reflects after setting attribute in same update cycle', async () => { + class A extends UpdatingElement { + + foo?: boolean; + bar?: string; + + static get properties() { + return { + foo: {type: Boolean, reflect: true}, + bar: {type: String, reflect: true} + }; + } + + } + customElements.define(generateElementName(), A); + const a = new A(); + container.appendChild(a); + a.setAttribute('foo', ''); + a.removeAttribute('foo'); + a.foo = true; + await a.updateComplete; + assert.isTrue(a.hasAttribute('foo')); + a.setAttribute('bar', 'hi'); + a.bar = 'yo'; + await a.updateComplete; + assert.equal(a.getAttribute('bar'), 'yo'); + }); }); From 65bc612b01d937d1f0df99a580a2011bbb0d9729 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Wed, 13 Mar 2019 11:39:05 -0700 Subject: [PATCH 2/2] Address review feedback. --- src/lib/updating-element.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index 73f9cb14..4cca413d 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -586,7 +586,6 @@ export abstract class UpdatingElement extends HTMLElement { ctor._classProperties!.get(name) || defaultPropertyDeclaration; if (ctor._valueHasChanged( this[name as keyof this], oldValue, options.hasChanged)) { - // Track old value when changing. if (!this._changedProperties.has(name)) { this._changedProperties.set(name, oldValue); } @@ -601,8 +600,8 @@ export abstract class UpdatingElement extends HTMLElement { } this._reflectingProperties.set(name, options); } - // Abort the request if the property should not be considered changed. } else { + // Abort the request if the property should not be considered changed. shouldRequestUpdate = false; } }