Lit-html: HTMLTemplate as a supported Data Type

[paulvarache]

Both TemplateResults and HTMLElements are supported as data types.
I am wondering if HTMLTemplates should also be supported.

Example:

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

        const htmlTpl = document.createElement('template');

        htmlTpl.innerHTML = '<div>42</div>';

        const tpl = html`
            <div>${htmlTpl}</div>
        `;

        render(tpl, document.body);

I can work my way around this by using htmlTpl.cloneNode(true).content, and could make a directive that does that for me, but as lit is powered by HTMLTemplate is was wondering if it would make sense to support it as a Data type

[justinfagnani]

Interesting. Can you tell me your use case?

[paulvarache]

My use case is as follow:

I am creating a shared svg icons library. Each icon is a template with the svg element inside. This is to share across multiple projects using different libraries/frameworks. Using HTMLTemplateElements allows these icons to be used everywhere without depending on any library.

If I have these icons:

import { svg } from './svg-string-to-template.js';
export const logout = svg`<svg xmlns="...">...</svg>`;
export const login = svg`<svg xmlns="...">...</svg>`;

With Polymer I can do:

import { logout } from './icons.js';

class MyEl extends PolymerElement {
    static get template() {
        return html`
            ...
            ${logout}
            ...
        `;
    }
}

As I said before, I can definitely work with lit using a directive. I am wondering though what would be the performance implications of adding one more data type.
Especially if it is more rarely used.

[justinfagnani]

Seems reasonable. It wouldn't be much of a perf hit.

[paulvarache]

I will make a PR then

[ruphin]

As I mentioned in #583 I think this feature can easily be implemented with a directive:

const templateMap = new WeakMap();

const fromTemplate = directive(template => part => {
  if (templateMap.get(part) === template) {
    return;
  }
  templateMap.set(part, template)
  const content = template.cloneNode(true).content;
  part.setValue(content);
  part.commit();
}

// Usage:
const myTemplate = document.createElement('template');
myTemplate.innerHTML = '<p></p>'

html`<div>${ fromTemplate(myTemplate) }</div>`;

Since this is not a common use case, I don't see why we should build in support directly in lit-html core, because it will cause overhead to all other renders.

[justinfagnani]

There in a cost to all renders, only a very small cost to Node values, which will be dominated by the DOM mutation. The current behavior isn't useful, so this makes sense to me.

[ruphin]

What I understood from earlier discussions is that it's really desirable to keep the core lightweight, and to prefer adding features through directives.

A lot of commits eventually cascade to Node values, since all Templates and serialisable values eventually commit Nodes. Almost every NodePart value leads to eventually calling commitNode. I can't think of a single render call that does not call commitNode at least once.

We're considering the performance impact of different types of string comparisons in JavaScript engines to try and improve performance. I don't see why we don't consider the performance impact of adding a feature like this with the same scrutiny.

I understand the argument that the current behaviour isn't very useful, but the same could be said for passing all sorts of Node types or other primitives such as Promises. That doesn't mean we should support all of them in the core.

In this case we're dealing with a very unusual use-case, which can be implemented in a 5-line directive (with feature parity), and adds overhead (not significant overhead, but still overhead) to all renders when added to core. Even if the overhead amounts to just 0.0001%, I'm fairly certain that a significantly smaller portion of render calls will actually use this feature.

[justinfagnani]

So we have two questions:

1. Is the feature worthwhile?
2. Is the code size and perf change minimal?

The feature

On it's own the feature makes a lot of sense to me. lit-html allows for Nodes to be passed as values, and <template> is a special node that we don't have special handling for. lit-html allows you to compose a template from other templates, but so far only if they're lit-html templates. That we don't allow you to compose with plain platform templates seems like a bit of an oversight here.

It's not such a missing feature that a directive isn't also an ok way to deal with this, but it seems like eventually with Template Instantiation we'll want to treat templates very much like template results (check template origin against existing value, instantiate or update, etc.). This is a first, very minimal, step in that direction.

The cost

The nodeName check would be done only after other checks have been cleared and we determine that we need to actually write a value. At that point a string comparison is going to be dominated by the DOM mutation itself.

We can limit the check to the one place where Node are given directly to setValue with just a little duplicated code. The other paths into _commitNode we know have another node type. It's the balance between that and the partial-line patch, which is just: this._insert(value.nodeName === 'TEMPLATE' ? value.content.cloneNode(true) : value)

---

I'm not in a rush to add this - I want to consider it carefully, but my initial thinking is that this capability fits squarely into our domain and existing adjacent capabilities.

[keanulee]

But what if I do want a <template> element in the content (e.g. iron-list expects a <template> element in its light DOM). I don't think <template> should necessarily be treated in a special way (compared to other Nodes).

Alternative would be to make a clone of the template once (for each place in the document it's used), then use that in your template:

const icon = template.cloneNode(true).content;

function update() {
  render(html`<div>${icon}</div>`, document.body);
}

If using a component model, each component instance would have its own icon.

[justinfagnani]

@keanulee that's an interesting point. I think it's really rare that you'd want insert a template into the DOM, rather than clone it, but it is possible. There's no analog with TemplateResult in that case, because there's no such thing as inserting a TemplateResult _without_ rendering it.

[justinfagnani]

We talked about this a while back on the team and the the point that @keanulee brings up is important. We can't know that a template shouldn't be actually inserted into the DOM. A directive could handle this, or the pattern Keanu showed.

[justinfagnani]

Note that this was addressed by #1058