From a0585c9a9a8c505b91c68e446d5328f7be493e88 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Wed, 19 Dec 2018 15:03:36 +0100 Subject: [PATCH] fix(ivy): correct content projection with nested templates (#27755) Previously ivy code generation was emmiting the projectionDef instruction in a template where the tag was found. This code generation logic was incorrect since the ivy runtime expects the projectionDef instruction to be present in the main template only. This PR ammends the code generation logic so that the projectionDef instruction is emmitedin the main template only. PR Close #27755 --- .../compliance/r3_compiler_compliance_spec.ts | 341 +++++++++++------- .../compiler/src/render3/view/template.ts | 51 ++- .../linker/projection_integration_spec.ts | 67 +++- 3 files changed, 301 insertions(+), 158 deletions(-) diff --git a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts index f0edc25f51..4ca1f7181e 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -1092,156 +1092,225 @@ describe('compiler compliance', () => { }); }); - it('should support content projection in root template', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, Directive, NgModule, TemplateRef} from '@angular/core'; + describe('content projection', () => { - @Component({selector: 'simple', template: '
'}) - export class SimpleComponent {} - - @Component({ - selector: 'complex', - template: \` -
-
\` + it('should support content projection in root template', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, Directive, NgModule, TemplateRef} from '@angular/core'; + + @Component({selector: 'simple', template: '
'}) + export class SimpleComponent {} + + @Component({ + selector: 'complex', + template: \` +
+
\` + }) + export class ComplexComponent { } + + @NgModule({declarations: [SimpleComponent, ComplexComponent]}) + export class MyModule {} + + @Component({ + selector: 'my-app', + template: 'content ' }) - export class ComplexComponent { } + export class MyApp {} + ` + } + }; - @NgModule({declarations: [SimpleComponent, ComplexComponent]}) - export class MyModule {} + const SimpleComponentDefinition = ` + SimpleComponent.ngComponentDef = $r3$.ɵdefineComponent({ + type: SimpleComponent, + selectors: [["simple"]], + factory: function SimpleComponent_Factory(t) { return new (t || SimpleComponent)(); }, + consts: 2, + vars: 0, + template: function SimpleComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵprojectionDef(); + $r3$.ɵelementStart(0, "div"); + $r3$.ɵprojection(1); + $r3$.ɵelementEnd(); + } + }, + encapsulation: 2 + });`; - @Component({ - selector: 'my-app', - template: 'content ' - }) - export class MyApp {} - ` - } - }; + const ComplexComponentDefinition = ` + const $c3$ = ["id","first"]; + const $c4$ = ["id","second"]; + const $c1$ = [[["span", "title", "tofirst"]], [["span", "title", "tosecond"]]]; + const $c2$ = ["span[title=toFirst]", "span[title=toSecond]"]; + … + ComplexComponent.ngComponentDef = $r3$.ɵdefineComponent({ + type: ComplexComponent, + selectors: [["complex"]], + factory: function ComplexComponent_Factory(t) { return new (t || ComplexComponent)(); }, + consts: 4, + vars: 0, + template: function ComplexComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵprojectionDef($c1$, $c2$); + $r3$.ɵelementStart(0, "div", $c3$); + $r3$.ɵprojection(1, 1); + $r3$.ɵelementEnd(); + $r3$.ɵelementStart(2, "div", $c4$); + $r3$.ɵprojection(3, 2); + $r3$.ɵelementEnd(); + } + }, + encapsulation: 2 + }); + `; - const SimpleComponentDefinition = ` - SimpleComponent.ngComponentDef = $r3$.ɵdefineComponent({ - type: SimpleComponent, - selectors: [["simple"]], - factory: function SimpleComponent_Factory(t) { return new (t || SimpleComponent)(); }, - consts: 2, - vars: 0, - template: function SimpleComponent_Template(rf, ctx) { - if (rf & 1) { - $r3$.ɵprojectionDef(); - $r3$.ɵelementStart(0, "div"); - $r3$.ɵprojection(1); - $r3$.ɵelementEnd(); - } - }, - encapsulation: 2 - });`; + const result = compile(files, angularFiles); + const source = result.source; - const ComplexComponentDefinition = ` - const $c3$ = ["id","first"]; - const $c4$ = ["id","second"]; - const $c1$ = [[["span", "title", "tofirst"]], [["span", "title", "tosecond"]]]; - const $c2$ = ["span[title=toFirst]", "span[title=toSecond]"]; - … - ComplexComponent.ngComponentDef = $r3$.ɵdefineComponent({ - type: ComplexComponent, - selectors: [["complex"]], - factory: function ComplexComponent_Factory(t) { return new (t || ComplexComponent)(); }, - consts: 4, - vars: 0, - template: function ComplexComponent_Template(rf, ctx) { - if (rf & 1) { - $r3$.ɵprojectionDef($c1$, $c2$); - $r3$.ɵelementStart(0, "div", $c3$); + expectEmit( + result.source, SimpleComponentDefinition, 'Incorrect SimpleComponent definition'); + expectEmit( + result.source, ComplexComponentDefinition, 'Incorrect ComplexComponent definition'); + }); + + it('should support content projection in nested templates', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + template: \` +
+ +
+
+ No ng-content, no instructions generated. +
+ + '*' selector: + + \`, + }) + class Cmp {} + + @NgModule({ declarations: [Cmp] }) + class Module {} + ` + } + }; + const output = ` + const $_c0$ = [1, "ngIf"]; + const $_c1$ = ["id", "second"]; + function Cmp_div_Template_0(rf, ctx) { if (rf & 1) { + $r3$.ɵelementStart(0, "div", $_c1$); $r3$.ɵprojection(1, 1); $r3$.ɵelementEnd(); - $r3$.ɵelementStart(2, "div", $c4$); - $r3$.ɵprojection(3, 2); + } } + const $_c4$ = ["id", "third"]; + function Cmp_div_Template_1(rf, ctx) { + if (rf & 1) { + $r3$.ɵelementStart(0, "div", $_c4$); + $r3$.ɵtext(1, " No ng-content, no instructions generated. "); $r3$.ɵelementEnd(); } - }, - encapsulation: 2 - }); - `; - - const result = compile(files, angularFiles); - const source = result.source; - - expectEmit(result.source, SimpleComponentDefinition, 'Incorrect SimpleComponent definition'); - expectEmit( - result.source, ComplexComponentDefinition, 'Incorrect ComplexComponent definition'); - }); - - it('should support content projection in nested templates', () => { - const files = { - app: { - 'spec.ts': ` - import {Component, NgModule} from '@angular/core'; - - @Component({ - template: \` -
- -
-
- No ng-content, no instructions generated. -
- - '*' selector: - - \`, - }) - class Cmp {} - - @NgModule({ declarations: [Cmp] }) - class Module {} - ` - } - }; - const output = ` - const $_c0$ = [1, "ngIf"]; - const $_c1$ = ["id", "second"]; - const $_c2$ = [[["span", "title", "tofirst"]]]; - const $_c3$ = ["span[title=toFirst]"]; - function Cmp_div_Template_0(rf, ctx) { if (rf & 1) { - $r3$.ɵprojectionDef($_c2$, $_c3$); - $r3$.ɵelementStart(0, "div", $_c1$); - $r3$.ɵprojection(1, 1); - $r3$.ɵelementEnd(); - } } - const $_c4$ = ["id", "third"]; - function Cmp_div_Template_1(rf, ctx) { - if (rf & 1) { - $r3$.ɵelementStart(0, "div", $_c4$); - $r3$.ɵtext(1, " No ng-content, no instructions generated. "); - $r3$.ɵelementEnd(); } - } - function Cmp_ng_template_Template_2(rf, ctx) { - if (rf & 1) { - $r3$.ɵprojectionDef(); - $r3$.ɵtext(0, " '*' selector: "); - $r3$.ɵprojection(1); + function Cmp_ng_template_Template_2(rf, ctx) { + if (rf & 1) { + $r3$.ɵtext(0, " '*' selector: "); + $r3$.ɵprojection(1); + } } - } - … - template: function Cmp_Template(rf, ctx) { - if (rf & 1) { - $r3$.ɵtemplate(0, Cmp_div_Template_0, 2, 0, "div", $_c0$); - $r3$.ɵtemplate(1, Cmp_div_Template_1, 2, 0, "div", $_c0$); - $r3$.ɵtemplate(2, Cmp_ng_template_Template_2, 2, 0, "ng-template"); + const $_c2$ = [[["span", "title", "tofirst"]]]; + const $_c3$ = ["span[title=toFirst]"]; + … + template: function Cmp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵprojectionDef($_c2$, $_c3$); + $r3$.ɵtemplate(0, Cmp_div_Template_0, 2, 0, "div", $_c0$); + $r3$.ɵtemplate(1, Cmp_div_Template_1, 2, 0, "div", $_c0$); + $r3$.ɵtemplate(2, Cmp_ng_template_Template_2, 2, 0, "ng-template"); + } + if (rf & 2) { + $r3$.ɵelementProperty(0, "ngIf", $r3$.ɵbind(ctx.visible)); + $r3$.ɵelementProperty(1, "ngIf", $r3$.ɵbind(ctx.visible)); + } } - if (rf & 2) { - $r3$.ɵelementProperty(0, "ngIf", $r3$.ɵbind(ctx.visible)); - $r3$.ɵelementProperty(1, "ngIf", $r3$.ɵbind(ctx.visible)); - } - } - `; + `; + + const {source} = compile(files, angularFiles); + expectEmit(source, output, 'Invalid content projection instructions generated'); + }); + + it('should support content projection in both the root and nested templates', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + template: \` + + + + + + + + + '*' selector in a template: + + + \`, + }) + class Cmp {} + + @NgModule({ declarations: [Cmp] }) + class Module {} + ` + } + }; + + const output = ` + function Cmp_ng_template_ng_template_Template_1(rf, ctx) { + if (rf & 1) { + $r3$.ɵprojection(0, 4); + } + } + function Cmp_ng_template_Template_1(rf, ctx) { + if (rf & 1) { + $r3$.ɵprojection(0, 3); + $r3$.ɵtemplate(1, Cmp_ng_template_ng_template_Template_1, 1, 0, "ng-template"); + } + } + function Cmp_ng_template_Template_2(rf, ctx) { + if (rf & 1) { + $r3$.ɵtext(0, " '*' selector in a template: "); + $r3$.ɵprojection(1); + } + } + const $_c0$ = [[["", "id", "tomainbefore"]], [["", "id", "tomainafter"]], [["", "id", "totemplate"]], [["", "id", "tonestedtemplate"]]]; + const $_c1$ = ["[id=toMainBefore]", "[id=toMainAfter]", "[id=toTemplate]", "[id=toNestedTemplate]"]; + … + template: function Cmp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵprojectionDef($_c2$, $_c3$); + $r3$.ɵprojection(0, 1); + $r3$.ɵtemplate(1, Cmp_ng_template_Template_1, 2, 0, "ng-template"); + $r3$.ɵtemplate(2, Cmp_ng_template_Template_2, 2, 0, "ng-template"); + $r3$.ɵprojection(3, 2); + } + } + `; + + const {source} = compile(files, angularFiles); + expectEmit(source, output, 'Invalid content projection instructions generated'); + }); - const {source} = compile(files, angularFiles); - expectEmit(source, output, 'Invalid content projection instructions generated'); }); describe('queries', () => { diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 21ee059439..9ec9f62ab8 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -114,6 +114,10 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // Selectors found in the tags in the template. private _ngContentSelectors: string[] = []; + // Number of non-default selectors found in all parent templates of this template. We need to + // track it to properly adjust projection bucket index in the `projection` instruction. + private _ngContentSelectorsOffset = 0; + constructor( private constantPool: ConstantPool, parentBindingScope: BindingScope, private level = 0, private contextName: string|null, private i18nContext: I18nContext|null, @@ -166,7 +170,11 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver }); } - buildTemplateFunction(nodes: t.Node[], variables: t.Variable[], i18n?: i18n.AST): o.FunctionExpr { + buildTemplateFunction( + nodes: t.Node[], variables: t.Variable[], ngContentSelectorsOffset: number = 0, + i18n?: i18n.AST): o.FunctionExpr { + this._ngContentSelectorsOffset = ngContentSelectorsOffset; + if (this._namespace !== R3.namespaceHTML) { this.creationInstruction(null, this._namespace); } @@ -192,8 +200,23 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // resolving bindings. We also count bindings in this pass as we walk bound expressions. t.visitAll(this, nodes); - // Output a `ProjectionDef` instruction when some `` are present - if (this._hasNgContent) { + // Add total binding count to pure function count so pure function instructions are + // generated with the correct slot offset when update instructions are processed. + this._pureFunctionSlots += this._bindingSlots; + + // Pipes are walked in the first pass (to enqueue `pipe()` creation instructions and + // `pipeBind` update instructions), so we have to update the slot offsets manually + // to account for bindings. + this._valueConverter.updatePipeSlotOffsets(this._bindingSlots); + + // Nested templates must be processed before creation instructions so template() + // instructions can be generated with the correct internal const count. + this._nestedTemplateFns.forEach(buildTemplateFn => buildTemplateFn()); + + // Output the `projectionDef` instruction when some `` are present. + // The `projectionDef` instruction only emitted for the component template and it is skipped for + // nested templates ( tags). + if (this.level === 0 && this._hasNgContent) { const parameters: o.Expression[] = []; // Only selectors with a non-default value are generated @@ -212,19 +235,6 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this.creationInstruction(null, R3.projectionDef, parameters, /* prepend */ true); } - // Add total binding count to pure function count so pure function instructions are - // generated with the correct slot offset when update instructions are processed. - this._pureFunctionSlots += this._bindingSlots; - - // Pipes are walked in the first pass (to enqueue `pipe()` creation instructions and - // `pipeBind` update instructions), so we have to update the slot offsets manually - // to account for bindings. - this._valueConverter.updatePipeSlotOffsets(this._bindingSlots); - - // Nested templates must be processed before creation instructions so template() - // instructions can be generated with the correct internal const count. - this._nestedTemplateFns.forEach(buildTemplateFn => buildTemplateFn()); - if (initI18nContext) { this.i18nEnd(null, selfClosingI18nInstruction); } @@ -419,7 +429,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const slot = this.allocateDataSlot(); let selectorIndex = ngContent.selector === DEFAULT_NG_CONTENT_SELECTOR ? 0 : - this._ngContentSelectors.push(ngContent.selector); + this._ngContentSelectors.push(ngContent.selector) + this._ngContentSelectorsOffset; const parameters: o.Expression[] = [o.literal(slot)]; const attributeAsList: string[] = []; @@ -738,8 +748,13 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // template definition. e.g.
{{ foo }}
this._nestedTemplateFns.push(() => { const templateFunctionExpr = templateVisitor.buildTemplateFunction( - template.children, template.variables, template.i18n); + template.children, template.variables, + this._ngContentSelectors.length + this._ngContentSelectorsOffset, template.i18n); this.constantPool.statements.push(templateFunctionExpr.toDeclStmt(templateName, null)); + if (templateVisitor._hasNgContent) { + this._hasNgContent = true; + this._ngContentSelectors.push(...templateVisitor._ngContentSelectors); + } }); // e.g. template(1, MyComp_Template_1) diff --git a/packages/core/test/linker/projection_integration_spec.ts b/packages/core/test/linker/projection_integration_spec.ts index 105705b263..d41a092e41 100644 --- a/packages/core/test/linker/projection_integration_spec.ts +++ b/packages/core/test/linker/projection_integration_spec.ts @@ -182,7 +182,7 @@ describe('projection', () => { expect(main.nativeElement).toHaveText('OUTER(INNER(INNERINNER(A,BC)))'); }); - fixmeIvy('FW-796: Content projection logic is incorrect for in nested templates') + fixmeIvy('FW-833: Directive / projected node matching against class name') .it('should redistribute when the shadow dom changes', () => { TestBed.configureTestingModule( {declarations: [ConditionalContentComponent, ManualViewportDirective]}); @@ -290,7 +290,7 @@ describe('projection', () => { expect(main.nativeElement).toHaveText('SIMPLE()START(A)END'); }); - fixmeIvy('FW-796: Content projection logic is incorrect for in nested templates') + fixmeIvy('FW-833: Directive / projected node matching against class name') .it('should support moving ng-content around', () => { TestBed.configureTestingModule({ declarations: [ConditionalContentComponent, ProjectDirective, ManualViewportDirective] @@ -430,7 +430,7 @@ describe('projection', () => { }); } - fixmeIvy('FW-796: Content projection logic is incorrect for in nested templates') + fixmeIvy('FW-869: debugElement.queryAllNodes returns nodes in the wrong order') .it('should support nested conditionals that contain ng-contents', () => { TestBed.configureTestingModule( {declarations: [ConditionalTextComponent, ManualViewportDirective]}); @@ -474,7 +474,66 @@ describe('projection', () => { 'a2b21b22'); }); - fixmeIvy('FW-796: Content projection logic is incorrect for in nested templates') + it('should project nodes into nested templates when the main template doesn\'t have ', + () => { + + @Component({ + selector: 'content-in-template', + template: + `()` + }) + class ContentInATemplateComponent { + } + + + TestBed.configureTestingModule( + {declarations: [ContentInATemplateComponent, ManualViewportDirective]}); + TestBed.overrideComponent( + MainComp, + {set: {template: `
A
`}}); + + const main = TestBed.createComponent(MainComp); + + main.detectChanges(); + expect(main.nativeElement).toHaveText('()'); + + let viewportElement = + main.debugElement.queryAllNodes(By.directive(ManualViewportDirective))[0]; + viewportElement.injector.get(ManualViewportDirective).show(); + expect(main.nativeElement).toHaveText('(A)'); + }); + + it('should project nodes into nested templates and the main template', () => { + + @Component({ + selector: 'content-in-main-and-template', + template: + `()` + }) + class ContentInMainAndTemplateComponent { + } + + + TestBed.configureTestingModule( + {declarations: [ContentInMainAndTemplateComponent, ManualViewportDirective]}); + TestBed.overrideComponent(MainComp, { + set: { + template: + `
A
B` + } + }); + + const main = TestBed.createComponent(MainComp); + + main.detectChanges(); + expect(main.nativeElement).toHaveText('B()'); + + let viewportElement = main.debugElement.queryAllNodes(By.directive(ManualViewportDirective))[0]; + viewportElement.injector.get(ManualViewportDirective).show(); + expect(main.nativeElement).toHaveText('B(A)'); + }); + + fixmeIvy('FW-833: Directive / projected node matching against class name') .it('should project filled view containers into a view container', () => { TestBed.configureTestingModule( {declarations: [ConditionalContentComponent, ManualViewportDirective]});