Skip to content

Commit 1b04f1b

Browse files
committed
fix(compiler): clone templates before compiling them
This is needed as the compiler changes templates during compilation and we are caching templates in the `TemplateLoader`. Closes angular#1058
1 parent f75a50c commit 1b04f1b

File tree

2 files changed

+40
-4
lines changed

2 files changed

+40
-4
lines changed

modules/angular2/src/render/dom/compiler/compiler.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ export class Compiler {
4747
_compileTemplate(viewDef: ViewDefinition, tplElement):Promise<ProtoViewDto> {
4848
var subTaskPromises = [];
4949
var pipeline = new CompilePipeline(this._stepFactory.createSteps(viewDef, subTaskPromises));
50-
var compileElements = pipeline.process(tplElement, viewDef.componentId);
50+
// we need to clone the template element as we modify it
51+
// and it might be given to use again as the TemplateLoader
52+
// uses an internal cache.
53+
var compileElements = pipeline.process(DOM.clone(tplElement), viewDef.componentId);
5154

5255
var protoView = compileElements[0].inheritedProtoView.build();
5356

modules/angular2/test/render/dom/compiler/compiler_common_tests.js

+36-3
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,34 @@ export function runCompilerCommonTests() {
9595
});
9696
}));
9797

98+
it('should not modify the cached url template', inject([AsyncTestCompleter], (async) => {
99+
var urlData = MapWrapper.createFromStringMap({
100+
'someUrl': '<div>a</div>'
101+
});
102+
var log = [];
103+
var logAndModifyCompileStep = (parent, current, control) => {
104+
var tagName = DOM.tagName(current.element).toUpperCase();
105+
if (tagName == 'DIV') {
106+
ListWrapper.push(log, DOM.getInnerHTML(current.element));
107+
DOM.setInnerHTML(current.element, '');
108+
}
109+
};
110+
var compiler = createCompiler(logAndModifyCompileStep, urlData);
111+
PromiseWrapper.all([
112+
compiler.compile(new ViewDefinition({
113+
componentId: 'someId',
114+
absUrl: 'someUrl'
115+
})),
116+
compiler.compile(new ViewDefinition({
117+
componentId: 'someId',
118+
absUrl: 'someUrl'
119+
}))
120+
]).then( (_) => {
121+
expect(log).toEqual(['a', 'a']);
122+
async.done();
123+
});
124+
}));
125+
98126
it('should report loading errors', inject([AsyncTestCompleter], (async) => {
99127
var compiler = createCompiler(EMPTY_STEP, MapWrapper.create());
100128
PromiseWrapper.catchError(compiler.compile(new ViewDefinition({
@@ -172,10 +200,15 @@ var EMPTY_STEP = (parent, current, control) => {
172200
};
173201

174202
class FakeTemplateLoader extends TemplateLoader {
175-
_urlData: Map<string, string>;
203+
_urlData: Map<string, any>;
176204
constructor(urlData) {
177205
super(null, new UrlResolver());
178-
this._urlData = urlData;
206+
this._urlData = MapWrapper.create();
207+
// Important: create the template elements only once,
208+
// as this is the behavior of the real template loader.
209+
MapWrapper.forEach(urlData, (content, url) => {
210+
MapWrapper.set(this._urlData, url, DOM.createTemplate(content));
211+
});
179212
}
180213

181214
load(template: ViewDefinition) {
@@ -186,7 +219,7 @@ class FakeTemplateLoader extends TemplateLoader {
186219
if (isPresent(template.absUrl)) {
187220
var content = MapWrapper.get(this._urlData, template.absUrl);
188221
if (isPresent(content)) {
189-
return PromiseWrapper.resolve(DOM.createTemplate(content));
222+
return PromiseWrapper.resolve(content);
190223
}
191224
}
192225

0 commit comments

Comments
 (0)