Internet Explorer: ViewFactory.create does not clone child template content

[jdanyow]

The ViewFactory class is responsible for creating new View instances. Each time the view factory creates a view it uses the cloneNode method to make a deep clone of a component's html template. The clone is what is ultimately data-bound to the view model instance and attached to the DOM.

In Internet Explorer there's an issue with cloning templates that contain other <template> elements. Internet Explorer doesn't have true <template> element support, which means it's cloneNode logic doesn't clone the content: DocumentFragment property.

Here's a plunker that demonstrates how this can cause a stack overflow in Internet Explorer. This issue was originally reported in #460.

One way we could fix this issue is to update ViewFactory.create's cloneNode logic. Here's what that might look like:

plunker

import {ViewFactory} from 'aurelia-templating';
import {FEATURE} from 'aurelia-pal';

ViewFactory.prototype.standardCreate = ViewFactory.prototype.create;
ViewFactory.prototype.create = function(container, createInstruction, element) {
  if (!FEATURE.htmlTemplateElement && !this.template.__safeToCloneNode) {
    const templates = this.template.querySelectorAll('template');
    if (templates.length === 0) {
      this.template.__safeToCloneNode = true;
    } else {
      this.template.standardCloneNode = this.template.cloneNode;
      this.template.cloneNode = function(deep) {
        const clone = this.standardCloneNode(deep);
        if (deep) {
          const clonedTemplates = clone.querySelectorAll('template');
          let i = clonedTemplates.length;
          while (i--) {
            clonedTemplates.item(i).content = templates.item(i).content;
          }
        }
        return clone;
      };
      this.template.__safeToCloneNode = true;
    }
  }
  
  return this.standardCreate(container, createInstruction, element);
};

This adds logic to check whether we have true <template> element support and lazily polyfilling content property cloning support in individual template instances, on an as-needed basis. When the template does not have any child elements, no polyfilling is needed.

@EisenbergEffect could you review this when you get a chance? Several developers have successfully patched their projects with this fix. If this approach looks good I'll continue working on landing this fix or helping someone else take it the rest of the way. This is a good one for learning the templating internals.

[EisenbergEffect]

Makes sense. Maybe @bigopon wants to work on this?

[bigopon]

Maybe it's more reasonable to wait for someone who actually fought this issue to PR ? I'll be fallback fixer 😄 after few days / a week

[StrahilKazlachev]

@jdanyow I do not understand how this clonedTemplates.item(i).content = templates.item(i).content; is cloning. Isn't this making the original and cloned <template> element hold the same #document-fragment, or is that irrelevant?

[jdanyow]

That's intentional, all we do with the content property is call content.cloneNode(true). No need to clone it here, just need to make sure the property exists.

[gheoan]

The fix should definitely be in pal-browser as suggested by @StrahilKazlachev, not in the ViewFactory.