Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ideas for LitElement 3.0 #1077

Closed
sorvell opened this issue Sep 9, 2020 · 95 comments
Closed

Ideas for LitElement 3.0 #1077

sorvell opened this issue Sep 9, 2020 · 95 comments

Comments

@sorvell
Copy link
Member

sorvell commented Sep 9, 2020

LitElement 2.x has been out for about a year, and we’ve been considering what changes we should make for the next major version. We wanted to share our thoughts and get feedback from our users about potential improvements and breaking changes.

Overall, we’re generally pretty satisfied with the core functionality and API and we definitely want to keep in mind that dealing with breaking changes is a hassle for users. We do, however, think there are some good opportunities for improvement. The lit-html library is also considering changes for the next major version, and we’ll definitely be including the next version of lit-html with the next version of LitElement.

Goals

Backwards compatibility for most users

We know that dealing with breaking changes is annoying and we’re trying to carefully manage our change budget such that each change has either a big return on investment or is likely to affect very few users. If necessary, we may elect to create a compatibility version that maintains 2.x behavior if it makes sense.

Performance

We love performance and are always striving to improve it. The next version of lit-html has a number of performance improvements from which LitElement will naturally benefit.

Size reductions

There’s some good opportunity for improvement in the core size of LitElement. First, the next version of lit-html should be getting a good bit smaller. Then, there’s a good bit of refactoring we can do to help make sure users are only getting the code that they use, including making old browser support opt-in, making decorators opt-in and more granular, and streamlining some of our internal API.

In addition, we may publish a minified, bundled build so that we can ensure that a heavily minified version works correctly. While we believe minification is ideally an app concern, we realize that apps won't know all the transforms that can be safely applied to our code.

Developer Experience

We want to make sure LitElement is easy to use for all our users. We know that some web devs love the pure experience of downloading some code, editing in a text editor, and seeing the output in a browser and we want to make sure that we support that use case better.

We would like to improve error messages, and not let publishing minified code negatively impact development. We'll look into producing both production and development builds.

Easier SSR

We know users want SSR, both for SEO and first paint performance. By leveraging the support for SSR being built into the next version of lit-html, LitElement will support SSR and it will be able to take advantage of the declarative Shadow DOM feature being added to Chrome.

New Features

While we want to consider some feature additions, we’re pretty happy with the core functionality. One thing we’d like to explore is providing a package of helpers for features not everyone needs but are really helpful for specific use cases or are just one way to skin the cat.

Fix bugs that require minor breaking changes

We have a number of longstanding minor issues that we haven’t been able to fix because they are just slightly breaking changes. We’d like to evaluate those and address what we can.

API cleanup

We think there’s a number of opportunities to reduce code size by carefully sweeping through the API. This should mostly be internal changes, but there may be some minor changes to the public facing API where we think there’s a strong benefit.

Potential Changes

Opt-in polyfill support

Although the Shady DOM polyfill itself is opt-in, there is some code baked into LitElement to use the polyfill that all users must pay for, which adds close to 1KB (~15%) to the bundle size. We’d like to make this opt-in so users who don’t need to support older browsers don’t pay the cost of downloading the polyfill specific code.

Opt-in and granular decorators

LitElement's main module exports all of the decorators we support regardless of whether or not they are used. We will break these out into granular modules that users will import if they use. This will also allow us to more freely add new decorators where useful.

Package that works out of the box without building

For optimal code size a build will always be best, but we want to provide an easy way to try LitElement without using npm and building the code.

Guidance for use in popular frameworks

The main value proposition of web components is that they work everywhere the (modern) web works. That’s true but they should also work seamlessly with modern web frameworks as well. We want to make sure LitElements work great in popular frameworks like React, Vue, and Angular. As shown on custom elements everywhere, the situation today is pretty good, and we may only need to create a set of examples showing what’s possible. However, we will be considering patterns and helpers that can make things easier for framework users.

Code sharing primitive

