From 4d7a9db44c4be0bae4240591c6a529b66e88c682 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 2 Jan 2020 09:49:18 -0800 Subject: [PATCH] fix(ivy): Ensure ngProjectAs marker name appears at even attribute index (#34617) The `getProjectAsAttrValue` in `node_selector_matcher` finds the ProjectAs marker and then additionally checks that the marker appears in an even index of the node attributes because "attribute names are stored at even indexes". This is true for "regular" attribute bindings but classes, styles, bindings, templates, and i18n do not necessarily follow this rule because there can be an uneven number of them, causing the next "special" attribute "name" to appear at an odd index. To address this issue, ensure ngProjectAs is placed right after "regular" attributes. PR Close #34617 --- .../compliance/r3_compiler_compliance_spec.ts | 2 +- .../compiler/src/render3/view/template.ts | 74 +++++++------------ packages/core/test/acceptance/content_spec.ts | 44 +++++++++++ 3 files changed, 72 insertions(+), 48 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 7ef43a01fb..abb77063b8 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -1536,7 +1536,7 @@ describe('compiler compliance', () => { decls: 1, vars: 1, consts: [ - ["ngProjectAs", ".someclass", ${AttributeMarker.Template}, "ngIf", ${AttributeMarker.ProjectAs}, ["", 8, "someclass"]], + ["ngProjectAs", ".someclass", ${AttributeMarker.ProjectAs}, ["", 8, "someclass"], ${AttributeMarker.Template}, "ngIf"], ["ngProjectAs", ".someclass", ${AttributeMarker.ProjectAs}, ["", 8, "someclass"]] ], template: function MyApp_Template(rf, ctx) { diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 06f8d84887..d94c65d6dd 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -499,24 +499,12 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const slot = this.allocateDataSlot(); const projectionSlotIdx = this._ngContentSelectorsOffset + this._ngContentReservedSlots.length; const parameters: o.Expression[] = [o.literal(slot)]; - const attributes: o.Expression[] = []; - let ngProjectAsAttr: t.TextAttribute|undefined; this._ngContentReservedSlots.push(ngContent.selector); - ngContent.attributes.forEach((attribute) => { - const {name, value} = attribute; - if (name === NG_PROJECT_AS_ATTR_NAME) { - ngProjectAsAttr = attribute; - } - if (name.toLowerCase() !== NG_CONTENT_SELECT_ATTR) { - attributes.push(o.literal(name), o.literal(value)); - } - }); - - if (ngProjectAsAttr) { - attributes.push(...getNgProjectAsLiteral(ngProjectAsAttr)); - } + const nonContentSelectAttributes = + ngContent.attributes.filter(attr => attr.name.toLowerCase() !== NG_CONTENT_SELECT_ATTR); + const attributes = this.getAttributeExpressions(nonContentSelectAttributes, [], []); if (attributes.length > 0) { parameters.push(o.literal(projectionSlotIdx), o.literalArr(attributes)); @@ -540,7 +528,6 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const i18nAttrs: (t.TextAttribute | t.BoundAttribute)[] = []; const outputAttrs: t.TextAttribute[] = []; - let ngProjectAsAttr: t.TextAttribute|undefined; const [namespaceKey, elementName] = splitNsName(element.name); const isNgContainer = checkIsNgContainer(element.name); @@ -555,9 +542,6 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } else if (name === 'class') { stylingBuilder.registerClassAttr(value); } else { - if (attr.name === NG_PROJECT_AS_ATTR_NAME) { - ngProjectAsAttr = attr; - } if (attr.i18n) { // Place attributes into a separate array for i18n processing, but also keep such // attributes in the main list to make them available for directive matching at runtime. @@ -580,7 +564,6 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } // Add the attributes - const attributes: o.Expression[] = []; const allOtherInputs: t.BoundAttribute[] = []; element.inputs.forEach((input: t.BoundAttribute) => { @@ -598,13 +581,9 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } }); - outputAttrs.forEach(attr => { - attributes.push(...getAttributeNameLiterals(attr.name), o.literal(attr.value)); - }); - // add attributes for directive and projection matching purposes - attributes.push(...this.prepareNonRenderAttrs( - allOtherInputs, element.outputs, stylingBuilder, [], i18nAttrs, ngProjectAsAttr)); + const attributes: o.Expression[] = this.getAttributeExpressions( + outputAttrs, allOtherInputs, element.outputs, stylingBuilder, [], i18nAttrs); parameters.push(this.addAttrsToConsts(attributes)); // local refs (ex.:
) @@ -844,7 +823,6 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver visitTemplate(template: t.Template) { const NG_TEMPLATE_TAG_NAME = 'ng-template'; const templateIndex = this.allocateDataSlot(); - let ngProjectAsAttr: t.TextAttribute|undefined; if (this.i18n) { this.i18n.appendTemplate(template.i18n !, templateIndex); @@ -867,16 +845,9 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this.matchDirectives(NG_TEMPLATE_TAG_NAME, template); // prepare attributes parameter (including attributes used for directive matching) - const attrsExprs: o.Expression[] = []; - template.attributes.forEach((attr: t.TextAttribute) => { - if (attr.name === NG_PROJECT_AS_ATTR_NAME) { - ngProjectAsAttr = attr; - } - attrsExprs.push(asLiteral(attr.name), asLiteral(attr.value)); - }); - attrsExprs.push(...this.prepareNonRenderAttrs( - template.inputs, template.outputs, undefined, template.templateAttrs, undefined, - ngProjectAsAttr)); + const attrsExprs: o.Expression[] = this.getAttributeExpressions( + template.attributes, template.inputs, template.outputs, undefined, template.templateAttrs, + undefined); parameters.push(this.addAttrsToConsts(attrsExprs)); // local refs (ex.: ) @@ -1243,24 +1214,37 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver * * ``` * attrs = [prop, value, prop2, value2, + * PROJECT_AS, selector, * CLASSES, class1, class2, * STYLES, style1, value1, style2, value2, * BINDINGS, name1, name2, name3, * TEMPLATE, name4, name5, name6, - * PROJECT_AS, selector, * I18N, name7, name8, ...] * ``` * * Note that this function will fully ignore all synthetic (@foo) attribute values * because those values are intended to always be generated as property instructions. */ - private prepareNonRenderAttrs( - inputs: t.BoundAttribute[], outputs: t.BoundEvent[], styles?: StylingBuilder, - templateAttrs: (t.BoundAttribute|t.TextAttribute)[] = [], - i18nAttrs: (t.BoundAttribute|t.TextAttribute)[] = [], - ngProjectAsAttr?: t.TextAttribute): o.Expression[] { + private getAttributeExpressions( + renderAttributes: t.TextAttribute[], inputs: t.BoundAttribute[], outputs: t.BoundEvent[], + styles?: StylingBuilder, templateAttrs: (t.BoundAttribute|t.TextAttribute)[] = [], + i18nAttrs: (t.BoundAttribute|t.TextAttribute)[] = []): o.Expression[] { const alreadySeen = new Set(); const attrExprs: o.Expression[] = []; + let ngProjectAsAttr: t.TextAttribute|undefined; + + renderAttributes.forEach((attr: t.TextAttribute) => { + if (attr.name === NG_PROJECT_AS_ATTR_NAME) { + ngProjectAsAttr = attr; + } + attrExprs.push(...getAttributeNameLiterals(attr.name), asLiteral(attr.value)); + }); + + // Keep ngProjectAs next to the other name, value pairs so we can verify that we match + // ngProjectAs marker in the attribute name slot. + if (ngProjectAsAttr) { + attrExprs.push(...getNgProjectAsLiteral(ngProjectAsAttr)); + } function addAttrExpr(key: string | number, value?: o.Expression): void { if (typeof key === 'string') { @@ -1314,10 +1298,6 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver templateAttrs.forEach(attr => addAttrExpr(attr.name)); } - if (ngProjectAsAttr) { - attrExprs.push(...getNgProjectAsLiteral(ngProjectAsAttr)); - } - if (i18nAttrs.length) { attrExprs.push(o.literal(core.AttributeMarker.I18n)); i18nAttrs.forEach(attr => addAttrExpr(attr.name)); diff --git a/packages/core/test/acceptance/content_spec.ts b/packages/core/test/acceptance/content_spec.ts index a064b2e00c..9aef0532a4 100644 --- a/packages/core/test/acceptance/content_spec.ts +++ b/packages/core/test/acceptance/content_spec.ts @@ -955,6 +955,50 @@ describe('projection', () => { expect(fixture.nativeElement).toHaveText('hello'); }); + it('should support ngProjectAs with a various number of other bindings and attributes', () => { + @Directive({selector: '[color],[margin]'}) + class ElDecorator { + @Input() color?: string; + @Input() margin?: number; + } + @Component({ + selector: 'card', + template: ` + + --- + + --- + + --- + + ` + }) + class Card { + } + + @Component({ + selector: 'card-with-title', + template: ` + +

Title

+

Subtitle

+
content
+
footer
+ + ` + }) + class CardWithTitle { + } + + TestBed.configureTestingModule({declarations: [Card, CardWithTitle, ElDecorator]}); + const fixture = TestBed.createComponent(CardWithTitle); + fixture.detectChanges(); + + // Compare the text output, because Ivy and ViewEngine produce slightly different HTML. + expect(fixture.nativeElement.textContent) + .toContain('Title --- Subtitle --- content --- footer'); + }); + it('should support ngProjectAs on elements (including )', () => { @Component({ selector: 'card',