Fix inheriting styles from a superclass (#470)

* Add test for extending a class with styles

* Fix `styles` in extended elements

* UpdatingElement: makes `finalize` protected
* LitElement: build `styles` in `finalize`, taking care to inherit when needed

* Fix shadowing of `styles` in `_getUniqueStyles`

* [ci skip] Update changelog

Author: dfreedm

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Fixed
 * Fixed a bug where we broke compatibility with closure compiler's property renaming optimizations. JSCompiler_renameProperty can't be a module export ([#465](https://github.com/Polymer/lit-element/pull/465)).
+* Fixed an issue with inheriting from `styles` property when extending a superclass that is never instanced. ([#470](https://github.com/Polymer/lit-element/pull/470)).
 
 ## [2.0.0-rc.3] - 2019-01-18
 ### Fixed

src/lib/updating-element.ts

@@ -232,8 +232,8 @@ export abstract class UpdatingElement extends HTMLElement {
    * @nocollapse
    */
   static get observedAttributes() {
-    // note: piggy backing on this to ensure we're _finalized.
-    this._finalize();
+    // note: piggy backing on this to ensure we're finalized.
+    this.finalize();
     const attributes: string[] = [];
     // Use forEach so this works even if for/of loops are compiled to for loops
     // expecting arrays
@@ -249,7 +249,7 @@ export abstract class UpdatingElement extends HTMLElement {
 
   /**
    * Ensures the private `_classProperties` property metadata is created.
-   * In addition to `_finalize` this is also called in `createProperty` to
+   * In addition to `finalize` this is also called in `createProperty` to
    * ensure the `@property` decorator can add property metadata.
    */
   /** @nocollapse */
@@ -278,7 +278,7 @@ export abstract class UpdatingElement extends HTMLElement {
                         options:
                             PropertyDeclaration = defaultPropertyDeclaration) {
     // Note, since this can be called by the `@property` decorator which
-    // is called before `_finalize`, we ensure storage exists for property
+    // is called before `finalize`, we ensure storage exists for property
     // metadata.
     this._ensureClassProperties();
     this._classProperties!.set(name, options);
@@ -309,15 +309,15 @@ export abstract class UpdatingElement extends HTMLElement {
    * any superclasses are also finalized.
    * @nocollapse
    */
-  private static _finalize() {
+  protected static finalize() {
     if (this.hasOwnProperty(JSCompiler_renameProperty('finalized', this)) &&
         this.finalized) {
       return;
     }
     // finalize any superclasses
     const superCtor = Object.getPrototypeOf(this);
-    if (typeof superCtor._finalize === 'function') {
-      superCtor._finalize();
+    if (typeof superCtor.finalize === 'function') {
+      superCtor.finalize();
     }
     this.finalized = true;
     this._ensureClassProperties();

src/lit-element.ts

@@ -70,40 +70,42 @@ export class LitElement extends UpdatingElement {
 
   private static _styles: CSSResult[]|undefined;
 
-  private static get _uniqueStyles(): CSSResult[] {
-    if (!this.hasOwnProperty(JSCompiler_renameProperty('_styles', this))) {
-      // Inherit styles from superclass if none have been set.
-      if (!this.hasOwnProperty(JSCompiler_renameProperty('styles', this))) {
-        this._styles = this._styles !== undefined ? this._styles : [];
-      } else {
-        // Take care not to call `this.styles` multiple times since this generates
-        // new CSSResults each time.
-        // TODO(sorvell): Since we do not cache CSSResults by input, any
-        // shared styles will generate new stylesheet objects, which is wasteful.
-        // This should be addressed when a browser ships constructable
-        // stylesheets.
-        const userStyles = this.styles;
-        if (Array.isArray(userStyles)) {
-          const styles = flattenStyles(userStyles);
-          // As a performance optimization to avoid duplicated styling that can
-          // occur especially when composing via subclassing, de-duplicate styles
-          // preserving the last item in the list. The last item is kept to
-          // try to preserve cascade order with the assumption that it's most
-          // important that last added styles override previous styles.
-          const styleSet = styles.reduceRight((set, s) => {
-            set.add(s);
-            // on IE set.add does not return the set.
-            return set;
-          }, new Set<CSSResult>());
-          // Array.from does not work on Set in IE
-          this._styles = [];
-          styleSet.forEach((v) => this._styles!.unshift(v));
-        } else {
-          this._styles = userStyles ? [userStyles] : [];
-        }
-      }
+  protected static finalize() {
+    super.finalize();
+    // Prepare styling that is stamped at first render time. Styling
+    // is built from user provided `styles` or is inherited from the superclass.
+    this._styles = this.hasOwnProperty(JSCompiler_renameProperty('styles', this)) ?
+      this._getUniqueStyles() :
+      this._styles || [];
+  }
+
+  private static _getUniqueStyles(): CSSResult[] {
+    // Take care not to call `this.styles` multiple times since this generates
+    // new CSSResults each time.
+    // TODO(sorvell): Since we do not cache CSSResults by input, any
+    // shared styles will generate new stylesheet objects, which is wasteful.
+    // This should be addressed when a browser ships constructable
+    // stylesheets.
+    const userStyles = this.styles;
+    let styles: CSSResult[] = [];
+    if (Array.isArray(userStyles)) {
+      styles = flattenStyles(userStyles);
+      // As a performance optimization to avoid duplicated styling that can
+      // occur especially when composing via subclassing, de-duplicate styles
+      // preserving the last item in the list. The last item is kept to
+      // try to preserve cascade order with the assumption that it's most
+      // important that last added styles override previous styles.
+      const styleSet = styles.reduceRight((set, s) => {
+        set.add(s);
+        // on IE set.add does not return the set.
+        return set;
+      }, new Set<CSSResult>());
+      // Array.from does not work on Set in IE
+      styleSet.forEach((v) => styles!.unshift(v));
+    } else if (userStyles) {
+      styles = [userStyles];
     }
-    return this._styles as CSSResult[];
+    return styles;
   }
 
   private _needsShimAdoptedStyleSheets?: boolean;
@@ -151,7 +153,7 @@ export class LitElement extends UpdatingElement {
    * behavior](https://wicg.github.io/construct-stylesheets/#using-constructed-stylesheets).
    */
   protected adoptStyles() {
-    const styles = (this.constructor as typeof LitElement)._uniqueStyles;
+    const styles = (this.constructor as typeof LitElement)._styles!;
     if (styles.length === 0) {
       return;
     }
@@ -201,7 +203,7 @@ export class LitElement extends UpdatingElement {
     // priority.
     if (this._needsShimAdoptedStyleSheets) {
       this._needsShimAdoptedStyleSheets = false;
-      (this.constructor as typeof LitElement)._uniqueStyles.forEach((s) => {
+      (this.constructor as typeof LitElement)._styles!.forEach((s) => {
         const style = document.createElement('style');
         style.textContent = s.cssText;
         this.renderRoot!.appendChild(style);

src/test/lit-element_styling_test.ts

@@ -690,6 +690,26 @@ suite('Static get styles', () => {
     assert.equal(getComputedStyleValue(div!, 'border-top-width').trim(),
                  '4px');
   });
+
+  test('elements should inherit `styles` by default', async () => {
+    const base = generateElementName();
+    customElements.define(base, class extends LitElement {
+      static styles = css`div {border: 4px solid black;}`;
+    });
+
+    const sub = generateElementName();
+    customElements.define(sub, class extends customElements.get(base) {
+      render() {
+        return htmlWithStyles`<div></div>`;
+      }
+    });
+
+    const el = document.createElement(sub);
+    container.appendChild(el);
+    await (el as LitElement).updateComplete;
+    const div = el.shadowRoot!.querySelector('div');
+    assert.equal(getComputedStyle(div!).getPropertyValue('border-top-width').trim(), '4px');
+  });
 });
 
 suite('ShadyDOM', () => {