From eda4c3eb4c823029b885fdfcd602a8c3c93f0d31 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Wed, 13 Jan 2016 17:11:43 -0800 Subject: [PATCH] fix(template_compiler): Fix erroneous cycle detection Before, the check for cycles was wrong and lead to false positives. Fixes #6404 Closes #6474 --- .../src/compiler/template_compiler.ts | 17 ++++---- .../test/compiler/template_compiler_spec.ts | 39 +++++++++++++++++++ 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/modules/angular2/src/compiler/template_compiler.ts b/modules/angular2/src/compiler/template_compiler.ts index 54a5fccef8..04a33eadbd 100644 --- a/modules/angular2/src/compiler/template_compiler.ts +++ b/modules/angular2/src/compiler/template_compiler.ts @@ -124,7 +124,7 @@ export class TemplateCompiler { var hostMeta: CompileDirectiveMetadata = createHostComponentMeta(compMeta.type, compMeta.selector); - this._compileComponentRuntime(hostCacheKey, hostMeta, [compMeta], [], new Set()); + this._compileComponentRuntime(hostCacheKey, hostMeta, [compMeta], [], []); } return this._compiledTemplateDone.get(hostCacheKey) .then((compiledTemplate: CompiledTemplate) => @@ -172,7 +172,7 @@ export class TemplateCompiler { private _compileComponentRuntime(cacheKey: any, compMeta: CompileDirectiveMetadata, viewDirectives: CompileDirectiveMetadata[], pipes: CompilePipeMetadata[], - compilingComponentCacheKeys: Set): CompiledTemplate { + compilingComponentsPath: any[]): CompiledTemplate { let uniqViewDirectives = removeDuplicates(viewDirectives); let uniqViewPipes = removeDuplicates(pipes); var compiledTemplate = this._compiledTemplateCache.get(cacheKey); @@ -180,7 +180,6 @@ export class TemplateCompiler { if (isBlank(compiledTemplate)) { compiledTemplate = new CompiledTemplate(); this._compiledTemplateCache.set(cacheKey, compiledTemplate); - compilingComponentCacheKeys.add(cacheKey); done = PromiseWrapper .all([this._styleCompiler.compileComponentRuntime(compMeta.template)].concat( uniqViewDirectives.map(dirMeta => this.normalizeDirectiveMetadata(dirMeta)))) @@ -195,14 +194,13 @@ export class TemplateCompiler { var usedDirectives = DirectiveCollector.findUsedDirectives(parsedTemplate); usedDirectives.components.forEach( component => this._compileNestedComponentRuntime( - component, compilingComponentCacheKeys, childPromises)); + component, compilingComponentsPath, childPromises)); return PromiseWrapper.all(childPromises) .then((_) => { var filteredPipes = filterPipes(parsedTemplate, uniqViewPipes); compiledTemplate.init(this._createViewFactoryRuntime( compMeta, parsedTemplate, usedDirectives.directives, styles, filteredPipes)); - SetWrapper.delete(compilingComponentCacheKeys, cacheKey); return compiledTemplate; }); }); @@ -212,16 +210,19 @@ export class TemplateCompiler { } private _compileNestedComponentRuntime(childComponentDir: CompileDirectiveMetadata, - compilingComponentCacheKeys: Set, + parentCompilingComponentsPath: any[], childPromises: Promise[]) { + var compilingComponentsPath = ListWrapper.clone(parentCompilingComponentsPath); + var childCacheKey = childComponentDir.type.runtime; var childViewDirectives: CompileDirectiveMetadata[] = this._runtimeMetadataResolver.getViewDirectivesMetadata(childComponentDir.type.runtime); var childViewPipes: CompilePipeMetadata[] = this._runtimeMetadataResolver.getViewPipesMetadata(childComponentDir.type.runtime); - var childIsRecursive = SetWrapper.has(compilingComponentCacheKeys, childCacheKey); + var childIsRecursive = ListWrapper.contains(compilingComponentsPath, childCacheKey); + compilingComponentsPath.push(childCacheKey); this._compileComponentRuntime(childCacheKey, childComponentDir, childViewDirectives, - childViewPipes, compilingComponentCacheKeys); + childViewPipes, compilingComponentsPath); if (!childIsRecursive) { // Only wait for a child if it is not a cycle childPromises.push(this._compiledTemplateDone.get(childCacheKey)); diff --git a/modules/angular2/test/compiler/template_compiler_spec.ts b/modules/angular2/test/compiler/template_compiler_spec.ts index aac4fdf523..b1d144fb82 100644 --- a/modules/angular2/test/compiler/template_compiler_spec.ts +++ b/modules/angular2/test/compiler/template_compiler_spec.ts @@ -105,6 +105,22 @@ export function main() { }); })); + it('should compile components at various nesting levels', + inject([AsyncTestCompleter], (async) => { + compile([CompWith2NestedComps, Comp1, Comp2]) + .then((humanizedView) => { + expect(humanizedView['elements']).toEqual(['']); + expect(humanizedView['componentViews'][0]['elements']) + .toEqual(['', '']); + expect(humanizedView['componentViews'][0]['componentViews'][0]['elements']) + .toEqual(['', '']); + expect(humanizedView['componentViews'][0]['componentViews'][1]['elements']) + .toEqual(['']); + + async.done(); + }); + })); + it('should compile recursive components', inject([AsyncTestCompleter], (async) => { compile([TreeComp]) .then((humanizedView) => { @@ -365,6 +381,29 @@ export class NonComponent { } +@Component({selector: 'comp2', moduleId: THIS_MODULE_ID}) +@View({template: '', encapsulation: ViewEncapsulation.None}) +export class Comp2 { +} + +@Component({selector: 'comp1', moduleId: THIS_MODULE_ID}) +@View({ + template: ', ', + encapsulation: ViewEncapsulation.None, + directives: [Comp2] +}) +export class Comp1 { +} + +@Component({selector: 'comp-with-2nested', moduleId: THIS_MODULE_ID}) +@View({ + template: ', ', + encapsulation: ViewEncapsulation.None, + directives: [Comp1, Comp2] +}) +export class CompWith2NestedComps { +} + function testableTemplateModule(sourceModule: SourceModule, normComp: CompileDirectiveMetadata): SourceModule { var testableSource = `