From f8c075ae27cad8191c3571b9d23e0bce3814c93d Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Fri, 17 Mar 2017 15:34:37 -0700 Subject: [PATCH] =?UTF-8?q?fix(core):=20don=E2=80=99t=20create=20a=20comme?= =?UTF-8?q?nt=20for=20components=20with=20empty=20template.=20(#15260)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #15143 PR Close #15260 --- .../src/view_compiler/view_compiler.ts | 23 +++++++------- packages/core/src/view/view.ts | 7 ++--- .../linker/regression_integration_spec.ts | 30 +++++++++++++++++++ 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/packages/compiler/src/view_compiler/view_compiler.ts b/packages/compiler/src/view_compiler/view_compiler.ts index 450fba5bd2..dbf2869ba6 100644 --- a/packages/compiler/src/view_compiler/view_compiler.ts +++ b/packages/compiler/src/view_compiler/view_compiler.ts @@ -166,10 +166,8 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { }); } templateVisitAll(this, astNodes); - if (astNodes.length === 0 || - (this.parent && needsAdditionalRootNode(astNodes[astNodes.length - 1]))) { - // if the view is empty, or an embedded view has a view container as last root nde, - // create an additional root node. + if (this.parent && (astNodes.length === 0 || needsAdditionalRootNode(astNodes))) { + // if the view is an embedded view, then we need to add an additional root node in some cases this.nodes.push(() => ({ sourceSpan: null, nodeDef: o.importExpr(createIdentifier(Identifiers.anchorDef)).callFn([ @@ -971,19 +969,20 @@ function depDef(dep: CompileDiDependencyMetadata): o.Expression { return flags === DepFlags.None ? expr : o.literalArr([o.literal(flags), expr]); } -function needsAdditionalRootNode(ast: TemplateAst): boolean { - if (ast instanceof EmbeddedTemplateAst) { - return ast.hasViewContainer; +function needsAdditionalRootNode(astNodes: TemplateAst[]): boolean { + const lastAstNode = astNodes[astNodes.length - 1]; + if (lastAstNode instanceof EmbeddedTemplateAst) { + return lastAstNode.hasViewContainer; } - if (ast instanceof ElementAst) { - if (ast.name === NG_CONTAINER_TAG && ast.children.length) { - return needsAdditionalRootNode(ast.children[ast.children.length - 1]); + if (lastAstNode instanceof ElementAst) { + if (lastAstNode.name === NG_CONTAINER_TAG && lastAstNode.children.length) { + return needsAdditionalRootNode(lastAstNode.children); } - return ast.hasViewContainer; + return lastAstNode.hasViewContainer; } - return ast instanceof NgContentAst; + return lastAstNode instanceof NgContentAst; } function lifecycleHookToNodeFlag(lifecycleHook: LifecycleHooks): NodeFlags { diff --git a/packages/core/src/view/view.ts b/packages/core/src/view/view.ts index aaa117cee9..9681446f36 100644 --- a/packages/core/src/view/view.ts +++ b/packages/core/src/view/view.ts @@ -24,10 +24,6 @@ export function viewDef( flags: ViewFlags, nodes: NodeDef[], updateDirectives?: ViewUpdateFn, updateRenderer?: ViewUpdateFn): ViewDefinition { // clone nodes and set auto calculated values - if (nodes.length === 0) { - throw new Error(`Illegal State: Views without nodes are not allowed!`); - } - const reverseChildNodes: NodeDef[] = new Array(nodes.length); let viewBindingCount = 0; let viewDisposableCount = 0; @@ -152,6 +148,9 @@ export function viewDef( function validateNode(parent: NodeDef, node: NodeDef, nodeCount: number) { const template = node.element && node.element.template; if (template) { + if (!template.lastRenderRootNode) { + throw new Error(`Illegal State: Embedded templates without nodes are not allowed!`); + } if (template.lastRenderRootNode && template.lastRenderRootNode.flags & NodeFlags.EmbeddedViews) { throw new Error( diff --git a/packages/core/test/linker/regression_integration_spec.ts b/packages/core/test/linker/regression_integration_spec.ts index cf571c4924..5028abdd5d 100644 --- a/packages/core/test/linker/regression_integration_spec.ts +++ b/packages/core/test/linker/regression_integration_spec.ts @@ -306,6 +306,36 @@ function declareTests({useJit}: {useJit: boolean}) { ctx.detectChanges(); expect(ctx.componentInstance.viewContainers.first).toBe(vc); }); + + describe('empty templates - #15143', () => { + it('should allow empty components', () => { + @Component({template: ''}) + class MyComp { + } + + const fixture = + TestBed.configureTestingModule({declarations: [MyComp]}).createComponent(MyComp); + fixture.detectChanges(); + + expect(fixture.debugElement.childNodes.length).toBe(0); + }); + + it('should allow empty embedded templates', () => { + @Component({template: ''}) + class MyComp { + } + + const fixture = + TestBed.configureTestingModule({declarations: [MyComp]}).createComponent(MyComp); + fixture.detectChanges(); + + // Note: We always need to create at least a comment in an embedded template, + // so we can append other templates after it. + // 1 comment for the anchor, + // 1 comment for the empty embedded template. + expect(fixture.debugElement.childNodes.length).toBe(2); + }); + }); }); }