Netflix / x-element

A dead simple starting point for custom elements.
Apache License 2.0
28 stars 12 forks source link

Can we help you possibly use LitElement? #53

Open justinfagnani opened 4 years ago

justinfagnani commented 4 years ago

It's exciting to see another web component base class based on lit-html, but from a quick glance this seems to do a lot of the same things that LitElement does, and some of the additional things (like property observers) could be layered on. Is there anything we can do on the LitElement side of things to make it so you can use that instead of maintaining your own base class? I'm happy to help explore ideas if this is interesting to you at all.

theengineear commented 4 years ago

Thanks for reaching out @justinfagnani! We're definitely interested trying to extend LitElement. In fact, we drafted off of Polymer and watched the development of LitElement to help guide some decision making. We'd be willing to try and extend LitElement, especially if you would be willing to lend a hand now and again.

I think a good starting point would be to ask if you have examples of others who have built out support for observing properties and computing properties. We could then try and boot up an alternative base element and start trying it out.

justinfagnani commented 4 years ago

I could definitely lend a hand. LitElement should be pretty extensible already, and we're landing some changes to make it even more so.

For observing properties, I've used a subclass like this: https://gist.github.com/justinfagnani/d7f73971bfa3c96d5a08f9dadda6ea08

For computing properties, can you clarify what you mean? I typically just use class getters. Do you need more?

theengineear commented 4 years ago

For computing properties, can you clarify what you mean? I typically just use class getters. Do you need more?

We've been doing the computation up-front (vs lazily through getters) to avoid memoization and allow reflection and observation to follow from computation. However, I think your suggestion is to invert that mindset and let observation and reflection motivate the computation of properties -- handling memoization of computation as an optimization.

In general, I have a strong feeling that we'll be able to successfully extend LitElement. I think we should maybe just attempt to create a POC and report back with any roadblocks.

I'll try to find some time to do that and I'll let you know how it goes!

theengineear commented 4 years ago

@justinfagnani, I took a little time to start sizing up the task of trying to extend lit-element and a couple snags have come up so far:

  1. We currently do type coercion immediately upon setting. E.g., if you set type: Number in a property declaration, setting the property to the string '5' will immediately be coerced to the number 5.
  2. We rely heavily on computed, reflected properties. Maybe I'm mistaken, but it seems like there would not be an easy way for lazily-evaluated getters to hook into the attribute reflection machinery of the property declarations.
  3. Computed properties need to be read-only, is there a mechanism to do that?

A reasonable base test would be for us to support the following properties block:

static get properties() {
  return {
    a: { type: Number },
    b: { type: Number },
    c: { type: Number, computed: 'computeC(a, b)' },
    isEven: { type: Boolean, computed: 'computeIsEven(c)', observer: 'observeC', reflect: true },
  }
}

I don't think i'm completely stuck yet and I do intend to try and get something working so that we can both see how easy / hard it is to extend lit-element for our use case. However, I just wanted to keep you in the loop as I've hit a couple road bumps so far. I'll probably pick this up again in few days to continue fiddling.

justinfagnani commented 4 years ago

cc @sorvell @kevinpschaaf

Thanks for the feedback. I think we're actually looking pretty good here.

For (1), we just landed https://github.com/Polymer/lit-element/pull/914 which makes it easier to override the property definition. You should be able to grab the type from the options and do the coercion in the setter.

For (2) and (3) things might work already. If you define a property for a getter, you can set the reflect and it should be reflected in update(). This will be asynchronous, like all DOM mutations. Example with decorators:

class MyElement extends XElement {
  @property({reflect: true})
  a;

  @property({reflect: true})
  b;

  @property({reflect: true})
  get c() { return a + b; }
}
theengineear commented 4 years ago

We don't transpile our code, so I'm using UpdatingElement.createProperty (which seems to be equivalent). I tried putting together this quick demo but I don't think that even is being property reflected here. Perhaps I'm not instantiating the class-level properties correctly?

// index.html
<!doctype html>
<html>
  <head>
    <meta charset="UTF-8">
    <title>lit-element-test</title>
  </head>
  <body>
    <lit-element-test></lit-element-test>
    <script src="index.js" type="module"></script>
  </body>
</html>
// index.js
import { LitElement, css } from 'https://cdn.pika.dev/lit-element/^2.2.1';
import { html } from 'https://cdn.pika.dev/lit-html@^1.1.2';

const STYLE = css`
:host {
  display: block;
}

div {
  max-width: min-content;
  white-space: nowrap;
}

div + div {
  margin-top: 4px;
}

#equation,
#not-even {
  background-color: coral;
}

:host([even]) #equation,
#even {
  background-color: skyblue;
}
`;

export default class LitElementTest extends LitElement {
  constructor() {
    super();
    this.a = 1;
    this.b = 3;
  }

  get c() {
    return this.a + this.b;
  }

  get even() {
    return this.c % 2 === 0;
  }

  static get styles() {
    return [STYLE];
  }

  render() {
    return html`
      <div id="not-even">not even</div>
      <div id="even">even</div>
      <div id="equation">${this.a} + ${this.b} = ${this.c}</div>
    `;
  }
}

LitElementTest.createProperty('a', { type: Number });
LitElementTest.createProperty('b', { type: Number });
LitElementTest.createProperty('c', { type: Number });
LitElementTest.createProperty('even', { type: Boolean, reflect: true });

customElements.define('lit-element-test', LitElementTest);
justinfagnani commented 4 years ago

Unrelated to the the issue with even: instead of createProperty you should use the static properties block:

export default class LitElementTest extends LitElement {
  static properties = {
    a: {type: Number},
    b: {type: Number},
    c: {type: Number},
    even: {type: Boolean, reflect: true},
  };
}

The reason why the computed properties aren't reflected is that they're never marked as changed in the update lifecycle. I was able to work around this manually by overriding update() and check if the dependencies changed. See the StackBlitz here for a working example: https://stackblitz.com/edit/lit-element-reflect-computed?file=src/index.js

This requires a bug fix that landed in lit-element 2.3.0 so that a requestUpdate() before super.update() will be processed in the same render cycle. 2.3.0 also changed some of the property creation methods so they're more easily overridable, so you should be able to do something like:

export default class LitElementTest extends XElement {
  static properties = {
    a: {type: Number},
    b: {type: Number},
    c: {type: Number, dependsOn: ['a', 'b']},
    even: {type: Boolean, , dependsOn: ['c'], reflect: true},
  };
}

And have an override of update() and/or requestUpdate() and createProperty() take care of setting changed properties correctly.

Because these dependency chains are order-dependent or may form a cycle, you might have to do the checks in declaration order and document that, do a topological sort on the dependencies, or set changed properties synchronously (in requestUpdate() I think), and short-circuit appropriately.

@sorvell and @kevinpschaaf for an interesting use case for property customization.