Bundling templates with controlling JavaScript

[nomego]

I've been experimenting with a way to bundle the JavaScript nicely with template views.

As far as I've noticed, I cannot easily access query/path parameters in a nested template (or can I?) so there should only be one (root) <template> tag in a template view.

The best I've come up with is a root <template> element (that cannot be auto-binding, this will trigger the script twice) with an inline <script> tag that finds its template with document.currentScript.templateInstance.model. Not sure if I can count on this being set to the template?

However, ideally, I'd like to separate the template and its JavaScript and only vulcanize/combine them before publishing to production.

When adding a src attribute to the <script> tag, the tag gets loaded two times, once when the HTML import is run, and once when the <template> tag gets inserted into the DOM. During the HTML import, the relative path is correct, that is, if the template is at /views/module1/view1.html and the src tag says view1.js, /views/module1/view1.js is imported. But when inserted into the DOM, the relative path is lost and the browser tries to load /view1.js.

One way to get around the second import is to move the <script src> tag out of the root template tag. That way, it's only loaded during the HTML import and with the expected relative path.

However, when doing this, there are two issues:

1. The <script> element is located in its own, disconnected DOM, so finding the corresponding template with relative JavaScript like code like above or document.currentScript.parentElement.querySelector('template[is="auto-binding"]') wouldn't work.
2. The script will be run before the template is inserted to the DOM, so to find the template we'd also have to wait for that event to happen.

Also regardless of the solution, will it be possible to vulcanize/bundle multiple template view "bundles" into one file for the client ?

[erikringsmuth]

I see the end goal you're going for and it makes sense. Getting there might be tricky though. I'll give my 2 cents on the issues you're seeing.

Nested <template> tags won't be able to access variables in their parent template unless you explicitely bind the variable like this https://www.polymer-project.org/docs/polymer/binding-types.html#single-template-instances. Using one root <template> is simpler because you don't need to worry about template scopes. This is just the way Polymer data binding works...

<template is="auto-binding"> is Polymer specific and tells it to bind the contents of the template as soon as it's parsed. This doesn't help app-router since it will bind templates itself.

Running a <script> tag inside the <template> tag and looking for document.currentScript.templateInstance.model sounds right to me. You should be able to count on this. document.currentScript is standard DOM and templateInstance.model is Polymer, but it should be there by the time the script gets run.

Are you saying this script gets run twice?

<template>
  <script src="..."></script>
</template>

That seems weird to me. Possibly a browser or Polymer bug. I also think this looks like one of the best approaches if you get it working.

Moving the script outside of the template will only run the script once because HTML imports are de-duped and only get loaded once. It'll also only get run the first time it's loaded.

<script src="..."></script>
<template>
</template>

Different paths during HTML import and DOM insertion also makes sense since the script is running from a different part of the document.

The two issues you identified both make sense. The script will have trouble finding the correct template and it will be run before the template is inserted into the DOM.

This all said, it's not going to be easy to make a <template>, <script> combo that operates in a sane way. I'm pretty sure there will be some convention and boilerplate code you'll need to use everywhere to make this work well.

Now, for the last point, right now there's no way for app-router to distinguish multiple templates in a single file. Something like this could be implemented to specify an ID of a template.

<template id="template1">
  <script src="..."></script>
</template>

<template id="template2">
  <script src="..."></script>
</template>

<app-route path="/example" import="/pages/example-page.html" template="template2"></app-route>

[nomego]

Thanks for your elaborate answer!

Nested templates might actually be the way to go, if I can get <template bind> to work for parameters. Do you have a working example of this? Also, I can only get the sub-template to work if it's auto-binding? But this would make the view even more declarative, showing what path/query parameters are allowed/expected and preventing a parameter from affecting the view (un)intentionally.

Having only one root template makes it hard (impossible?) to find the right template object to control/enrich with JavaScript after it's inserted into the DOM? Also, weirdly enough, var template = document.currentScript.parentElement.querySelector('template'); will find the root template element (before it's moved to the DOM) in the HTML import but even though I can log out the content with template.innerHTML and such, I can't seem to navigate within it? (template.querySelector('*')),

auto-binding templates' scripts get run twice, so maybe app-router should assist by not binding or by removing the auto-binding attribute if the root template has it. (Or just warn about it, or leave it as is and document it?) The script only gets run twice if the template is auto-binding.

