🐛 #bugfix Issue #135 multiple custom elements only render the first element

[snuggs]

🐛 bugfix #135 @albertoponti

Reproduction

Prerequisite Understanding

• MDN document.querySelector
• MDN document.querySelectorAll
• MDN <template> Element

Problem

The logic we used here is based off document.querySelector to find all templates within the custom element. (Or so we thought). document.querySelector only finds the FIRST template it finds from the selector. Which will always be the first element.

Code in Question

1. this.render () Templating logic
2. HTMLTemplateElement initialization routine

Solution

We now are sending the template directly from within the scope of the custom element. Before we were just sending template name into new Template. However new Template('somename') is merely a standalone way of using Template. It will just go looking in the document for a template of the name you pass in. Alternatively (and this is a big alternative) new Template (node) can also take a Node as a parameter.

In our this.render routine we did the following operation:

problematic operations highlighted

1. Find all templates with a name property within custom element.
2. map a collection of names for each template.
3. call new Template (name) for each template. a. use document.querySelector to grab <template name=...>
4. .bind (data) to <template>

We can actually totally remove 2 and we ensure the templates retrieved are scoped to the custom element.

Questions

1. What do we do in the situation of multiple templates of the same name within a single custom element? (@brandondees @robcole @angelocordon)

<foo-bar>
  <template name=baz>...</template>
  <template name=baz>...</template>
</foo-bar>

2. What standalone API do we want to support (this is huge @albertoponti @brandondees @robcole @tmornini because #developerErgonomics )

new Template('name') // I think should be a classic default
let node = document.querySelector ('template')
new Template (node) // creating from existing template node

Template `name` // modern approach but can't send a node in via tagged templates.

@robcole we actually have this issue in Funderbolt but circumvent by explicit binding the (new Template (...)).bind (data) which is similar to how snuggsi internals work (which means we're doing too much).

@tmornini dude perfect example of removing code and removing a bug too. Actually gaining functionality by removing code. Check the diff. ;-)

[brandondees]

as a rubyist, i'd be inclined to expect a second template with the same name to override / replace the first one, at least as far as using templates by name is concerned.

for Q2 I'd really expect both lines 1 and 3 to work, and 4 would just be nice sugar to have available for consistency with other snuggsi stuff we're doing already. If only one of three is available, i think 4 is cleanest/easiest but I'm not clear if there are use case differences with the others or not.

[snuggs]

All are available today actually @brandondees. Just making sure we don't have too much "Flexibility". As I learned from @tmornini TOO MUCH "flexibility" actually summons the devil. My gut instinct, going off usages with @robcole, working with the spec implementations, etc. all point to there being valid use cases for all 3 approaches. Agreed? We currently use 1 and 2 within core. And 3 is a must have for standalone usage.

<template name=somename>{foo}</template>

<script src=//unpkg.com/snuggsi></script>
<script>
  // returns a cloned copy of the template decorated with a `template.bind (data)` method.
  let
    template = Template `somename` // perhaps new (Template `somename`) returns the clone?
  , context  = { foo: 'bar' }

  template.bind (context)

</script>

Is an ergonomic win win all around when just trying to grab a cloned JS instance of the template that can call template.bind (data) IMHO. Curious what @rianby64 @kurtcagle @janz93 have to say about this.

[snuggs]

@albertoponti this also fixes a situation was having with a tag-complete component being used for an email composer. Since there were two on the same page the document.querySelector was only returning first template. Now we pass that responsibility of template collection up to the custom element. Then we explicitly pass a template to upgrade for each (child) template. Thus fixing all the bugs. Also related to #139 /cc @robcole