From b5c5b4d94d419ab1dd1a1ec8c78c59119f0fcc8d Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Tue, 19 Dec 2023 15:31:11 -0800 Subject: [PATCH] refactor(compiler): Fix handling of namespaced attributes (#53646) It's possible for attributes to have a namespace, we need to handle this possiblity for both attribute instructions and attributes extracted to the consts array. PR Close #53646 --- .../elements/GOLDEN_PARTIAL.js | 34 +++++++++++++++++++ .../elements/TEST_CASES.json | 3 +- .../elements/namespace_attr.js | 6 ++-- .../elements/namespace_attr.ts | 5 ++- .../template/pipeline/ir/src/ops/create.ts | 10 ++++-- .../template/pipeline/ir/src/ops/update.ts | 8 ++++- .../src/template/pipeline/src/ingest.ts | 4 +-- .../src/template/pipeline/src/instruction.ts | 10 ++++-- .../src/phases/attribute_extraction.ts | 9 ++--- .../src/phases/binding_specialization.ts | 8 +++-- .../pipeline/src/phases/const_collection.ts | 20 +++++------ .../src/phases/parse_extracted_styles.ts | 4 +-- .../src/template/pipeline/src/phases/reify.ts | 2 +- 13 files changed, 87 insertions(+), 36 deletions(-) diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/GOLDEN_PARTIAL.js index 94682744bf1eed..078c2be9c26d9b 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/GOLDEN_PARTIAL.js @@ -718,3 +718,37 @@ export declare class MyComponent { static ɵcmp: i0.ɵɵComponentDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: namespace_attr.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyComponent { +} +MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, isStandalone: true, selector: "my-component", ngImport: i0, template: ` + + + `, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{ + type: Component, + args: [{ + selector: 'my-component', + standalone: true, + template: ` + + + ` + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: namespace_attr.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyComponent { + value: any; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/TEST_CASES.json index d4bfeb9c11f640..31f925ce05bd0b 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/TEST_CASES.json @@ -275,8 +275,7 @@ { "failureMessage": "Incorrect generated template." } - ], - "focusTest": true + ] } ] } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.js b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.js index 0736b215d7a57c..debf11b7501eca 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.js @@ -1,9 +1,9 @@ -consts: [["id", ""]], +consts: [["id", "foo", __AttributeMarker.NamespaceURI__, "xlink", "href", "/foo", "name", "foo"]], template: function MyComponent_Template(rf, ctx) { if (rf & 1) { i0.ɵɵnamespaceSVG(); - i0.ɵɵelement(0, "use", null, 0); + i0.ɵɵelement(0, "use")(1, "use", 0); } if (rf & 2) { i0.ɵɵattribute("href", ctx.value, null, "xlink"); } -} \ No newline at end of file +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.ts b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.ts index c1d95237c47c75..3347ee99297fcf 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.ts +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/namespace_attr.ts @@ -3,7 +3,10 @@ import {Component} from '@angular/core'; @Component({ selector: 'my-component', standalone: true, - template: `` + template: ` + + + ` }) export class MyComponent { value: any; diff --git a/packages/compiler/src/template/pipeline/ir/src/ops/create.ts b/packages/compiler/src/template/pipeline/ir/src/ops/create.ts index 595f4ec4cfa137..fb806391df5e6b 100644 --- a/packages/compiler/src/template/pipeline/ir/src/ops/create.ts +++ b/packages/compiler/src/template/pipeline/ir/src/ops/create.ts @@ -683,6 +683,11 @@ export interface ExtractedAttributeOp extends Op { */ bindingKind: BindingKind; + /** + * The namespace of the attribute (or null if none). + */ + namespace: string|null; + /** * The name of the extracted attribute. */ @@ -716,13 +721,14 @@ export interface ExtractedAttributeOp extends Op { * Create an `ExtractedAttributeOp`. */ export function createExtractedAttributeOp( - target: XrefId, bindingKind: BindingKind, name: string, expression: o.Expression|null, - i18nContext: XrefId|null, i18nMessage: i18n.Message|null, + target: XrefId, bindingKind: BindingKind, namespace: string|null, name: string, + expression: o.Expression|null, i18nContext: XrefId|null, i18nMessage: i18n.Message|null, securityContext: SecurityContext|SecurityContext[]): ExtractedAttributeOp { return { kind: OpKind.ExtractedAttribute, target, bindingKind, + namespace, name, expression, i18nContext, diff --git a/packages/compiler/src/template/pipeline/ir/src/ops/update.ts b/packages/compiler/src/template/pipeline/ir/src/ops/update.ts index 449b5f84cede61..4e06a2cf5cd980 100644 --- a/packages/compiler/src/template/pipeline/ir/src/ops/update.ts +++ b/packages/compiler/src/template/pipeline/ir/src/ops/update.ts @@ -406,6 +406,11 @@ export interface AttributeOp extends Op { */ target: XrefId; + /** + * The namespace of the attribute (or null if none). + */ + namespace: string|null; + /** * The name of the attribute. */ @@ -456,13 +461,14 @@ export interface AttributeOp extends Op { * Create an `AttributeOp`. */ export function createAttributeOp( - target: XrefId, name: string, expression: o.Expression|Interpolation, + target: XrefId, namespace: string|null, name: string, expression: o.Expression|Interpolation, securityContext: SecurityContext|SecurityContext[], isTextAttribute: boolean, isStructuralTemplateAttribute: boolean, templateKind: TemplateKind|null, i18nMessage: i18n.Message|null, sourceSpan: ParseSourceSpan): AttributeOp { return { kind: OpKind.Attribute, target, + namespace, name, expression, securityContext, diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index 3e2a55f3ad8cd5..ca8cf4aaeecbd6 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -967,7 +967,7 @@ function ingestTemplateBindings( // Animation bindings are excluded from the structural template's const array. const securityContext = domSchema.securityContext(NG_TEMPLATE_TAG_NAME, output.name, false); unit.create.push(ir.createExtractedAttributeOp( - op.xref, ir.BindingKind.Property, output.name, null, null, null, securityContext)); + op.xref, ir.BindingKind.Property, null, output.name, null, null, null, securityContext)); } } @@ -1024,7 +1024,7 @@ function createTemplateBinding( // the ng-template's consts (e.g. for the purposes of directive matching). However, we should // not generate an update instruction for it. return ir.createExtractedAttributeOp( - xref, ir.BindingKind.Property, name, null, null, i18nMessage, securityContext); + xref, ir.BindingKind.Property, null, name, null, null, i18nMessage, securityContext); } if (!isTextBinding && (type === e.BindingType.Attribute || type === e.BindingType.Animation)) { diff --git a/packages/compiler/src/template/pipeline/src/instruction.ts b/packages/compiler/src/template/pipeline/src/instruction.ts index 1b71b64c1c4e4a..ee5c0f2592212d 100644 --- a/packages/compiler/src/template/pipeline/src/instruction.ts +++ b/packages/compiler/src/template/pipeline/src/instruction.ts @@ -331,10 +331,14 @@ export function property( } export function attribute( - name: string, expression: o.Expression, sanitizer: o.Expression|null): ir.UpdateOp { + name: string, expression: o.Expression, sanitizer: o.Expression|null, + namespace: string|null): ir.UpdateOp { const args = [o.literal(name), expression]; - if (sanitizer !== null) { - args.push(sanitizer); + if (sanitizer !== null || namespace !== null) { + args.push(sanitizer ?? o.literal(null)); + } + if (namespace !== null) { + args.push(o.literal(namespace)); } return call(Identifiers.attribute, args, null); } diff --git a/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts b/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts index b155a7277f127b..b7ad93ac31721c 100644 --- a/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts +++ b/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts @@ -40,7 +40,8 @@ export function extractAttributes(job: CompilationJob): void { ir.OpList.insertBefore( // Deliberaly null i18nMessage value ir.createExtractedAttributeOp( - op.target, bindingKind, op.name, /* expression */ null, /* i18nContext */ null, + op.target, bindingKind, null, op.name, /* expression */ null, + /* i18nContext */ null, /* i18nMessage */ null, op.securityContext), lookupElement(elements, op.target)); } @@ -56,7 +57,7 @@ export function extractAttributes(job: CompilationJob): void { op.expression instanceof ir.EmptyExpr) { ir.OpList.insertBefore( ir.createExtractedAttributeOp( - op.target, ir.BindingKind.Property, op.name, /* expression */ null, + op.target, ir.BindingKind.Property, null, op.name, /* expression */ null, /* i18nContext */ null, /* i18nMessage */ null, SecurityContext.STYLE), lookupElement(elements, op.target)); @@ -65,7 +66,7 @@ export function extractAttributes(job: CompilationJob): void { case ir.OpKind.Listener: if (!op.isAnimationListener) { const extractedAttributeOp = ir.createExtractedAttributeOp( - op.target, ir.BindingKind.Property, op.name, /* expression */ null, + op.target, ir.BindingKind.Property, null, op.name, /* expression */ null, /* i18nContext */ null, /* i18nMessage */ null, SecurityContext.NONE); if (job.kind === CompilationJobKind.Host) { @@ -122,7 +123,7 @@ function extractAttributeOp( const extractedAttributeOp = ir.createExtractedAttributeOp( op.target, op.isStructuralTemplateAttribute ? ir.BindingKind.Template : ir.BindingKind.Attribute, - op.name, op.expression, op.i18nContext, op.i18nMessage, op.securityContext); + op.namespace, op.name, op.expression, op.i18nContext, op.i18nMessage, op.securityContext); if (unit.job.kind === CompilationJobKind.Host) { // This attribute will apply to the enclosing host binding compilation unit, so order doesn't // matter. diff --git a/packages/compiler/src/template/pipeline/src/phases/binding_specialization.ts b/packages/compiler/src/template/pipeline/src/phases/binding_specialization.ts index 0eb38ee90efd49..1f8358e23a7e90 100644 --- a/packages/compiler/src/template/pipeline/src/phases/binding_specialization.ts +++ b/packages/compiler/src/template/pipeline/src/phases/binding_specialization.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {splitNsName} from '../../../../ml_parser/tags'; import * as ir from '../../ir'; import {CompilationJob, CompilationJobKind} from '../compilation'; @@ -44,12 +45,13 @@ export function specializeBindings(job: CompilationJob): void { const target = lookupElement(elements, op.target); target.nonBindable = true; } else { + const [namespace, name] = splitNsName(op.name); ir.OpList.replace( op, ir.createAttributeOp( - op.target, op.name, op.expression, op.securityContext, op.isTextAttribute, - op.isStructuralTemplateAttribute, op.templateKind, op.i18nMessage, - op.sourceSpan)); + op.target, namespace, name, op.expression, op.securityContext, + op.isTextAttribute, op.isStructuralTemplateAttribute, op.templateKind, + op.i18nMessage, op.sourceSpan)); } break; case ir.BindingKind.Property: diff --git a/packages/compiler/src/template/pipeline/src/phases/const_collection.ts b/packages/compiler/src/template/pipeline/src/phases/const_collection.ts index 42383a03e4f4b4..b07758243142fa 100644 --- a/packages/compiler/src/template/pipeline/src/phases/const_collection.ts +++ b/packages/compiler/src/template/pipeline/src/phases/const_collection.ts @@ -8,7 +8,6 @@ import * as core from '../../../../core'; -import {splitNsName} from '../../../../ml_parser/tags'; import * as o from '../../../../output/output_ast'; import * as ir from '../../ir'; @@ -28,7 +27,7 @@ export function collectElementConsts(job: CompilationJob): void { const attributes = allElementAttributes.get(op.target) || new ElementAttributes(job.compatibility); allElementAttributes.set(op.target, attributes); - attributes.add(op.bindingKind, op.name, op.expression, op.trustedValueFn); + attributes.add(op.bindingKind, op.name, op.expression, op.namespace, op.trustedValueFn); ir.OpList.remove(op); } } @@ -139,7 +138,7 @@ class ElementAttributes { return false; } - add(kind: ir.BindingKind, name: string, value: o.Expression|null, + add(kind: ir.BindingKind, name: string, value: o.Expression|null, namespace: string|null, trustedValueFn: o.Expression|null): void { // TemplateDefinitionBuilder puts duplicate attribute, class, and style values into the consts // array. This seems inefficient, we can probably keep just the first one or the last value @@ -164,7 +163,7 @@ class ElementAttributes { const array = this.arrayFor(kind); - array.push(...getAttributeNameLiterals(name)); + array.push(...getAttributeNameLiterals(namespace, name)); if (kind === ir.BindingKind.Attribute || kind === ir.BindingKind.StyleProperty) { if (value === null) { throw Error('Attribute, i18n attribute, & style element attributes must have a value'); @@ -193,14 +192,11 @@ class ElementAttributes { /** * Gets an array of literal expressions representing the attribute's namespaced name. */ -function getAttributeNameLiterals(name: string): o.LiteralExpr[] { - const [attributeNamespace, attributeName] = splitNsName(name, false); - const nameLiteral = o.literal(attributeName); - - if (attributeNamespace) { - return [ - o.literal(core.AttributeMarker.NamespaceURI), o.literal(attributeNamespace), nameLiteral - ]; +function getAttributeNameLiterals(namespace: string|null, name: string): o.LiteralExpr[] { + const nameLiteral = o.literal(name); + + if (namespace) { + return [o.literal(core.AttributeMarker.NamespaceURI), o.literal(namespace), nameLiteral]; } return [nameLiteral]; diff --git a/packages/compiler/src/template/pipeline/src/phases/parse_extracted_styles.ts b/packages/compiler/src/template/pipeline/src/phases/parse_extracted_styles.ts index 4695046e0d4cfe..16aa28204925ad 100644 --- a/packages/compiler/src/template/pipeline/src/phases/parse_extracted_styles.ts +++ b/packages/compiler/src/template/pipeline/src/phases/parse_extracted_styles.ts @@ -49,7 +49,7 @@ export function parseExtractedStyles(job: CompilationJob) { for (let i = 0; i < parsedStyles.length - 1; i += 2) { ir.OpList.insertBefore( ir.createExtractedAttributeOp( - op.target, ir.BindingKind.StyleProperty, parsedStyles[i], + op.target, ir.BindingKind.StyleProperty, null, parsedStyles[i], o.literal(parsedStyles[i + 1]), null, null, SecurityContext.STYLE), op); } @@ -59,7 +59,7 @@ export function parseExtractedStyles(job: CompilationJob) { for (const parsedClass of parsedClasses) { ir.OpList.insertBefore( ir.createExtractedAttributeOp( - op.target, ir.BindingKind.ClassName, parsedClass, null, null, null, + op.target, ir.BindingKind.ClassName, null, parsedClass, null, null, null, SecurityContext.NONE), op); } diff --git a/packages/compiler/src/template/pipeline/src/phases/reify.ts b/packages/compiler/src/template/pipeline/src/phases/reify.ts index f90cefec5d8b46..0d556b084d380a 100644 --- a/packages/compiler/src/template/pipeline/src/phases/reify.ts +++ b/packages/compiler/src/template/pipeline/src/phases/reify.ts @@ -354,7 +354,7 @@ function reifyUpdateOperations(_unit: CompilationUnit, ops: ir.OpList