Merge pull request #173 from LostInBrittany/bug-should-update

Ensure `firstUpdated` is always called when the element first updated

Author: Steve Orvell

src/lib/updating-element.ts

@@ -511,17 +511,17 @@ 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._markUpdated();
     }
   }
-
   private _markUpdated() {
     this._changedProperties = new Map();
-    this._updateState = this._updateState & ~STATE_UPDATE_REQUESTED | STATE_HAS_UPDATED;
+    this._updateState = this._updateState & ~STATE_UPDATE_REQUESTED;
   }
 
   /**

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 {