fix(template_compiler): Fix erroneous cycle detection

tbosch · tbosch · commit eda4c3eb4c82 · 2016-01-14T23:08:30.000Z
Before, the check for cycles was wrong and lead to false positives. Fixes angular#6404 Closes angular#6474
diff --git 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,15 +172,14 @@ export class TemplateCompiler {
   private _compileComponentRuntime(cacheKey: any, compMeta: CompileDirectiveMetadata,
                                    viewDirectives: CompileDirectiveMetadata[],
                                    pipes: CompilePipeMetadata[],
-                                   compilingComponentCacheKeys: Set<any>): CompiledTemplate {
+                                   compilingComponentsPath: any[]): CompiledTemplate {
     let uniqViewDirectives = <CompileDirectiveMetadata[]>removeDuplicates(viewDirectives);
     let uniqViewPipes = <CompilePipeMetadata[]>removeDuplicates(pipes);
     var compiledTemplate = this._compiledTemplateCache.get(cacheKey);
     var done = this._compiledTemplateDone.get(cacheKey);
     if (isBlank(compiledTemplate)) {
       compiledTemplate = new CompiledTemplate();
       this._compiledTemplateCache.set(cacheKey, compiledTemplate);
-      compilingComponentCacheKeys.add(cacheKey);
       done = PromiseWrapper
                  .all([<any>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<Type>,
+                                         parentCompilingComponentsPath: any[],
                                          childPromises: Promise<any>[]) {
+    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
@@ -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(['<comp-with-2nested>']);
+                   expect(humanizedView['componentViews'][0]['elements'])
+                       .toEqual(['<comp1>', '<comp2>']);
+                   expect(humanizedView['componentViews'][0]['componentViews'][0]['elements'])
+                       .toEqual(['<a>', '<comp2>']);
+                   expect(humanizedView['componentViews'][0]['componentViews'][1]['elements'])
+                       .toEqual(['<b>']);
+
+                   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: '<b></b>', encapsulation: ViewEncapsulation.None})
+export class Comp2 {
+}
+
+@Component({selector: 'comp1', moduleId: THIS_MODULE_ID})
+@View({
+  template: '<a></a>, <comp2></comp2>',
+  encapsulation: ViewEncapsulation.None,
+  directives: [Comp2]
+})
+export class Comp1 {
+}
+
+@Component({selector: 'comp-with-2nested', moduleId: THIS_MODULE_ID})
+@View({
+  template: '<comp1></comp1>, <comp2></comp2>',
+  encapsulation: ViewEncapsulation.None,
+  directives: [Comp1, Comp2]
+})
+export class CompWith2NestedComps {
+}
+
 function testableTemplateModule(sourceModule: SourceModule,
                                 normComp: CompileDirectiveMetadata): SourceModule {
   var testableSource = `