But since the mismatch between HTML import path and DOM insertion path, the browser tries to load it twice, with different URLs. (It probably won't run it twice, though, right?) This feels like one of the major issues. Not sure if app-router re-writing relative src-paths before injecting into DOM would make any sense either.. ? As far as I see it, the only way to prevent the error request that at best will 404 but at work will import a script with the same name in a different path is to leave the <script src> outside the root <template>. This would also work if the build process injects the script-file inline. (But not with vulcanizing..)

Based on this new information, the best approach is starting to look like:

<template>
  <template bind="{{ id as param_id }}" bind="{{ p2 as param_p2 }}">
  </template>
</template>
<script src="script.js"></script>

The script file would then need to (somehow) find the template in the HTML import context, and hook into it before it gets moved to the DOM, or maybe cleaner, find its own <app-route> and subscribe to the before-data-binding event, and update the model then.

Unless the relative path of the script can be fixed for non-HTML import contexts, when it would make more sense to put the script within the root template element.

But actually, where all this comes from is a concern that registering a unique element for every view will pile up a lot of elements in the browser that doesn't get garbage collected when the user navigates from the view, since the browser still needs to be aware of the element declaration? Or can we unregister elements as we navigate away from the views? Or is this performance/memory footprint concern not valid until there are 500+ views and the user is using an old mobile phone ?

[plequang]

A small note when using nested templates : there is a bug in removeRouteContent if using nested templates : https://github.com/erikringsmuth/app-router/blob/master/src/app-router.js#L372 the function won't remove a 'template' node inserted in app-route children, so they will accumulate each time you navigate to this route. This also means queries like querySelector('template[is="auto-binding"]') will return an outdated template instance (I guess it will always return the first inserted one).

[erikringsmuth]

Oh boy there are a lot of questions here ;)

Starting with the last note about memory management. Template vs. custom element routes will consume similar amounts of memory. It's the HTML imports that will be consuming most of the memory. When you import an HTML document, the browser keeps a reference to it as long as you are on that page. Custom elements will consume slightly more memory becuase the element definition is a JS object, but the main chunk of the memory will be the template that it references in the HTML import. The instances of the custom elements and templates will all be garbage collected, but not the definitions.

This is a valid concern though. The more I work with client side code, the more I end up thinking server side rendering is the way to optimize for mobile. It's going to be a tough problem for web components in general and I don't have any solutions.

So... I also don't have answers to most of the rest of these questions. I'd need a lot more free time to dig into all of that. It'll be cool if you get it all working! The complexity is why I just wrapped all my pages in custom elements.

The one thing I'll say is I don't want the router to have to rewrite URLs. That's an area that can get ugly really quick. Polymer does it and was the reason why <a> tags were buggy until a few weeks ago when they released Polymer 0.5.4.

@plequang Want to open a separate issue for the bug? That way we can keep track of it on it's own.

[nomego]

Haha sorry this has turned into a philosophical thread :)

Well regarding the memory management, isn't the correct condition that the HTML import will have a reference to the import document as long as the <link> tag i present on the page you're at?

Quoting http://www.html5rocks.com/en/tutorials/webcomponents/imports/:

link.import is null under the following conditions: ... The <link> has been removed from the DOM. ...

It would suggest that you could also "unimport" HTML documents, which hopefully would garbage collect everything nicely. Unless.. they're all registered custom elements that the browser needs to keep track of ?

If so, <template> based views' memory footprint can be freed for other views, whereas custom element based ones would linger until you've left the page.

Also, does HTML imports land in the browser cache? If so, re-inserting the same ones might not actually cause re-fetching?

Don't give up on the client-side stuff! :)

If the above theories are true, one way to go would be to focus on the template based views and make sure that mobile clients doesn't have too many imports. <app-router max-imports="{{ mobile ? 10 : 100 }}"> ? :)

Ok so I had a first look at it and found a solution that will be good enough for now. It has a one-line change dependency to app-router, which is to associate the route with the <link> tag it generated, at the end of importAndActivate():

@@ -274,6 +274,7 @@
         importLoadedCallback();
       }
     }
+    route.importLink = importLink;
   }

   // Activate the imported custom element or template

Then, scoped outside the <app-router> tag, globally or preferably in the wrapping custom element, two things need to be done.

First, set up a script to callback map:

        created: function () {
            this.importScriptElements = [];
        },

        registerScriptCallback: function (callback) {
            this.importScriptElements.push({
                script: document.currentScript,
                callback: callback
            });
        },

