[w3c/webcomponents] [templates] Parts APIs surface concerns (#680)

I have a few minor issues with the template parts APIs:

## Shared

- I'm not sure the benefits of a shared `value` getter/setter outweigh the confusion. `Attr`s have values, so it seems OK there, but for NodeTemplatePart, it seems rather arbitrary that you picked textContent semantics. I'd prefer to only have it for AttributeTemplatePart, and have more specific setters for NodeTemplatePart (see below).
- I'm not sure whether there should be a `TemplatePart` base class at all; just a mixin might suffice. I don't feel strongly, but I'd like to hear more about what you think inheritance gives here.

## Attribute parts

- `booleanValue` is not good name. It's a HTML convention that presence/absence is treated as "boolean". A quick fix would be to change this to `attributePresent` or similar. But, see next point.
- In general I think trying to put an API to turn on/off the entire attribute into an API that's primarily about substititing a string value into an expression seems wrong. It would be ideal if we could split these things out. For example, maybe controlling the presence or absence of attributes need a separate syntax? Or, perhaps `foo={{bar}}` should generate one type of template part, where `bar` is always interpreted as a boolean and controls presence/absence, whereas `foo="{{bar}}"` generates a different type of template part where `bar` is interpreted as a string? This latter idea seems pretty nice to me.

## Node parts

- In the IDL: `ContainerNode` is not a thing. I think it should just be `Node`.
- I'm not sure what justifies `previousSibling`/`nextSibling` but not all the other traversal and manipulation methods. Maybe it's OK, but right now I feel the choice is a bit arbitrary, and would appreciate convincing that this is the correct subset of `Node` to include.
- `replacementNodes` being `[NewObject]` is very bad. It should be a method, if it's going to return a new `NodeList` every time. I'm not sure why it has to return a new object every time though? Can't it only return a new object when they change?
- `replacementNodes` should probably return a `FrozenArray<Node>` instead of a `NodeList`. Or, if it becomes a method, just a `sequence<Node>`.
- The choice of mutating methods (`value` setter for textContent behavior, `replace()` for `replaceWith()` behavior, and `replaceHTML()` for `innerHTML` behavior) is strange to me. I'd prefer to use the conventional names (`textContent`, `replaceWith()`, and `innerHTML`). I'm also not convinced these are exactly the right choices: I think just `replaceWith()` would suffice, although `innerHTML` is convenient (since otherwise you need to do the template dance to create a suitable `DocumentFragment` to pass to `replaceWith()`).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/webcomponents/issues/680