Internet Explorer: ViewFactory.create does not clone child template content
- Dominant language
- TypeScript
- Stars
- 113
- Forks
- 101
- PR merge metrics
- No merged PRs in 30d
Description
The `ViewFactory` class is responsible for creating new `View` instances. Each time the view factory creates a view it uses the [cloneNode](https://developer.mozilla.org/en-US/docs/Web/API/Node/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 `` elements. Internet Explorer doesn't have true `` element support, which means it's `cloneNode` logic doesn't clone the [`content: DocumentFragment`](https://developer.mozilla.org/en-US/docs/Web/API/HTMLTemplateElement/content) property.
Here's a [plunker](https://plnkr.co/edit/JbnJOGh7Ge2baFYB2elG?p=preview) 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](https://plnkr.co/edit/JCAByeyLqzwtyxgL1A4I?p=preview)
```js
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 `` 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.
Contributor guide
Research direction
Start at the ViewFactory.create entry point and review the cloneNode behavior described in the issue, using the linked Plunker reproduction in Internet Explorer. Compare the nested template case with a template without child templates, and consider the issue's proposed behavior complete when child template content is preserved without the stack overflow.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, typescript
- Domain
- frontend, web-dev
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100