Then, subscribe to the 'activate-route-end' event. This event handler would look through the route's HTML import, and fire any callbacks that was registered from any of the imported scripts. To solve the nested template binding for parameters, I simply assign event.detail.model to template.params. Initially I just called the template script with the event.detail.model parameter on the 'before-data-binding' event, but there was too much functionality lacking, like adding functions or accessing elements.

        activateRouteEnd: function (e) {

            var i,
                o,
                route = e.detail.route,
                importLink = route.importLink,
                scripts = importLink ? importLink.import.scripts : null,
                template = route.querySelector('template[is=auto-binding]');

            if (template !== null) {

                template.params = e.detail.model;

                for (i = 0; i < scripts.length; i += 1) {
                    for (o = 0; o < this.importScriptElements.length; o += 1) {
                        if (this.importScriptElements[o].script === scripts[i]) {
                            this.importScriptElements[o].callback(template);
                        }
                    }
                }
            }
        },

The template view structure would then need to be:

<template>
    <template is="auto-binding">
        <div id="message"></div>
    </template>
</template>
<script type="text/javascript" src="script.js"></script>

With a script.js like:

appElement.registerScriptCallback(function (template) {
    template.addEventListener('template-bound', function () {
        console.log('after autobind');
    });
    template.message = template.params.qp1 || 'no qp1 param!';
});

A nice detail about the script.js above is that the callback actually is called before the auto-binding takes place, so we can enrich the model/template before the first render and avoid a performance hit there, while still having the option to do stuff after the template is bound, like looking into template.$. This approach should also be pretty safe for async loading and to prevent URI/view/script mismatch scenarios.

Although this creates an unnecessary templateInstance.

Even better would be to add auto-binding template support to app-router:

