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

[lit-html] Add syntax for spreading attributes, properties, and event listeners #923

Open
straversi opened this issue May 21, 2019 · 65 comments
Assignees

Comments

@straversi
Copy link
Contributor

It would be nice to be able to spread bindings in lit-html. One would be able to spread properties, attributes, and event listeners.

Syntax could look something like this:

const spreadable = { '.property': 1, 'attribute': 'foo', '@click': handler }
html`<div ...=${spreadable}>How convenient</div>`

This feature request was discussed at length in this pull request: #213

We're investigating the tradeoff between convenience and bloat associated with this feature. This issue is for feature tracking.

@straversi
Copy link
Contributor Author

Here's an implementation of this feature: #925

It added 0.18 KB to the minified and gzipped package size, for a new size of 3.46 KB.

Questions:

  • How much do people want this feature?
  • Is it worth the size cost?
  • Is there another more cost-effective way to implement this? (as a directive...?)

@daKmoR
Copy link
Contributor

daKmoR commented May 21, 2019

uuhhh nice 🤗 - I am probably biased but I would say it's worth the cost 🤗 especially as I would consider it sort of a core features the same as ? or @ or ....

@ruphin
Copy link
Contributor

ruphin commented May 21, 2019

The updating mechanism for this is fundamentally different from how current property/attribute setters work. How will caching/updating things work? What if you spread one less property than in the previous render, does it remove the property from the node? Does it remember the previous value for every property ever spread indefinitely, so it can do updates only when the values changed?

@straversi
Copy link
Contributor Author

How will caching/updating things work?

At least in #925, Parts are still created for each key-value pair in a spread object. The intention is for each attribute/property/event in a spread object to keep its setValue and commit behavior, including dirty checking.

What if you spread one less property than in the previous render, does it remove the property from the node?

Our thinking is that if a key-value pair is removed from the spread object, that attribute/property/event would also be removed from the element.

@Westbrook
Copy link
Contributor

I am very excited for how this spreads attributes. The possibility of simplifying passing and template writing is real here. Hope some important questions can find answers/documentation to move this forward.

However, I'm a little wary that the work to form an object to {'.value': bar, '@click': baz, '?disabled', bot} is just about the same as:

<input
  .value=${bar}
  @click=${baz}
  ?disabled=${bot}
/>

With the various part types applied via the same ... attribute I'm not sure the extension of the syntax really buys much. I'd also say this would be better as only an attribute or only a property spreader, but I feel there will be as much confusion on either side of the barrier.

I think what would be closer to expectation would be something like ..., ...., @..., ?..., but I can't imagine that could be handled with such a small addition as is being employed in the PR at this point and I understand that it's quite a complicated hurdle when learning the syntax.

@justinfagnani
Copy link
Collaborator

@Westbrook I get the concern around object keys. My thinking with keeping the keys aligned to the existing syntax is that once on the JS side of things, regular functions can map the names, ie:

const propsFrom = (o) => ({...Object.entries(o).map(([k, v]) => [`.{k}`, v])});
const eventsFrom = (o) => ({...Object.entries(o).map(([k, v]) => [`@{k}`, v])});

const obj = {value: 'foo', tabIndex: -1};
html`<input ...=${propsFrom(obj)}>`;

@Westbrook
Copy link
Contributor

@justinfagnani that makes sense and solves most of what I'm getting at. Would that mean that something like: <input ...=${propsFrom(objProps) ...=${eventsFrom(objEvents) ...=${objAttributes} /> will be possible?

And, do you think lit-html will vend these helpers?

@CaptainCodeman
Copy link

I'm not sold that this provides anything genuinely necessary, not something I'd want to pay a price in terms of bytes and possible performance / complexity for anyway. I hope if it's added that it's an ignorable thing that the majority of people who will never need or use it won't have to "pay" for.

It sounds like just another DSL layer to pass into the existing templating DSL

@daKmoR
Copy link
Contributor

daKmoR commented May 22, 2019

for a concrete user case we have lot's of small "templates" - here is one in "psoidocode".

export function fooTemplate(options) {
  return html`
     <button 
       @click=${options.button1click} 
       ?disabled=${options.button1disabled}>${options.button1label}</button>
     <button @click=${options.button2click}>no</button>
  `
}

// other file
import { fooTemplate } from './fooTemplate.js';

class Foo extends LitElement {
  render() {
    const fooOptions = { button1click, button1disabled, button1label, button2click };
    return html`${fooTemplate(fooOptions)}`;
  }

this has 2 drawbacks:

  1. if anyone "extends" my class and he want's to extend then it becomes quite complex as they will need to also copy all the "logic" => which makes it rather fragile as if something get's added to the original template I will need to copy and adjust again
// example I want to wrap the 2nd button in an extra div as my styling is different
export function myFooTemplate(options) {
  return html`
     <button 
         @click=${options.button1click} 
         ?disabled=${options.button1disabled}>${options.button1label}</button>
     <div>
       <button @click=${options.button2click}>no</button>
     </div>
  `
}
  1. whenever I want to add an additional property/content I will need to add it to the fooOptions, fooTemplate function and fooTemplate template literal.

if ... would be available it would make it easier to maintain an extension layer.

// example I want to wrap the 2nd button in an extra div as my styling is different
export function myFooTemplate(options) {
  return html`
     <button ...=${options.button1}>${options.button1label}</button>
     <div>
       <button ...=${options.button2}>no</button>
     </div>
  `;
}

Doing so the code I am extending can still add additional attributes/properties/events if needed without the extender needing to do anything.

PS: deliberately not using object destructuring on this example to make it "easier" to compare

@justinfagnani
Copy link
Collaborator

@CaptainCodeman I sympathize with that opinion a lot. To be clear, we're not definitely going to land this, but we wanted to see what a new implementation would look like and how much it would cost. I do want to see concrete use cases, and on that front, thanks @daKmoR :)

I wonder if anyone with experience with React spread props can help us see if the use cases translate. At a glance the one form their docs page doesn't: https://zhenyong.github.io/react/docs/jsx-spread.html

@ruphin
Copy link
Contributor

ruphin commented May 22, 2019

Personally, this feature feels like syntax sugar to me, and I don't think it adds significant new capabilities to the library. I'd welcome this as a directive, but I'm not sure adding additional overhead to the core library is worth it to me. I understand the use case of having a dynamic set of properties, and wanting to keep a declarative format. However, I have not encountered this case in any of the the projects that I have worked on, so that makes me feel like it's not a common case. To me, this change will effectively be 5% additional overhead.

I did encounter several developers who would like to use spread operators in their templates, not because their properties are dynamic, but simply because they like the style. I do expect that when this feature is added, that a significant number of developers will adopt this as a default way to bind their properties for that reason. If this feature has a performance cost compared to normal static bindings (which I believe it does), that may be an additional reason to be careful with adding this feature. Having different ways to do the same thing is generally not a great pattern in software.

@thepassle
Copy link
Contributor

As a user, i'd like to give my 2c here as well, and I'm with captaincodeman and ruphin on this one. I don't see how this adds significant value for whats seemingly only a 'nice to have'/sugar/familiarity with other frameworks. Also having to use helpers like <input ...=${propsFrom(objProps) ...=${eventsFrom(objEvents) />, or pre-define objects as {'.value': bar, '@click': baz} seem to beat the purpose of the simplicity of the spread operator.

@logicalphase
Copy link

logicalphase commented May 22, 2019 via email

@ruphin
Copy link
Contributor

ruphin commented May 22, 2019

The current proposed implementation is a builtin, so it is not optional.

@justinfagnani
Copy link
Collaborator

This is a way to make this optional by subclassing the DefaultTemplateProcessor:

export class SpreadTemplateProcessor extends DefaultTemplateProcessor {
  handleAttributeExpressions(
      element: Element, name: string, strings: string[],
      options: RenderOptions): ReadonlyArray<Part> {
    if (name === '...') {
      // Only one part per key is allowed via Spread
      const attributeHandler = (name: string): Part =>
          this.handleAttributeExpressions(element, name, ['', ''], options)[0];
      return [new SpreadPart(element, attributeHandler)];
    }
    return super(element, name, strings, options);
  }
}

export const spreadTemplateProcessor = new SpreadTemplateProcessor();

export const html = (strings: TemplateStringsArray, ...values: unknown[]) =>
    new TemplateResult(strings, values, 'html', spreadTemplateProcessor);

export const svg = (strings: TemplateStringsArray, ...values: unknown[]) =>
    new SVGTemplateResult(strings, values, 'svg', spreadTemplateProcessor);

To reiterate, this issue is very much up for debate and in no way guaranteed to to be added. One reason @straversi worked on this was to at least close out the oldest PR we had and use this as a tool to get to know the library. Those purposes are satisfied even if the new PR gets closed out too.

@logicalphase
Copy link

logicalphase commented May 23, 2019 via email

@petecarapetyan
Copy link

Per @ruphin

Having different ways to do the same thing is generally not a great pattern in software.

Thus the lift would need to be sufficiently great that it justify the increased surface area of the learning interface.

@chase-moskal
Copy link

chase-moskal commented May 23, 2019

hello friends

i want to be clear about the premise of this discussion

  1. are we skeptical about the spread feature altogether?

  2. or are we really only skeptical about adding @click and ?bool and .prop syntax to the spread feature?

i hope it's just premise 2, because the basic ability to spread simple string attributes will save me a lot of ugliness — please humor me about my use case with a video element i'm working on, i want help finding the best practice here

<my-video
  title=${video.title}
  description=${video.description}
  numeral=${video.numeral}
  thumbnail=${video.thumbnail}
  duration=${video.duration}
  >
</my-video>

this obviously isn't scaling well, so i assumed i'd use spread, as is familiar in other templating libraries and also javascript generally, <my-video ...${video}></my-video>, and i was surprised to find myself here in this thread instead of in lit-html's spread feature documentation

i can hear you say it already: "just use .video=${video}"
however i cannot, because this component is intended to be used via pure html (neither js nor lit-html are required), and since string attributes are what pure html supports, that's what my video component accepts — it's a good user experience for pure html usage

however in lit-html, without spread, the user experience of this component is bad — when you are handed a video object, you have to be as verbose as my bad example above

from what i've gathered, there seem to be two possible resolutions to my dilemma

  1. spread for lit-html would simply knock this problem out of the park

  2. i should always implement both interfaces — <my-video> could accept all of the string attributes from pure-html users, but then also accept a .video property from js — then i must have some disgusting logic in the lit-elements like this.videoActual = this.video || this or perhaps Object.assign(this, this.video) — one caveat to this workaround is that the video object cannot have a child video property of the same name — (edit: somebody in this thread mentioned that it's bad in software to have two ways of doing the same thing, so here i think about the duplication that this lack of spread is forcing me to endure, forging two separate routes in my lit-elements to pass the same information)

obviously until spread lands, i'll be using resolution 2 as my workaround, but i'm not really sure if it's pretty cool, or kind of hacky

maybe there's a better remedy for my use case? or a different way of thinking about the problem? i'd love to hear different perspectives about how you folks think around this kind of use-case

👋 thanks fellas

@CaptainCodeman
Copy link

CaptainCodeman commented May 24, 2019

I'm skeptical that it's really required, especially with the extra needs for the different types (props, attributes, events) and because it will impact package sizes + perf (I assume).

For your example, I'd rather use something like:

<my-video .video=${video}></my-video>

I'd have the video element responsible for understanding and handling the video object passed into it.

It might come down to coding style (and using a lot of Go) but I've just found it easier if I add a property to an object if I don't have to go updating all the element signatures. Just like if you passed an object to a function vs the individual values as primitive types (e.g. do you break an address into street / city / region / country / country / postal code or just pass "address").

It may depend on whether you are building elements for in-app use vs public consumption. There's a stronger case for using primitive types for the latter so they behave more like regular elements, allowing attributes to set properties etc... but if you're binding to the properties in lit, I don't see why you wouldn't pass an object instead of breaking the object up just to make the call.

@chase-moskal
Copy link

chase-moskal commented May 24, 2019

@CaptainCodeman

For your example, I'd rather use something like:

<my-video .video=${video}></my-video>

i'm afraid you've missed my point

as i mentioned in my previous post, using the .video=${video} syntax is not a valid option for my use-case, because we want to allow simple configuration via html attributes (without having to use javascript or lit-html)

the pattern you described is only possible when using lit-html, and actually then cannot be configured simply via html attributes

in typescript terms, MyVideo component implements the Video interface, whose properties are realized in terms of html attributes — this is simple, and makes for a good user experience for html-only users

There's a stronger case for using primitive types for the latter so they behave more like regular elements, allowing attributes to set properties etc... but if you're binding to the properties in lit, I don't see why you wouldn't pass an object instead of breaking the object up just to make the call.

exactly as you said it, "so they behave more like regular elements" is a requirement for my use-case

but if you're binding to the properties in lit, I don't see why you wouldn't pass an object instead of breaking the object up just to make the call.

exactly the point: in some cases, we're not using lit, just html

my concern is that if this use-case is overlooked, lit won't play seamlessly with other components in the ecosystem because we can't nicely use an element configured by many attributes

i see the lack of spread as an interoperability problem

i'll use my workaround for the time being, however i'm open to any alternative solutions

@CaptainCodeman
Copy link

CaptainCodeman commented May 24, 2019

I don't quite follow - if you're creating a component for public use and want to support the separate attributes then how does the spread operator help you? Aren't they going to be consuming those attributes / properties separately (so they are already split up)? Your example has them using lit which is maybe where I'm confused (and I have flu, I may re-read this in the morning and realize I'm talking gibberish)

Is it that you are then passing those elements on to a child element? In which case, if you're then back at the point where you can just pass them as an object - the same object that you might want to "spread" (?)

@chase-moskal
Copy link

chase-moskal commented May 24, 2019

@CaptainCodeman

I don't quite follow - if you're creating a component for public use and want to support the separate attributes then how does the spread operator help you?

we need robust components that are easy to use in different contexts

  • ✓ pure html users just declare the attributes directly:
    <my-video title="cool video" description="very cool video">
  • ✓ jsx users can spread via <my-video {...video}>
  • htm users can spread via <my-video ...${video}>
  • esx users can spread via <my-video ...${video}>
  • ✘ lit-html users cannot spread :(

as i mentioned before, i can use that workaround having two separate ways for each component to ingest data — however that's a special thing i'd have to do just for lit-html users, because lit-html lacks this common feature in a flourishing ecosystem that includes the interplay of many different templating libraries

edit: the interoperability concern: imagine you're consuming a web component whose author used htm, and it accepts a hundred different attributes, and it gives you an api function which returns an object for all of those attributes — the author is expecting you to spread, just like you do in htm or in jsx or in esx — however because you are a lit-html user, you're now in a pickle: you have no recourse but to map every single attribute like in my bad example, or fork the component to add the workaround specific to lit-html — i hope this helps illustrate how spread is actually an important feature for lit's interoperability with the ecosystem

@chase-moskal
Copy link

chase-moskal commented May 24, 2019

i'm curious about why the syntax includes the equal sign?

<my-video ...=${video}>

other templating languages are not using an equal sign for spread (jsx, esx, htm, even javascript itself)

<my-video ...${video}>

in this proposal, is the equal sign optional?

@ruphin
Copy link
Contributor

ruphin commented May 24, 2019

The equals sign is not optional, because of how the parser works. The spread is interpreted as a special case of an attribute, which requires the equals sign.

Supporting the spread without equals sign would require changes to the parser.

@kevinpschaaf kevinpschaaf self-assigned this Dec 8, 2020
@Haprog
Copy link

Haprog commented Dec 16, 2020

@ferhtgoldaraz the spread directive linked above (https://open-wc.org/developing/lit-helpers.html#spread-directives) should be able to do this for you right now.

That link seems to be broken now. Looks like the page has moved here (without redirect):
https://open-wc.org/docs/development/lit-helpers/#spread-directives

@justinfagnani
Copy link
Collaborator

We won't be adding a specific spread syntax for the foreseeable future, though we do have "element expressions" as mentioned, which will allows us or users to create a spread directive without having to use a fake attribute name.

@justinfagnani
Copy link
Collaborator

Reopening so we can use this issue to track the spread directive(s).

@dkozma
Copy link

dkozma commented Dec 2, 2021

@daKmoR Is an out-of-the-box solution still available for spread helpers? I noticed that it looks like it was taken out of the open-wc library in open-wc/open-wc@f3cbb2a with a note that says:

the spread and spreadProps directives no longer work with the updated directive API of lit. They will need to be recreated and we will do this in lit-labs.

However, I don't see anything in the current directory structure that looks like it would account for this.

@abdonrd
Copy link
Contributor

abdonrd commented Dec 2, 2021

However, I don't see anything in the current directory structure that looks like it would account for this.

There is an PR about this here: #1960

@augustjk augustjk moved this to 🔥 Front Burner in Lit Project Board Feb 10, 2022
@justinfagnani justinfagnani changed the title Add syntax for spreading attributes, properties, and event listeners [lit-html] Add syntax for spreading attributes, properties, and event listeners Jun 17, 2022
@justinfagnani justinfagnani moved this from 🔥 Front Burner to 📋 Triaged in Lit Project Board Jun 17, 2022
@vladnicula
Copy link

Hey everyone. I've been using LIT in https://play.teleporthq.io/ in some isolated cases. I'm now looking at writing our entire rendering system inside the visual editor using lit directives. A deeper support for spread would be very welcome for our use-case.

The spread directive which is mentioned a couple of time above is great for static attributes, as long as we are not server side rendering. It does not support nested attribute directives which we need.

We don't know what attributes our element directives will have (so we need to spread a data driven input as attributes), and on top of that each attribute is data bound and can change over time, ideally without re-rendering the entire tag, just updating itself. The AttributePart directive seems to allow us to create this dynamic data bind for each attribute, but it does not seem to work in combination with the Spread directive, as the spread directive writes directly to the DOM, it does not give anything to lit to work with.

Is the code from @justinfagnani 's comment mentioning expanding the template function valid or usable in any way? Can anyone advice on how we might patch this in on top of lit html so that we get to use it for our usecase, even if it does not make it as a supported feature?

@ryanolsonx
Copy link

Any update on this? I'd love to use lit-html in a design system project and spreading attributes will be pretty important for components.

@hjopel
Copy link

hjopel commented Mar 12, 2023

@ryanolsonx Same here, but as @vladnicula mentioned above, open web components can be used and modified for your exact purpose for the time being I guess

@oligafner
Copy link

oligafner commented Jul 13, 2023

@daKmoR Is an out-of-the-box solution still available for spread helpers? I noticed that it looks like it was taken out of the open-wc library in open-wc/open-wc@f3cbb2a with a note that says:

the spread and spreadProps directives no longer work with the updated directive API of lit. They will need to be recreated and we will do this in lit-labs.

However, I don't see anything in the current directory structure that looks like it would account for this.

Looks like spread is back in @open-wc/lit-helpers
https://github.com/open-wc/open-wc/blob/master/packages/lit-helpers/CHANGELOG.md 🥳

@andrehil
Copy link

andrehil commented Dec 6, 2023

Can this functionality please be added to the library?
Having to add another dependency just for prop spreading is a bit overkill.
I feel like this is an important functionality for a library like Lit.

@emersonbottero
Copy link

I implemented this for. Attributes.

import { ChildPart, directive, Directive} from 'lit/directive.js'

Class Attrs extends Directive {
     update (part: ChildPart) {

         const host = part.options?host! as Element
        //@ts-ignore  
         const  element = part.element as HTMLElement

          for (const attr of host.attributes) {
               If ( attr.name == "class") element.classList.add(attr.value) 
               Else element.setAttribute(arrt.name, attr.value)
          }
     }

      render() {}
}

export const attrs  = directive (Attrs)

export type { Attrs }

Of course it can be improve, i.e. styles .. etc... but it is working for me ...

I think it for sure could be a lab...

@emersonbottero
Copy link

P.S.: I don't know the correct way to get element from part without a ts-ignore... I would love to know how...

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

No branches or pull requests