Only call lit-html render if LitElement subclass implements render (#917

) * Only call lit-html render if LitElement subclass implements render - #712 Introduced a breaking behavior change for situations where `render` is unimplemented, and DOM is added before being connected to the document. * update CHANGELOG * Remove `connectedCallback` in render test
lit · Mar 7, 2020 · 0df1580 · 0df1580
1 parent 672e457
commit 0df1580
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 `LitElement` renders when updates are triggered as a result of rendering ([#549](https://github.com/Polymer/lit-element/issues/549)).
 * Properties annotated with the `eventOptions` decorator will now survive property renaming optimizations when used with tsickle and Closure JS Compiler.
 * Moved style gathering from `finalize` to `initialize` to be more lazy, and create stylesheets on the first instance initializing [#866](https://github.com/Polymer/lit-element/pull/866).
+* Fixed behavior change for components that do not implement `render()` introduced in ([#712](https://github.com/Polymer/lit-element/pull/712)) ([#917](https://github.com/Polymer/lit-element/pull/917))
 
 ## [2.2.1] - 2019-07-23
 ### Changed

diff --git a/src/lit-element.ts b/src/lit-element.ts
@@ -35,6 +35,12 @@ declare global {
 
 export interface CSSResultArray extends Array<CSSResult|CSSResultArray> {}
 
+/**
+ * Sentinal value used to avoid calling lit-html's render function when
+ * subclasses do not implement `render`
+ */
+const renderNotImplemented = {};
+
 export class LitElement extends UpdatingElement {
   /**
    * Ensure this class is marked as `finalized` as an optimization ensuring
@@ -204,11 +210,14 @@ export class LitElement extends UpdatingElement {
     // before that.
     const templateResult = this.render();
     super.update(changedProperties);
-    (this.constructor as typeof LitElement)
-        .render(
-            templateResult,
-            this.renderRoot,
-            {scopeName: this.localName, eventContext: this});
+    // If render is not implemented by the component, don't call lit-html render
+    if (templateResult !== renderNotImplemented) {
+      (this.constructor as typeof LitElement)
+          .render(
+              templateResult,
+              this.renderRoot,
+              {scopeName: this.localName, eventContext: this});
+    }
     // When native Shadow DOM is used but adoptedStyles are not supported,
     // insert styling after rendering to ensure adoptedStyles have highest
     // priority.
@@ -229,6 +238,6 @@ export class LitElement extends UpdatingElement {
    * update.
    */
   protected render(): unknown {
-    return undefined;
+    return renderNotImplemented;
   }
 }
diff --git a/src/test/lit-element_test.ts b/src/test/lit-element_test.ts
@@ -195,39 +195,42 @@ suite('LitElement', () => {
     assert.equal(window['litElementVersions'].length, 1);
   });
 
-  test('event fired during rendering element can trigger an update', async () => {
-    class E extends LitElement {
-      connectedCallback() {
-        super.connectedCallback();
-        this.dispatchEvent(new CustomEvent('foo', {bubbles: true, detail: 'foo'}));
-      }
-    }
-    customElements.define('x-child-61012', E);
-
-    class F extends LitElement {
-
-      static get properties() {
-        return {foo: {type: String}};
-      }
+  test(
+      'event fired during rendering element can trigger an update',
+      async () => {
+        class E extends LitElement {
+          connectedCallback() {
+            super.connectedCallback();
+            this.dispatchEvent(
+                new CustomEvent('foo', {bubbles: true, detail: 'foo'}));
+          }
+        }
+        customElements.define('x-child-61012', E);
 
-      foo = '';
+        class F extends LitElement {
+          static get properties() {
+            return {foo: {type: String}};
+          }
 
-      render() {
-        return html`<x-child-61012 @foo=${this._handleFoo}></x-child-61012><span>${this.foo}</span>`;
-      }
+          foo = '';
 
-      _handleFoo(e: CustomEvent) {
-        this.foo = e.detail;
-      }
+          render() {
+            return html`<x-child-61012 @foo=${
+                this._handleFoo}></x-child-61012><span>${this.foo}</span>`;
+          }
 
-    }
+          _handleFoo(e: CustomEvent) {
+            this.foo = e.detail;
+          }
+        }
 
-    customElements.define(generateElementName(), F);
-    const el = new F();
-    container.appendChild(el);
-    while (!(await el.updateComplete)) {}
-    assert.equal(el.shadowRoot!.textContent, 'foo');
-  });
+        customElements.define(generateElementName(), F);
+        const el = new F();
+        container.appendChild(el);
+        while (!(await el.updateComplete)) {
+        }
+        assert.equal(el.shadowRoot!.textContent, 'foo');
+      });
 
   test(
       'exceptions in `render` throw but do not prevent further updates',
@@ -266,4 +269,24 @@ suite('LitElement', () => {
         assert.equal(a.foo, 20);
         assert.equal(a.shadowRoot!.textContent, '20');
       });
+
+  test(
+      'if `render` is unimplemented, do not overwrite renderRoot', async () => {
+        class A extends LitElement {
+          addedDom: HTMLElement|null = null;
+          createRenderRoot() {
+            return this;
+          }
+        }
+        customElements.define(generateElementName(), A);
+        const a = new A();
+        const testDom = document.createElement('div');
+        a.appendChild(testDom);
+        container.appendChild(a);
+        await a.updateComplete;
+        assert.equal(
+            testDom.parentNode,
+            a,
+            'testDom should be a child of the component');
+      });
 });