We think that subclassing and mixins cover most of the cases where users want to share code between elements. However, there are some cases they don’t cover. We’ve been impressed by efforts in the community to address those use cases, in particular React Hooks and the Vue composition API. We think there’s an opportunity to do something similar but probably a lot simpler for LitElement. This might take the form of controllers that can bundle functionality via a has-a relationship with the element and that can interact with the lifecycle of LitElement. Since not everyone needs this feature, this is probably a good candidate for a helper package.

SSR Support

When SSR output is being generated we need to consider which parts of the element lifecycle to run. It’s a goal of our SSR implementation not to require a server side DOM implementation and therefore code that uses DOM APIs will have no or very limited support. We will likely be modifying the update cycle for the element when it’s run on the server to avoid methods which often perform DOM manipulation, for example the firstUpdated and updated methods.

Theming

Platform support for theming is still incomplete. CSS custom properties are great for tree-based theming of known sets of properties. The new ::part selector works well for arbitrary theming of specific elements. However, the exportparts attribute is a cumbersome way to expose parts up the tree. In addition, it’s often desirable to allow users to style a set of unknown properties but not everything. There are a lot of patterns that can help here and we may end up supporting a few of them, perhaps in a helper package. We like the Vaadin themable-mixin and something like that for LitElement may make sense. We also like the power and configurability of Sass mixins and may explore exposing something similar via a Javascript API.

Declarative events

Declaring what events your element fires is great for self-documenting. We can also follow the DOM convention of providing an on prefixed property to listen to the event. Having on prefixed properties also improves integration with frameworks like React that configure events this way. We will probably implement this as a decorator but will have some form of raw JS support as well. See this issue for more info.

Common customization like shadowRoot rendering

To customize how the element’s shadowRoot is created you currently have to override createRenderRoot. We’d like to make this a bit easier and may add a decorator or static field with which you can supply the options object to attachShadow. For example:

static shadowRootOptions = {mode: ‘open’, delegatesFocus: true}

We may also explore this for other common but simple to configure customization points.

Declarative host manipulation

While we use lit-html to render the element’s Shadow DOM, attributes and event listeners on the hosting element itself must be added imperatively. We may be able to use some of the new features in lit-html 2.x to create a declarative way of doing this. This might take the form of a renderHost method which returns a lit-html TemplateResult that’s rendered on the host element. See this issue for more info.

Update coordination

LitElement updates asynchronously at microtask timing, and while this can’t be observed by the user, it can be by code. After an element has been interacted with, it’s sometimes useful to know when it’s finished rendering; for example, this is useful in testing, when firing events, or when you need to know the element’s rendering is stable. Although this can also be used for measuring DOM, it’s usually better to measure in a requestAnimationFrame callback. LitElement provides the updateComplete promise for these use cases. However, this only covers the element itself and not any LitElements in its shadowRoot. To wait for those elements, typically you override the updateComplete property and await those elements as well. That’s a bit cumbersome, and we want to explore adding a helper that makes this easier. Since this is only sometimes needed, this might be another feature which gets added to a helper package.

@Christian24
Copy link
Contributor

Excited for this. A couple things that came to my mind:

  • Maybe Custom property Accessors linting issues with ts-lint #469 can be revisited? It is no longer much of an issue for us, because we nowadays mostly use the updated callback, but I still find the issue I raised there to be worthwhile fixing and now might be the time.
  • Using updated extensively, I like it, but I feel some of the user experience could be improved:
    • If someone renames a property they also have to rename all the occurences of that property in updated. Worse even there would be no (compile) error, so this would only ever be noticed when the component is in use or testing.
    • It would be nice to have this typed too.
      I do not know how to make this happen I am afraid, but maybe there is something Typescript can provide here.
  • Maybe the docs could provide some additional information on bindings and how to handle rerendering. Sometimes we encounter an issue that a property changes within a component and then the component gets rerendered and the state is lost. I would love some more info on when this happens, how this happens and how to best deal with it.

@jaredcwhite
Copy link

Oooo, lots of good stuff to chew on here… I'm particularly interested in the "Code sharing primitive" angle, as it something I was thinking about recently as well. I did a lot of StimulusJS work before becoming enamored with LitElement, and one advantage Stimulus has is you can attach multiple controllers to a single element. It would be cool if I could explore an analogous pattern in the LitElement world…

@justinfagnani
Copy link
Contributor

