From 58a94149edcab8ac223cd8967845391798abad8f Mon Sep 17 00:00:00 2001 From: Horacio Gonzalez Date: Fri, 7 Sep 2018 02:33:09 +0200 Subject: [PATCH 1/2] Ensure `firstUpdated` is always called when the element first updated Fix #172 --- src/lib/updating-element.ts | 7 +++-- src/test/lit-element_test.ts | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index 6ae2c74d..8d5a7aed 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -510,14 +510,17 @@ export abstract class UpdatingElement extends HTMLElement { } this.updated(changedProperties); } else { - this._markUpdated(); + this._markNotUpdated(); } } - private _markUpdated() { this._changedProperties = new Map(); this._updateState = this._updateState & ~STATE_UPDATE_REQUESTED | STATE_HAS_UPDATED; } + private _markNotUpdated() { + this._changedProperties = new Map(); + this._updateState = this._updateState & ~STATE_UPDATE_REQUESTED; + } /** * Returns a Promise that resolves when the element has completed updating. diff --git a/src/test/lit-element_test.ts b/src/test/lit-element_test.ts index d21a7274..1247bb2f 100644 --- a/src/test/lit-element_test.ts +++ b/src/test/lit-element_test.ts @@ -1063,6 +1063,56 @@ suite('LitElement', () => { assert.equal(el.wasFirstUpdated, 1); }); + test( + '`firstUpdated` called when element first updates even if first `shouldUpdate` returned false', async () => { + class E extends LitElement { + + @property() + foo = 1; + + triedToUpdatedCount = 0; + wasUpdatedCount = 0; + wasFirstUpdated = 0; + changedProperties: PropertyValues|undefined; + + shouldUpdate() { + this.triedToUpdatedCount++; + return this.triedToUpdatedCount > 1; + } + + update(changedProperties: PropertyValues) { + this.wasUpdatedCount++; + super.update(changedProperties); + } + + render() { return html ``; } + + firstUpdated(changedProperties: PropertyValues) { + this.changedProperties = changedProperties; + this.wasFirstUpdated++; + } + } + + customElements.define(generateElementName(), E); + const el = new E(); + container.appendChild(el); + await el.updateComplete; + const testMap = new Map(); + testMap.set('foo', undefined); + assert.equal(el.triedToUpdatedCount, 1); + assert.equal(el.wasUpdatedCount, 0); + assert.equal(el.wasFirstUpdated, 0); + await el.requestUpdate(); + assert.deepEqual(el.changedProperties, testMap); + assert.equal(el.triedToUpdatedCount, 2); + assert.equal(el.wasUpdatedCount, 1); + assert.equal(el.wasFirstUpdated, 1); + await el.requestUpdate(); + assert.equal(el.triedToUpdatedCount, 3); + assert.equal(el.wasUpdatedCount, 2); + assert.equal(el.wasFirstUpdated, 1); + }); + test( 'render lifecycle order', async () => { class E extends LitElement { From ce2e44ae3d7c4b64082d711d2906a31989b0e541 Mon Sep 17 00:00:00 2001 From: Horacio Gonzalez Date: Tue, 11 Sep 2018 19:08:25 +0200 Subject: [PATCH 2/2] Following @sorvell suggestion --- src/lib/updating-element.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/lib/updating-element.ts b/src/lib/updating-element.ts index 8d5a7aed..226e4ec1 100644 --- a/src/lib/updating-element.ts +++ b/src/lib/updating-element.ts @@ -506,18 +506,15 @@ export abstract class UpdatingElement extends HTMLElement { const needsFirstUpdate = !(this._updateState & STATE_HAS_UPDATED); this._markUpdated(); if (needsFirstUpdate) { + this._updateState = this._updateState | STATE_HAS_UPDATED; this.firstUpdated(changedProperties); } this.updated(changedProperties); } else { - this._markNotUpdated(); + this._markUpdated(); } } private _markUpdated() { - this._changedProperties = new Map(); - this._updateState = this._updateState & ~STATE_UPDATE_REQUESTED | STATE_HAS_UPDATED; - } - private _markNotUpdated() { this._changedProperties = new Map(); this._updateState = this._updateState & ~STATE_UPDATE_REQUESTED; }