@@ -305,7 +306,10 @@
   // Create an instance of the template
   function activateTemplate(router, template, route, url, eventDetail) {
     var templateInstance;
-    if ('createInstance' in template) {
+    if (template.getAttribute('is') === 'auto-binding') {
+      template.params = createModel(router, route, url, eventDetail);
+      templateInstance = template;
+    } else if ('createInstance' in template) {
       // template.createInstance(model) is a Polymer method that binds a model to a template and also fixes
       // https://github.com/erikringsmuth/app-router/issues/19
       var model = createModel(router, route, url, eventDetail);

I would actually prefer to have any path/query params scoped in a named object like params above, for non-auto-binding templates as well, to highlight more clearly in the template what kind of value we're binding.

Let me know what you think and I'll create PRs if you want me to.

[erikringsmuth]

That's an interesting point from html5rocks. That does suggest that an import could be un-loaded. I would guess that it would be garbage collected if there were no more references to the imported document. A registered custom element constructor would retain a reference to the template which would be a reason to avoid custom elements.

HTML imports do land in the browser cache which is handled separately from the page's JS/DOM heap. That means re-adding the HTML import link should already have most imports cached by the browser.

The thing I like about plain old custom elements and templates is that the API is set by the DOM spec. If we go toward an app-router registerScriptCallback() you end up writing app-router specific code.

Is there a minimal set of changes like adding route.importLink = importLink; that would get your way working without forcing a standard convention like registerScriptCallback()? That could maybe lead to an even simpler way in the future.

Also, I'm curious if auto-binding is needed at all.

If you import this template today, the model will be bound to the outer template already. I don't know if auto-binding is actually gaining anything here.

<template>
    <template is="auto-binding">
        <div id="message"></div>
    </template>
</template>

[nomego]

Great! It feels like I'm on the right track with my theories :)

Well for my implementation / pattern above, all that is really needed is the one-liner change to app-router to maintain a reference to the import in the route.

The rest I can be implemented in the outer application. However, I would ask you to consider to implement registerScriptCallback() as a helper function in app-router to aid others so they don't have to read through this whole issue and end up implementing it on their own, should they choose to :)

Well as far as templates go, I want to be able to access the actual template instance/object to be able to completely control the template. Things like accessing template.$ after the template is bound or assigning template.onButtonClick = function () { ... }. That is, things I can't do with the model during the 'before-data-binding' event. I haven't found a way to access the template object in another manner either. The reason for this is probably that template.createInstance(model) creates a template instance of the template content, without the wrapping template tags/object. So that entry point is lost. Whereas, with an auto-binding sub-template, the instance can actually be accessed from it's DOM tag object and controlled during it's lifecycle.

So for me, I'd rather drop the outer template and make app-router support auto-binding root templates, which actually would only mean not calling template.createInstance().

Well, to support parameters with auto-binding templates, I would suggest scoping the path/query parameter model in app-router. And the most sane way would be to adjust the current template model scope as well. This would be the biggest, most braking change, calling for a major version dump. The question here would only be what to call the object. params ? routeArguments (like the util method) ? arguments (like the JavaScript function standard) ?

I'll get busy with the PRs :)

[erikringsmuth]

I still don't fully understand how auto-binding is helping. Is it that you need access to the instance of the template or custom element after it's created? If that's the case we could add an instance field in the activate-route-end event.

Also, why does the model need to be wrapped in {params: model} when you end up using it likt this template.params = createModel(router, route, url, eventDetail).params;? This would be a big breaking change I want to avoid.

[nomego]

Yes, I need access to the instance.

But modifying the instance returned by

templateInstance = template.createInstance(model);

does not reflect/rebind on the rendered template, or I've missed something fundamental. And why I think this is, as I described in my last comment, that this instance only contains the content of the template, not the controlling template instance (template tag) itself.

If I'm missing something here and access to this instance can be solved someway, that would be great. But I don't see adding support for auto-binding being in the way of anything else. Instead of using params, the query information can be passed to an auto-binding template the same way as you currently do for custom elements:

    for (var property in model) {
      if (model.hasOwnProperty(property)) {
        customElement[property] = model[property];
      }
    }

Which brings me to the params-part.

Instead of looping over the model properties and injecting into the element/template instance, you will reduce this into a simple assignment: element.params = model.params.

Also, not having a scope for the query/path parameters and the above code, means that (as I wrote in the fourth commit not referenced here), ?id=mynewid or ?hidden=true will adjust native HTMLElement properties and/or conflict with other top-level properties in the template. Example: https://erikringsmuth.github.io/app-router/#/notes?hidden=true

While this not might be 'dangerous' since everything is client-side, it will produce a lot of pitfalls for developers, especially when parameters start conflicting with other properties in templates.

Finally, the reason why createModel() creates { params: model } instead of just model (and the assignment becoming element.params = model), is because createModel() also assigns the model to the before-data-binding events being fired, where I would expect to modify the model.params.id in case the model gets extended. Of course, this could be solved by doing eventDetail.model = { params: model } and returning model instead. But if the model is to be extended some day, I would recommend my approach.

[erikringsmuth]

Oh wow, I didn't realize URLs like https://erikringsmuth.github.io/app-router/#/notes?hidden=true would mess with the page like that. I agree with you on this one now. Although, I think createModel() should return an object like this:

{
  params: {
    // path variables and query parameters
  },
  router
}

Since the bindRouter attribute will also add the router to the model.

This should get a separate issue and PR since it will be breaking changes and a new major version. I want to delay this until a few other existing issues are resolved.

Back to the first issue. templateInstance = template.createInstance(model); will create an instance of the template with the model bound to it.

URL: www.example.com/user/Jon

route

<app-route path="/user/:name">
  <template>
    <p>Hello {{name}}</p>
  </template>
</app-route>

model

{
  name: "Jon"
}

route after it's activated

<app-route path="/user/:name">
  <template>
    <p>Hello {{name}}</p>
  </template>
  <p>Hello Jon</p>
</app-route>

When you say you want it to reflect/rebind, what functionality are you looking for?

[nomego]

Yes well regarding the model, it will look like that with my proposed change since model.router = router; will enrich the model in the way you describe later on in createModel().

Regarding the template instance, I would like to extend it with event handlers and code like:

<app-route path="/user/:name">
  <template>
    <p>Hello {{name}}</p>
    <paper-button on-click="{{ onButtonClick }}">Press me</paper-button>
    <paper-dialog id="myDialog">A lot of interesting information</paper-dialog>
  </template>
</app-route>

Now, if I assign model.onButtonClick = function () { window.alert('button clicked!'); } this code will not be triggered when the button is clicked, however, on a "real" template instance, it does work. (But not the current templateInstance.)

Going further, the polymer "automatic node finding" property $ would be nice to access:

template.onButtonClick = function () {
  this.$.myDialog.open();
}

I'll cancel my PR and redo the repo fork, and commit them to separate branches for each type of change.

[erikringsmuth]

I see. I'm going to dig through Polymer's auto-binding code this weekend and see if I can figure out what else they are doing. If I cant figure it out I'll merge your auto-binding root template code.

[erikringsmuth]

I spent a couple hours going over this and I'm running into problems. auto-binding templates get bound in the imported document AND the main document when the route is activated.

This imports the template rather than the instance of the template, but the instance is still created in the imported document.

importLink.import.querySelector('template')
...
templateInstance = document.importNode(template, true);
// rather than
templateInstance = document.importNode(template.content, true);

The script that get's run inside the auto-binding template is actually run once in the imported document the first time it's imported, then in the main document every time it's activated. I had a hunch that would be the case.

I also spent some time trying to figure out if the existing template binding could support things like this but had no luck.

<template>
  <paper-button on-click="{{ onButtonClick }}">Press me</paper-button>
  <paper-dialog id="myDialog">A lot of interesting information</paper-dialog>
  <script>
    template = ... // select the template
    template.onButtonClick = function () {
      this.$.myDialog.open();
    }
  </script>
</template>

There are also separate issues with inline auto-binding templates but I'm not going to bother with those.

So... I'm holding off on this for now. If you find a cleaner way, let me know.

[nomego]

Well the problem actually lies with document.importNode(). Every time that is run, a new template is being created (with a new script) and it auto-binds into a new instance.

Reverting the commit 623091f (and just assigning templateInstance = template; will just make the code run once since we "steal" the template and instance from the import document. Putting it into the main document DOM won't trigger the script again. This would actually allow for less memory usage as well since we can reuse the one instance. So we could actually change this to just "borrow" the instance and return it to the import document (I would assume?) instead of deleting it in removeRouteContent(). It would also enable us to leave pages in states, and restoring that state when the user comes back to a certain page. This for me would be a huge advantage since this is something our application requires as well (as I think more and more SPAs will).

Also, when it comes to the <template> / <script> relationship, I still haven't found a way to actually put the <script> within the root template because of how the paths in import/DOM context changes. So unless you always write inline scripts (which I don't really want to), the <script> tag must become a second (and third, fourth..) root tag after <template>. This will also only make it run once, regardless of the template duplication (or not) approach above.

[nomego]

I had a go at implementing this and as soon as I move the <template> element, it loses its binding magic, no matter if I just move it within its parent element or back to the html import document.

The approach I got to work was to simply leave any auto-binding templates beneath it's app-route once imported, and just re-inject any params when revisiting the view. (I guess for memory management and such, it doesn't matter if the template is stored in the import document or the main DOM?) With the current parameter approach, this could lead to parameters "lingering" from last visit unless overridden, but for app-router 3.0 with the params scoping, this would get solved as well.

Would you consider this approach to be a cleaner way? It would resemble the way inline templates are handled today, except that also the instance will be left in the DOM and reused.

It could probably be separated into its own keep argument/attribute to <app-route> elements to leave/reuse instances, which could be applicable for custom elements.

[erikringsmuth]

I set up examples and loading a route once auto-binds the template in the imported document, runs the script in the imported document, then the router clones the template with importNode(), attaches it to the document, then it auto-binds and runs the script inside the template again.

Persisting views in the background has been discussed before https://github.com/erikringsmuth/app-router/issues/36. I'm not a fan of the idea. State doesn't get reset, there is no way to use teardown methods like detached(), and it's still growing the DOM. If we allow for this it needs to be an additional feature and definitely not the default behavior.

[nomego]

Ok so as far as auto-binding goes, I think the cleanest way to approach this is to configure auto-binding on the routes instead.

If we introduce an attribute like autobind on <app-route> elements, app-router can "adjust" the imported template to be autobinding:

activateTemplate()

    if (route.autobind) {
      template.setAttribute('is', 'auto-binding');
      templateInstance = document.importNode(template, true);
      var model = createModel(router, route, url, eventDetail);
      injectModelProperties(templateInstance, model);
    } else if ('createInstance' in template) {

And the template itself would have to be written as <template> just like today, but app-router can adjust/upgrade it to auto-binding based on the app-route property. This way, it won't auto-bind during the HTML import, and not when setting the "is" attribute on the template node in the html import either, but rather when an importNode() version is attached to the DOM. But we'd still get the opportunity to access the template object itself from the DOM.

Well for persisting views, if keeping the state is the objective, the first two arguments doesn't make much sense, and it doesn't have to grow the DOM if we just steal the element from the import DOM. (It's in one DOM or the other..) I haven't really looked into how this is handled for custom elements but I'm sure something similar to my previous comments is possible.

I'd be glad to create another PR for an app-route keep or persist attribute which would let the view live on in the main DOM once loaded, but I'd want to come to a solution with the auto-binding stuff first. :)

[erikringsmuth]

I don't think a separate attribute makes sense. Most people wouldn't understand the difference between a template created with createInstance() vs is="auto-binding". Maybe if we could detect if is="auto-binding" is available, then do a 3 tier fallback from is="auto-binding" to createInstance() to importNode(). We would need to make sure this works for imported and inline templates and make sure it works when navigating to the route multiple times.

Another note. You can't steal or move elements between documents, you can only clone and add additional elements. document.importNode() clones an element.

if (route.autobind) {
  template.setAttribute('is', 'auto-binding');
  templateInstance = document.importNode(template, true);
  var model = createModel(router, route, url, eventDetail);
  injectModelProperties(templateInstance, model);
} else if ('createInstance' in template) {

templateInstance here isn't an instance, it is an exact clone of the template element. You have to call document.importNode(template.content, true) to create an instance. So actually in this case, we don't ever create the instance. Attaching a template with is="auto-binding" to the document will trigger creating the instance then.

Also, persisting elements will increase heap usage since the original template, a clone of the original template with is="auto-binding" added, and an instance of the template will all be present.

At this point I don't want to add persisting views as a feature. It feels like something that should be handled by conditional templates rather than a router.

[nomego]

So making auto-binding default if available? Well auto-binding is a Polymer feature, so this code would check for support:

if (typeof Polymer !== "undefined" && Polymer.elements) {
  for (var i = 0; i < Polymer.elements.length; i += 1) {
    if (Polymer.elements[i].name === 'auto-binding') {
      hasAutoBinding = true;
      break;
    }
  }
}

Or it could be an attribute to the template itself, <template will="auto-bind"> will turn into <template is="auto-binding"> once imported. :)

Well actually you can steal the instance, but of course not with importNode(). The original commit doesn't use importNode() but rather just assigns templateInstance = template as I noted a few comments ago. This actually extracts the DOM structure and template instance from the import document when it's inserted into the main DOM, thus "stealing" it.

So it would actually not increase heap usage since it's not a clone, but the actual instance. Also, for other approaches where we just import a template and persist it, we could actually remove the import document since it wouldn't be needed anymore, freeing heap again.

Well I hope you change your mind when 3.0 is released and you get a PR for persisted views :) How would this be handled by conditional templates do you mean?

[erikringsmuth]

The problem with moving a template instance from the imported document to the main document with templateInstance = template is this is only useful if you do persist views since it would move it out of the imported document.

Persisting views will not save heap space. Even if we do delete the link to the imported document we will still have the persisted view in memory in the main document. It's the same amount of memory as it is today where we delete the instance but don't delete the link. Also, I'd want some way of testing that deleting an import link does actually allow the imported document to be garbage collected.

I thought about auto-binding some more and I think we would always need the original template plus a clone of the template with is="auto-binding" set. This would increase the heap space so I want to avoid it. I'm back to the point where we need to figure out what else auto-binding is doing on top of createInstance() and add just that to get event mapping and automatic node finding.

I'm not 100% against persisted views, but I don't like the fact that navigating to a URL will have different effects depending on if the router has previously loaded the page. There's always the possibility of left over state which I can see leading to bugs. If you need to persist state when transitioning between views and you can't represent the state in the URL, that's where I feel like it should be controlled outside of the router with something like conditional templates. It shouldn't be a route if navigating to the URL can't represent the full state of the page.

I want to stress that whatever gets designed has to first and foremost work without persisting the view. If persisted views are added it would be an additional feature but never, ever, ever required. The API has to work consistently.

[nomego]

I'm with you on paragraph 1 and 2, except that the current approach has a template in the import document and also an instance of it in the DOM, while you're on that page. And with a simple template, the instance would contain duplicates of everything within the <template> tags in the template, say 95% of the template. So it would be all your templates + 1 instance. Whereas persisted views will only be all your templates' instances.

Regarding paragraph 3, original + instance is the way the current templates are handled, so why would that be a bad thing for auto-binding templates? The difference between a regular template and an auto-binding is just the wrapping <template> tag/object? Well if the actual template contains the is="auto-binding" then you'd also have two instances, but I thought you wanted app-router to add that once the template was imported?

I agree with your points on persisting views, while I at the same time hope for a scenario where is-autobinding in template + persist view would mean templateInstance = template; + drop html import. (With possibility of invalidating the view, which would cause a new html import, which would fetch it from browser cache.)

Also, going back to the root issue. The current behavior for auto-binding templates is that any template <script> is run twice and two instances are created - one auto in html import and one with template.createInstance() during the activation. So the current PR that uses importNode() instead, will actually not change the behavior except that a template object will follow into the DOM, which was all I was really looking for anyway. Because apart from <script>s being run one time too many, it's also the issue of the src relative path difference, so in our use case, we won't be putting <script> tags within the template anyway. (But, as discussed, putting it after the template will import and run it once with the correct path, which is enough for our needs.)

Avoiding the template to be auto-bound in the html import will not work unless the template removes the is="auto-binding" attribute. So the case of root templates having that attribute will always be a scenario - supported or not, perfect or not. Any approach that have been closer to your approval has been a regular <template>, enhanced. Therefore I would vote to include the current PR as it is to fix the most crucial issue with auto-binding templates right now, and (maybe?) figure out a better (co-existing) alternative long term.

A (hacky) way of preventing the <script>s to run twice is to declare them as <script type="js/run-later"> or something else that the browser doesn't support, and then loop through importLink.import.querySelectorAll('script[type=js/run-later]'); and update the type to text/javascript. That way they would not be run on import but rather on any DOM import / activation. :)

[erikringsmuth]

root auto-binding

I'm not going to add features that I've already found bugs with. Root templates with is="auto-binding" run scripts in the imported document and the main document. That may not be an issue for you but that will come back and bite me with tons of GitHub issues later. That means root templates with is="auto-binding" are out for good.

activate with auto-binding

The problem using auto-binding templates to instantiate regular templates is that you'd have to use it like this.

Example route with a template.

<app-route ...>
  <template>
    <el-1 val="{{value}}"></el-1>
    <el-2 val="{{value}}"></el-2>
  </template>
</app-route>

Activated by creating a clone with is="auto-binding" and attaching the clone.

<app-route ...>
  <!-- original, we can't just add is="auto-binding" because it's already attached -->
  <template>
    <el-1 val="{{value}}"></el-1>
    <el-2 val="{{value}}"></el-2>
  </template>

  <!-- clone with is="auto-binding" -->
  <template is="auto-binding">
    <el-1 val="{{value}}"></el-1>
    <el-2 val="{{value}}"></el-2>
  </template>

  <!-- instance -->
  <el-1 val="{{value}}"></el-1>
  <el-2 val="{{value}}"></el-2>
</app-route>

Which will result in an extra template instance. This is why I'd want to avoid is="auto-binding" and figure out what else needs to be added when we call createInstance().

persisted views

The scenario where we would import, move the instance to the main document, and delete the import link still would just barely beat the current implementation for heap usage.

Right now you have all of the imported documents + the instance for the current view. (n definitions + 1 instance)

In the import + move + unimport approach you still have persisted instances of all of the documents. The only space you save is the fact that the current view is only the instance rather than the original + the instance. That is a trivial amount of space saved for all of the added complexity and potential bugs introduced by persisting views. (n instances)

script type="js/run-later"

It's a hack. It could definitely work but I won't put it in the main router. It'd be a valid workaround for your usecase though, but you'd have to write the code in the activate-route-end event.

[nomego]

Ok well I feel that we're pinning stuff down anyways. :)

I agree with all your points except that I can't reproduce your "activate with auto-binding", what I get is:

<app-route ...>
  <!-- original, we can't just add is="auto-binding" because it's already attached -->
  <template>
    <el-1 val="{{value}}"></el-1>
    <el-2 val="{{value}}"></el-2>
  </template>

  <!-- clone with is="auto-binding" -->
  <template is="auto-binding" bind>
    #document-fragment
  </template>

  <!-- instance -->
  <el-1 val="{{value}}"></el-1>
  <el-2 val="{{value}}"></el-2>
</app-route>

So it should still be only one original template and one instance (+ controlling template tag).

Well, I think me and @plequang will sort out what feature branches you might be interested in and then maintain a fork with our auto-binding/persist features. Hope everything else gets merged! :)

[nomego]

I guess these discussions are done.