@Christian24 #469 is already addressed in 3.0. requestUpdate() does not return a Promise anymore, and you have to specifically call await this.updateComplete.

@busynest
Copy link

may we have good documentation on bundling large apps, and coding exercises / challenges.

@funglaub
Copy link

A functional approach such as react hooks would really be nice.

@maartenst
Copy link

may we have good documentation on bundling large apps, and coding exercises / challenges.

I think that's already taken care of by open-wc?

@Westbrook
Copy link
Contributor

How early do you think we could start to see tech specs on the controller proposal mentioned herein? It sounds like a great tool for empowering both early LitElement learning as well as advanced/shared application development. I’d love to see whether you’d see it deeply integrated with LitElement, a pair to lit-html, or something more abstractly applicable to UI development at large.

@justinfagnani
Copy link
Contributor

@funglaub

A functional approach such as react hooks would really be nice.

We feel that we already have a pretty functional approach with the render() method of LitElement.

There's a lot of dislike of classes out there, but for this case where we 1) are creating objects by nature of the DOM and 2) do need to declare properties and attributes, classes are actually more declarative than a "pure" functional API where attributes and properties must be declared without the advantage of native syntax for it.

@justinfagnani
Copy link
Contributor

@Westbrook

How early do you think we could start to see tech specs on the controller proposal mentioned herein?

I'll try to publish an issue today.

@ghost
Copy link

ghost commented Sep 11, 2020 via email

@tsavo-vdb-knott
Copy link

@Westbrook lit/lit#1246

@jordanfinners
Copy link

I really like LitElement, so thank you in advance!

I think it would be awesome to see like a showcase of who else/what companies are also using it as this would be great publicity and help advocate its usage in organisations.

I think more demos/starter kits, already mentioned is how to use it with other frameworks but I think examples of large applications/advanced usage would be great and also starter kits that can generate the boilerplate for standard usages e.g. with an app shell, routing, linting, testing etc. Would be happy to contribute to these!

@kmmbvnr
Copy link

kmmbvnr commented Sep 13, 2020

Replace html templates with compiled js. Like Svelte and solid-js

@justinfagnani
Copy link
Contributor

@kmmbvnr

Replace html templates with compiled js. Like Svelte and solid-js

Any reason why?

@kmmbvnr
Copy link

kmmbvnr commented Sep 13, 2020

@justinfagnani

Any reason why?

Performance ?

@timonweb
Copy link

It would be great to get support for <slot> or an equivalent for the light dom. If I'm not mistaken, Stencil supports this. The thing is, you don't always need or want shadow dom, but the concept of slots is still very handy.

@bennypowers
Copy link
Contributor

It could be nice to streamline this kind of pattern with decorator options

class WaitsForChildren extends LitElement {
  render() {
    return html`
      <x-l id="a"></x-l>
      <x-l id="b"></x-l>
    `;
  }
  
  async _getUpdateComplete() {
    await super._getUpdateComplete();
    await Promise.all(Array.from(this.shadowRoot.querySelectorAll('x-l'), x => x.updateComplete);
  }
}
class WaitsForChildren extends LitElement {
  @query('#a', { awaitUpdate: true }) a: XL;
  @query('#b', { awaitUpdate: true }) b: XL;
  render() {
    return html`
      <x-l id="a"></x-l>
      <x-l id="b"></x-l>
    `;
  }
}

@eavichay
Copy link

class CustomInput extends LitMixin(HTMLInputElement) { ... }

// or

@builtIn(HTMLInputElement)
class CustomInput extends LitElement { ... }

// or any other solution to reconstruct the prototype chain

@bengfarrell
Copy link

One thing I've been running into a bunch lately is when I have a Lit component that requires me to know my own component bounds as part of the render (I'm absolutely positioning elements), there's no great update mechanism. In fact it was kind of terrible previously, but at least now I can listen to ResizeObserver. It might be nice to have some set of bindable properties that the user doesn't change, but they can opt into that trigger requestUpdate.

For example:

@property({ type: Number, internal: 'bounds.width }) protected componentWidth: number;

Bounds of course would be from this.getBoundingClientRect.

I'm not married to the my implementation suggestion by any means, but all the same, if it was similarly implemented, it could open the door to anything that you might add-on here if there was an API to do so (timers, sensors, data request results, etc)

Honestly, this type of functionality probably doesn't have to be part of Lit, but I'm kinda sick of re-writing ways to handle this. So if not Lit core, some kind of companion util might nice

@justinfagnani
Copy link
Contributor

@bengfarrell I think this might be nicely addressed with what I've been calling recently "reactive controllers" - helper objects that are able to hook and trigger the host element's lifecycle.

Imagine a controller called BoundsController which installs a ResizeObserver and on resize pulls the bounding rect and triggers an update. It'd be useable like:

class MyElement extends LitElement {
  private _bounds = new BoundsController(this);

  render() {
    return html`<pre>width: ${this._bounds.width}, height: ${this._bounds.height}</pre>`
  }
}

Note, this is pretty easily writable today - it doesn't require any new APIs on LitElement.

@bengfarrell
Copy link

@justinfagnani I think that's exactly the type of thing I was steering towards if I put more thought into it. And yes, it IS easily writeable today, but as a user the design pattern didn't really occur to me until I thought about what I was missing. And this is why I'm wondering if a few reactive controllers could be shipped alongside Lit (just import when you need it, not part of the core lib), in the same way you're offering extra directives. The few that ship could solve some common problems like bounds, but more importantly offer guidance to create your own. Of course, this might fall under the category of "too opinionated" a design pattern to ship with Lit

@MichaelPeter
Copy link

@tvvignesh : Thanks for the answer, did not know this was also possible for custom events and outside of components.

@dannymcgee
Copy link

A missing feature that I forgot to mention in my last comment: I would love to be able to extend element classes other than the generic HTMLElement. I realize the utility of this is kind of limited since there are so many base elements you can't append a shadow root to, but it would still be great to have the option. It seems like it would be relatively easy to pull off by just turning LitElement/UpdatingElement into mixin functions:

@customElement('my-element')
class MyElement extends LitElement(HTMLTableElement) {
  ...
}
<table is="my-element"></table>

@tvvignesh
Copy link

tvvignesh commented Nov 17, 2020

If I am not wrong, it should already be possible (#228) but not documented as mentioned in #829 and you can probably also have a look at how @openwc does this here: https://open-wc.org/docs/development/dedupe-mixin/

@dannymcgee
Copy link

Awesome, thanks for the info @tvvignesh!

@justinfagnani
Copy link
Contributor

#829 is about writing mixins that extend LitElement, not making LitElement itself a mixin. We're not very interested in pursuing making LitElement a mixin since Safari/Webkit will not implement customized built-ins. We don't want to encourage making elements that require polyfills forever for a significant number of users.

@dannymcgee
Copy link

...Safari/Webkit will not implement customized built-ins. We don't want to encourage making elements that require polyfills forever for a significant number of users.

Damn, I didn't realize that; thanks for the info. Totally understandable.

@busynest
Copy link

Bind properties to pseudo class using javascript

@Atulin
Copy link

Atulin commented Feb 5, 2021

Quick suggestion: Vue-like event modifiers.

For example, I have a modal like

<div class="shade">
  <div class="body">...</div>
</div>

and I only want the modal to close when the user clicks on the shade, and not on the body. Would be nice being able to do something like

<div class="shade" @click.self="${() => this.visible = false}">
  <div class="body">...</div>
</div>

@justinfagnani
Copy link
Contributor

@Atulin we're very sensitive to the bundle size and to adding too much special syntax into the template strings. .prop, @click, and ?attr are probably about as far as we want to go.

In JS we have more flexibility and can have pay-as-you-go helpers. For your example, something like this should work:

class SelfEventDirective extends Directive {
  render(listener: EventListener) {
    return listener;
  }
  update(part: EventPart, [listener]: DirectiveParameters<this>) {
    return (e) => {
      if (e.target === part.element) {
        listener(e);
      }
    };
  }
}
const self = directive(SelfEventDirective);

html`
  <div class="shade" @click="${self((e) => this.visible = false)}">
    <div class="body">...</div>
  </div>
`;

A directive is only necessary in this case to get a reference to the target element. Many of Vue's modifiers are implementable without a full directive. lit-html supports event listener options already on the listener. In LitElemetn we expose this via a decorator:

import {LitElement, eventOptions} from 'lit-element';

class MyElement extends LitElement {

  render() {
    return html`<button @click=${this._click}>Click Me</button`;
  }

  @eventOptions({once: true})
  _click(e) {
    console.log('click');
  }
}

You can do that inline as well:

    return html`<button @click=${{handleEvent: () => console.log('click', once: true}}>Click Me</button`;

Maybe we should look at the remaining Vue (and Angular, etc.) modifiers and see if we should vend helpers for any of them.

@Atulin
Copy link

Atulin commented Feb 5, 2021

@justinfagnani, I eventually solved it fairly painlessly with

<div class="shade" @click="${() => this.visible = false}">
  <div class="body" @click="${e => e.stopPropagation()}">...</div>
</div>

but @click.self or something to that effect would certainly be nicer to use. I agree that bundle size should take the priority, though.

@chase-moskal
Copy link

i would like "virtual components" which do not need to be globally registered to the dom via customElements.define

html`<${MyVirtualElement} some-attr="some-value"/>`

i've been thinking about this lately, as it would be useful for creating "internal" components that don't clutter up the global namespace, and also for dynamically applying mixins:

const MySpecialElement = myMixin(MyVirtualElement)
html`<${MySpecialElement} some-attr="some-value"/>`

let me know if lit-element already has this capability by some mechanism

@justinfagnani
Copy link
Contributor

@chase-moskal the entire Lit project is oriented around using the standard HTML component model and not creating our own.

The scoped custom element registries proposal is intended to solve that problem for all custom elements, not just component used in Lit.

@chase-moskal
Copy link

@justinfagnani — the custom element registries proposal looks great!

is there a good way we could develop a polyfill for custom element registries?

@a11delavar
Copy link

One issue on the API is that we create an associated attribute for setting a property by default with @Property, even though we require an opt-in for reflection. If we moved all attribute-related features to a separate decorator, which would include the type and converter, I could see something like this:

@attribute('disabled', {type: Boolean})
@attribute('aria-disabled', {converter: booleanString})
@property()
disabled: boolean;

...
Again, I don't think we could do this by default for 3.0.

@justinfagnani

As I am absolutely in favor of this more modularized approach to the decorators despite the fact that it is a bit more verbose and regarding the changes that I've been following in the pre-releases of LitElement 3.0 (specifically 3.0.0-pre.1), I wanted to know in which direction is the decorators headed.

As I understand, the @internalProperty has been renamed to @state and that's about it for decorators in the v3.0. It is of course an improvement but does it mean that @state and @property are going to coexist for v3.0? And if it is the case because of the lack of breaking-change budget for v3.0, is it going to be changed in upcoming major releases, and what would be the goals?

Also, on a somewhat-related note and problem that I faced with LitElement 2.0: recommending @internalProperty (@state in the future) decorators to be private via tooling has the huge disadvantage of not being able to turn on the attribute reflection. I always needed that solely to be able to write conditional CSS selectors, not that the property was really supposed to be public. This is one of the reasons I really liked the modularized approach to the decorators.

@LukeTOBrien
Copy link

LukeTOBrien commented Feb 11, 2021

Could somebody look at #1152 and tell me if this is a legitimate scenario.
Perhaps there could be some way to optionally await the Event Loop to be empty in the render/firstUpdated functions?

render() {
  await eventLoopEmpty();
  return html`
  <div>
    ${mapChildren((el, index) => {
      ...
    })}
  `;
}

Would that make sense or is that a bad idea?

Edit

Basicly what I am doing is creating a <slot> inside a <div> for each child:

render() {
    return html`
        <nav>
        ${ChildrenMap(this, (ele, index) => {    // ChildrenMap -> Array.prototype.slice.call(this.children).map(...)
            let id = ele.id ? ele.id :  `item-${index}`;
            ele.setAttribute('slot', id);

            return html`
                <div data-index="${index}">
                <slot name="${id}"></slot>
                </div>`;
        })}
        </nav>
    `;
}

@justinfagnani
Copy link
Contributor

@LukeTOBrien render() is always synchronous. If you don't have all the data needed for a render, render some kind of placeholder state and re-render when the data is available. We really encourage this approach of scheduling renders asynchronously when data is available, and writing render to handle whatever state the component is currently in.

As for children, it's already possible to iterate on children and dynamically create slots. <model-viewer> does this for it's dynamic label system. I would recommend using an unnamed slot, and the slotchange event to trigger a new render (with this.requestRender().

@LukeTOBrien
Copy link

LukeTOBrien commented Feb 18, 2021

@justinfagnani What about another callback that fires after the children have been rendered, say childrenRendered()?

render some kind of placeholder state and re-render when the data is available

I am finding that in the firstUpdated even the children aren't available, so a childrenRendered() would tell me that the data is available, I have a few classes now where I am using setTimeout, even I am using it twice if I want to re-render and then I also have a service class that initialises somekind of logic.
So I have crazy looking code like:

firstUpdated() {
  setTimeout(() => {
    this.requestUpdate();
    setTimeout(() => this.service.init());
  }
}

On a second thought, have you wondered about a custom attribute class, similar to Angular Attribute Directives?
Sometime I would like to add logical behaviour with Java/TypeScript to an element with nothing to render:

@attribute('draggable')
export class Draggable extends LitAttribute {
  init\firstUpdated() {
    // Init draggable
  }
}
<div draggable style="width:100px;height:100px;background:blue">
</div>

@Atulin
Copy link

Atulin commented Feb 18, 2021

I'd love some way of handling click-outside events. Something like a built-in clickOutside() function or something of the sort. I did create such functionality but I'd rather have it built-in. Especially since it shouldn't increase the weight too much.

@justinfagnani
Copy link
Contributor

@LukeTOBrien LitElement doesn't render its own children, only its shadow DOM contents. I'm not sure why the children aren't available in your case, but it likely has more to do with the code that's managing your component, than the component itself. We also can't vend a childrenRendered() callback, because we don't know when they have been, and children can change at any time.

I'd recommend building your components so that they don't assume the children are done being "rendered" at any one point in time. You can do this with an unnamed <slot> and listening for the slotchange event.

Regarding "attribute directives" we already have something very similar with lit-html directives.

@justinfagnani
Copy link
Contributor

@Atulin the way we build functionality in lit-html and LitElement is to only include what's absolutely necessary and can't be separated out in core. Optional functionality is in other modules like directives, decorators, and new in Lit 2 - controllers. Something like clickOutside seems fine external to the main library. It might be a good candidate for a controller.

@LukeTOBrien
Copy link

LukeTOBrien commented Feb 20, 2021

Here's one I think you might agree with.

More control over how attributes are converted.

In your defaultConverter you have:

  fromAttribute(value, type) {
    switch (type) {
      case Boolean:
        return value !== null;

      case Number:
        return value === null ? null : Number(value);

      case Object:
      case Array:
        return JSON.parse(value);
    }

So the default Array parser is JSON.parse but I would prefer string.split.
I have experimented with different combinations and it seem arrays items needs to be double quoted to be vaild JSON:
<my-element values='["one","two","three"]'>
(Note the single and double quote).
What I would rather have is space seperated:
<my-element value="one two three">

So therefore I think a seperator passed into the @property decorator would do the trick:

@property({ type: Array, seperator: ' '})
values: string[]

You could even do item types:

@property({ type: Array, seperator: ' ', itemType: Number})
values: number[]

Then change the fromAttribute Array case to:

...
case Array:
    if (seperator) {
        return value.split(seperator)// If itemType == Number then .map(x =< Number(x))
    } else {
        return JSON.parse(value);
    }

What do you think?
Space seperated in standard in CSS and there is a lot you can do to make it more customisable:

@justinfagnani
Copy link
Contributor

Given how close we are to shipping, and our desire to keep this a mostly backwards compatible change for component authors, I'd be reluctant to change the behavior of type: Array.

I could however see including a few optional converters, like a ListConverter converter that takes a separator:

@property({converter: new ListConverter()})
items: string[]; // takes "one two three"

@property({converter: new ListConverter(',')})
items: string[]; // takes "one, two, three"

or a MapConverter for exportparts-like values:

@property({converter: new MapConverter()})
items: string[]; // takes "one:a two:b three" similar to {'one': 'a', 'two': 'b', 'three': 'three'}

Another converter we've thought about before is an enum converter that has a fallback for invalid values.

@LukeTOBrien
Copy link

LukeTOBrien commented Feb 21, 2021

@justinfagnani I have been thinking and I also think an optional converter function would be useful:

@property({converter: new ListConverter(',', (str: string) => {
    let ray = str.trim().split(' ').map(x => Number(x));
    return new Vector3(ray[0], ray[1], ray[2]);
})});
items: Vector3[]; // Takes "1 2 3, 1 2 3, 1 2 3"

The with an optional convertor function you could have unlimited different convertions.

Note the .trim() to remove the spaces so we allow for "one, two, three" as well as "one,two,three".

@urish
Copy link

urish commented Mar 10, 2021

Given how close we are to shipping

How close?

@CITguy
Copy link

CITguy commented May 13, 2021

(Please let me know if this should be made its own issue. I'm not sure if this would be a bug report or a feature request.)

What about guaranteeing String prop types if external logic triggers the removal of HTML attributes?

Example

Let's look at the following custom element consumption and its definition

<script src="my-demo-element.js"></script>
<my-demo name="john doe"></my-demo>
/* my-demo-element.js */
import { html, LitElement } from 'lit-element'
import { ifDefined } from 'lit-html/directives/if-defined'

class DemoElement extends LitElement {
  static get properties() {
    return { 
      name: { type: String } 
    }
  }

  constructor() {
    super()

    // default value if not defined via HTML attribute
    this.name = '' // isolated from the rest of the property definition
  }

  render() {
    return html`<div name="${ifDefined(this.name)}">${this.name}</div>`
  }
}
customElements.define('my-demo', DemoElement)

LitElement makes use of attributeChangedCallback(attr, oldVal, newVal) and observedAttributes internally to keep the prop in sync with the attribute and perform necessary type coersion. So when the element is first created, attributeChangedCallback() applies this.name = 'john doe'. Changing it to <my-demo name="jeff"></my-demo> will apply this.name = 'jeff'.

Current type coersion behavior works well, but only as long as the attribute remains present on the element instance. When the [name] attribute is removed from the instance (either explicitly or implicitly through 3rd party virtual dom reconciliation), the newVal passed to attributeChangedCallback() will be null, and LitElement internals will apply this.name = null, making it of type object (very much not a string).

The ifDefined() directive from lit-html used to only support undefined as the only "undefined" value, but I see that it has already been updated with the nullish coalescing operator (??) to account for both null and undefined values (👍🏼).

Workarounds

This simple example could be updated to use a custom converter, but that would require tedious work to implement consistently across a suite of custom elements, especially given that assignment of a default value has to be applied in the constructor(). However, the logic seems simple enough that it would make sense for it to be baked-in to LitElement itself.

Suggestion

Instead of a blind pass through for { type: String } props, LitElement should either:

  1. coerce null attribute values into an empty string ('')
    • only works for String types (quick fix, but less complete solution)
  2. support a default option when defining properties, that is used when newVal is null
    • works for any property type (probably not as quick, but a more complete solution)

If option 2 were chosen, the custom element definition would look something like the following:

/* _props.js */
export const Name = {
  type: String,
  default: '' // if present, automatically applied in constructor()
}
/* my-demo-element.js */
import { html, LitElement } from 'lit-element'
import { ifDefined } from 'lit-html/directives/if-defined'
import { Name } from '_props.js'

class DemoElement extends LitElement {
  static get properties() {
    return { 
      name: Name
    }
  }

  render() {
    return html`<div name="${ifDefined(this.name)}">${this.name}</div>`
  }
}
customElements.define('my-demo', DemoElement)

@kevinpschaaf
Copy link
Member

Closing this issue as Lit 2 / LitElement is now in a stable release. Please open new issues for any feature requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests