Lit-html: Promised attributes are not rendered properly

Created on 7 Feb 2018  路  3Comments  路  Source: Polymer/lit-html

Promised attribute are no rendered properly.

Using this code:

import { render, html } from "./lit-html.js";

const Main = () => {
  const postTitle = new Promise( (resolve, reject) => {
    setTimeout( () => { resolve('Title') }, 2000 );
  });
  const postClass = new Promise( (resolve, reject) => {
    setTimeout( () => { resolve('good') }, 1000 );
  });

  return html`<h1 class="test ${postClass}">Post: ${postTitle}</h1>`;
};
render(Main(), document.body);

Renders:

<h1 class="test [object Promise]">Post: Title</h1>

This seems to be caused by the fact that AttributePart._interpolate() treats all values as strings.

With the following change to lit-html.js, that can be remedied:

export class AttributePart {
  ...
  async _interpolate(values, startIndex) {
    ...
      const r = getValue(this, values[startIndex + i]);
      const v = (r instanceof Promise ? await r : r);
    ...
  }
  ...
  async setValue(values, startIndex) {
    ...
      value = await this._interpolate(values, startIndex);
    ...
  }
}
Medium Bug

Most helpful comment

The proposed change would make all calls to setValue asynchronous even if they don't have to be, which makes the library internals harder to understand.

There are other issues with the AttributePart: suppose I've got

const tpl = bar => html`
  <some-element some-attr="foo ${directive(part => part.setValue(bar))} baz">
  </some-element>
`;

render(tpl('bar'), container);

That'll yield

<some-element some-attr="foo [object Object] baz"></some-element>

and not the expected some-attr="foo bar baz". There are a couple of things wrong here:

  • The directive calls setValue with the wrong arguments: it should pass an array of values and an index of where to start in the array.
  • After the directive is executed, AttributePart#setValue continues and sets the value to "foo [object Object] baz". The [object Object] part comes from how directives are handled internally.

To get what I want, I'd need

const tpl = bar => html`
  <some-element some-attr="foo ${directive(part =>
    Promise.resolve().then(() => part.setValue([bar], 0)))} baz">
  </some-element>
`;

render(tpl('bar'), container);

which is hardly developer-friendly. It also becomes impossible to implement when there are two directives in one attribute.

A solution for both your issue and this lack of directive support would be to introduce a AttributePartPart (name is up for discussion) which is used for the interpolations in an attribute. To clarify:

  • AttributePart creates an AttributePartPart for every interpolation in the attribute. The value of the attribute is a concatenation of the literal strings and values of all AttributePartParts.
  • AttributePartPart stores the value in a property and notifies its parent AttributePart to change the attribute of the element when its value changes.
  • This AttributePartPart could then introduce logic similar to setValue in NodePart. Checking for directives, for iterables and for promises should suffice for now as putting a Node in an attribute doens't make sense. By properly supporting directives we could also look into what we need to do in asyncReplace/asyncAppend to support async iterables in attributes.

We'd need to think about the PropertyPart in lit-extended though, because we'd still want to make this usecase easy to support.

All 3 comments

The proposed change would make all calls to setValue asynchronous even if they don't have to be, which makes the library internals harder to understand.

There are other issues with the AttributePart: suppose I've got

const tpl = bar => html`
  <some-element some-attr="foo ${directive(part => part.setValue(bar))} baz">
  </some-element>
`;

render(tpl('bar'), container);

That'll yield

<some-element some-attr="foo [object Object] baz"></some-element>

and not the expected some-attr="foo bar baz". There are a couple of things wrong here:

  • The directive calls setValue with the wrong arguments: it should pass an array of values and an index of where to start in the array.
  • After the directive is executed, AttributePart#setValue continues and sets the value to "foo [object Object] baz". The [object Object] part comes from how directives are handled internally.

To get what I want, I'd need

const tpl = bar => html`
  <some-element some-attr="foo ${directive(part =>
    Promise.resolve().then(() => part.setValue([bar], 0)))} baz">
  </some-element>
`;

render(tpl('bar'), container);

which is hardly developer-friendly. It also becomes impossible to implement when there are two directives in one attribute.

A solution for both your issue and this lack of directive support would be to introduce a AttributePartPart (name is up for discussion) which is used for the interpolations in an attribute. To clarify:

  • AttributePart creates an AttributePartPart for every interpolation in the attribute. The value of the attribute is a concatenation of the literal strings and values of all AttributePartParts.
  • AttributePartPart stores the value in a property and notifies its parent AttributePart to change the attribute of the element when its value changes.
  • This AttributePartPart could then introduce logic similar to setValue in NodePart. Checking for directives, for iterables and for promises should suffice for now as putting a Node in an attribute doens't make sense. By properly supporting directives we could also look into what we need to do in asyncReplace/asyncAppend to support async iterables in attributes.

We'd need to think about the PropertyPart in lit-extended though, because we'd still want to make this usecase easy to support.

The suggested AttributePartPart changes by @bgotink have already been implemented with AttributePart / AttributeCommitter, so technically we can now implement Promises for AttributeParts.

However, the requested behaviour is already possible to implement with the new Directive API. If we want to support Promises here it will require additional implementation specific for this use-case. I'm not sure this is worth it, but I'll leave that decision to the maintainers.

Closing this because we're not going to support async attributes, but move all Promise support to a directive, in #555.

Was this page helpful?
0 / 5 - 0 ratings