[lit-element] Address minor issues with LitElement (#1229)

Non-breaking fixes for minor issues.

* fixes lit/lit-element#722
* fixes lit/lit-element#890

packages/lit-element/CHANGELOG.md

@@ -24,20 +24,20 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 * [Breaking] Decorators are no longer exported from the `lit-element` module. Instead import any decorators you use from `lit-element/decorators/*`.
 * [Breaking] `lit-html` has been updated to 2.x. Note, shady-render support has been removed. Import the shady-render package to support Shady DOM.
 
+### Fixed
+* Fixes exceptions when parsing attributes from JSON ([#722](https://github.com/Polymer/lit-element/issues/722)).
+* Fixes issue with combining `static get properties` on an undefined superclass with `@property` on a subclasss ([#890]https://github.com/Polymer/lit-element/issues/890));
+
 ## [2.4.0] - 2020-08-19
 
 ### Changed
 * Set type in package.json to "module" ([#974](https://github.com/Polymer/lit-element/pull/974))
 
 ### Added
 * Adds a `cache: boolean` argument to the `@query` decorator as a performance optimization for properties whose queried element is not expected to change. If cache is set to true, element DOM is queried when the property is first accessed, and the value is cached so it can be immediately returned on all subsequent property accesses. ([#1013](https://github.com/Polymer/lit-element/issues/1013))
-
 * Adds a `selector: string` argument to the `@queryAssignedNodes` decorator as a convenience to filter the assigned nodes by the given selector ([#1016](https://github.com/Polymer/lit-element/issues/1016)).
-
 * The `requestUpdateInternal(name, oldValue, options)` method has been added. This method is sometimes useful to call in a custom property setter to optimize performance. It is slightly more efficient than `requestUpdate` since it does not return the `updateComplete` property which can be overridden to do work.
-
 * The protected `performUpdate()` method may now be called to syncronously "flush" a pending update, for example via a property setter. Note, performing a synchronous update only updates the element and not any potentially pending descendants in the element's local DOM ([#959](https://github.com/Polymer/lit-element/issues/959)).
-
 * Constructible stylesheets may now be provided directly as styles, in addition to using the `css` tagged template function ([#853](https://github.com/Polymer/lit-element/issues/853)).
 
 ### Fixed

packages/lit-element/src/lib/updating-element.ts

@@ -151,27 +151,40 @@ export const defaultConverter: ComplexAttributeConverter = {
   toAttribute(value: unknown, type?: unknown): unknown {
     switch (type) {
       case Boolean:
-        return value ? '' : null;
+        value = value ? '' : null;
+        break;
       case Object:
       case Array:
         // if the value is `null` or `undefined` pass this through
         // to allow removing/no change behavior.
-        return value == null ? value : JSON.stringify(value);
+        value = value == null ? value : JSON.stringify(value);
+        break;
     }
     return value;
   },
 
   fromAttribute(value: string | null, type?: unknown) {
+    let fromValue: unknown = value;
     switch (type) {
       case Boolean:
-        return value !== null;
+        fromValue = value !== null;
+        break;
       case Number:
-        return value === null ? null : Number(value);
+        fromValue = value === null ? null : Number(value);
+        break;
       case Object:
       case Array:
-        return JSON.parse(value!);
+        // Do *not* generate exception when invalid JSON is set as elements
+        // don't normally complain on being mis-configured.
+        // TODO(sorvell): Do generate exception in *dev mode*.
+        try {
+          fromValue = JSON.parse(value!);
+        } catch (e) {
+          fromValue = null;
+        }
+        break;
     }
-    return value;
+    return fromValue;
   },
 };
 
@@ -322,9 +335,8 @@ 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
-    // metadata.
-    this._ensureClassProperties();
+    // is called before `finalize`, we ensure finalization has been kicked off.
+    this.finalize();
     this._classProperties!.set(name, options);
     // Do not generate an accessor if the prototype already has one, since
     // it would be lost otherwise and that would never be the user's intention;
@@ -416,12 +428,12 @@ export abstract class UpdatingElement extends HTMLElement {
    * @nocollapse
    */
   protected static finalize() {
-    // finalize any superclasses
-    const superCtor = Object.getPrototypeOf(this);
-    if (!superCtor.hasOwnProperty(finalized)) {
-      superCtor.finalize();
+    if (this.hasOwnProperty(finalized)) {
+      return;
     }
     this[finalized] = true;
+    // finalize any superclasses
+    Object.getPrototypeOf(this).finalize();
     this._ensureClassProperties();
     // initialize Map populated in observedAttributes
     this._attributeToPropertyMap = new Map();

packages/lit-element/src/test/updating-element_test.ts

@@ -573,6 +573,36 @@ suite('UpdatingElement', () => {
     assert.isFalse(el3.hasAttribute('foo'));
   });
 
+  test('can mix properties superclass with decorator on subclass', async () => {
+    class E extends UpdatingElement {
+      static get properties() {
+        return {
+          foo: {},
+        };
+      }
+
+      foo: string;
+
+      constructor() {
+        super();
+        this.foo = 'foo';
+      }
+    }
+
+    class F extends E {
+      @property() bar = 'bar';
+    }
+    customElements.define(generateElementName(), F);
+    const el = new F();
+    container.appendChild(el);
+    await el.updateComplete;
+    el.setAttribute('foo', 'foo2');
+    el.setAttribute('bar', 'bar2');
+    await el.updateComplete;
+    assert.equal(el.foo, 'foo2');
+    assert.equal(el.bar, 'bar2');
+  });
+
   test('can mix property options via decorator and via getter', async () => {
     const hasChanged = (value: any, old: any) =>
       old === undefined || value > old;
@@ -737,6 +767,40 @@ suite('UpdatingElement', () => {
     assert.deepEqual(el.arr, [1, 2, 3, 4]);
   });
 
+  test('deserializing from invalid values does not produce exception', async () => {
+    class E extends UpdatingElement {
+      static get properties() {
+        return {
+          obj: {type: Object, reflect: true},
+          arr: {type: Array, reflect: true},
+          prop: {reflect: true},
+        };
+      }
+
+      obj?: any;
+      arr?: any;
+      prop?: string;
+    }
+    const name = generateElementName();
+    let error = false;
+    const listener = () => {
+      error = true;
+    };
+    window.addEventListener('error', listener);
+    customElements.define(name, E);
+    container.innerHTML = `<${name}
+      obj='{foo: true}'
+      arr="[1, 2, 3, 4]"
+      prop="prop"></${name}>`;
+    const el = container.firstChild as E;
+    await el.updateComplete;
+    assert.isFalse(error);
+    assert.equal(el.obj, undefined);
+    assert.equal(el.prop, 'prop');
+    assert.deepEqual(el.arr, [1, 2, 3, 4]);
+    window.removeEventListener('error', listener);
+  });
+
   if ((Object as Partial<typeof Object>).getOwnPropertySymbols) {
     test('properties defined using symbols', async () => {
       const zug = Symbol();