WICG / webcomponents

Web Components specifications
Other
4.34k stars 373 forks source link

[templates] API for updating #685

Open domenic opened 6 years ago

domenic commented 6 years ago

I'm happy to see that updating existing DOM is accounted for (and fully specced!) in this model. It was never really clear to me how that would work, from previous discussions.

However, I think we might be able to come up with a better surface API.

In particular, I think it's weird that you essentially end up using TemplateInstance for two different things. First it holds your nodes with all the values stamped out. Then it transitions to a new phase of its lifecycle where you don't care about its children, but instead just care about using its update() method as an indirect path to talk to its former children.

I'm not sure what a better API would look like, at least tonight. I'll try to noodle on it before TPAC. But I think the current design is a little weird.

rniwa commented 6 years ago

Yeah, people did point out that weirdness. If someone can come up with a better API, we're more than happy to adopt that.

JanMiksovsky commented 6 years ago

I've spent the last few days pondering @domenic's comments. Component Kitchen is excited by this proposal, and share some of Domenic's concerns, so we wanted to take up @rniwa's invitation to think about the top-level API for template instantiation.

I thought live code experiments might be the best way to explore alternatives, and have put together some suggestions in the ReadMe at https://github.com/ComponentKitchen/template-instantiation.

At this point, the main suggestions are to consider separating template and parsing concerns, and to have the result of instantiation be two objects: a new instance and an updater.

// Parse a template with mustache syntax to obtain an element factory.
const factory = new ElementFactory(template);
// Use the factory to obtain both an instance and an updater.
const { instance, updater } = factory.instantiate({ name: 'world' });
// Add the instance to the document.
document.body.appendChild(instance);
// Later on, update.
updater.update({ name: 'Jane' });

Live demo (Source)

The repo's ReadMe goes into more detail. Among other thing, it explores use in web components, and how this approach could allow libraries like @justinfagnani's lit-html to efficiently participate in the new browser facility for element instantiation.

Coding with this feels fairly good. If this feels interesting, we're happy to develop this further, try out suggestions for changes, and see where it goes.

rniwa commented 6 years ago

I think I would prefer having a top-level object instead of returning a dictionary in instantiate. e.g.

const instance = factory.instantiate({ name: 'world' });
document.body.appendChild(content);
instance.update({ name: 'Jane' });
matthewp commented 6 years ago

@rniwa do you mean document.body.appendChild(instance.content) there?

rniwa commented 6 years ago

I meant that instance.content will be a DocumentFragment instead of factory.instantiate returning a DocumentFragment.

JanMiksovsky commented 6 years ago

@rniwa and I had a chance to talk about this last week. He was not in favor of most of my suggestions, although as he notes above, he is in favor of having a template's instantiate() call return a new type of object that's not an HTMLTemplateElement.

For now, let's say it's something like HTMLUpdatableInstance. This would have a content property that holds the document fragment, and an update method for updating those nodes.

const instance = template.instantiate();
document.body.appendChild(instance.content);
instance.update({ name: 'Jane' });

To me, at least, having this result be a distinct class feels better. Among other things, it means that HTMLTemplateElement doesn't get burdened with the notion of updatability.

With such an object returned by instantiate(), I believe I could still get destructuring along the lines shown above:

const { content, update } = template.instantiate();
document.body.appendChild(content);
update({ name: 'Jane' });