refactor(compiler): Support class and non-class attributes with the same name (#53574)

Some elements may have multiple bindings with the same name. We should accept and emit them all, as long as they have different kinds.

Co-authored-by: Miles Malerba <mmalerba@users.noreply.github.com>

PR Close #53574
This commit is contained in:
Dylan Hunn 2023-12-14 18:04:35 -08:00 committed by Jessica Janiuk
parent 39767e764f
commit 7ef5d9deef
7 changed files with 180 additions and 3 deletions

View file

@ -219,3 +219,50 @@ export declare class MyComponent {
static ɵcmp: i0.ɵɵComponentDeclaration<MyComponent, "my-component", never, {}, {}, never, never, true, never>;
}
/****************************************************************************************************
* PARTIAL FILE: shared_name_with_consts.js
****************************************************************************************************/
import { Component } from '@angular/core';
import * as i0 from "@angular/core";
export class MyComponent {
constructor() {
this.tabIndex = 0;
}
}
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: `
<div attr class="attr"></div>
<div ngProjectAs="selector" class="selector"></div>
<div style="width:0px" class="width"></div>
<div [tabindex]="tabIndex" class="tabindex"></div>
<div *ngIf="cond" class="ngIf"></div>
<div aria-label="label" i18n-aria-label class="aria-label"></div>
<div all ngProjectAs="all" style="all:all" [all]="all" *all="all" i18n-all class="all"></div>
`, 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: `
<div attr class="attr"></div>
<div ngProjectAs="selector" class="selector"></div>
<div style="width:0px" class="width"></div>
<div [tabindex]="tabIndex" class="tabindex"></div>
<div *ngIf="cond" class="ngIf"></div>
<div aria-label="label" i18n-aria-label class="aria-label"></div>
<div all ngProjectAs="all" style="all:all" [all]="all" *all="all" i18n-all class="all"></div>
`
}]
}] });
/****************************************************************************************************
* PARTIAL FILE: shared_name_with_consts.d.ts
****************************************************************************************************/
import * as i0 from "@angular/core";
export declare class MyComponent {
tabIndex: number;
static ɵfac: i0.ɵɵFactoryDeclaration<MyComponent, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyComponent, "my-component", never, {}, {}, never, never, true, never>;
}

View file

@ -70,6 +70,24 @@
]
}
]
},
{
"description": "should handle class that shares its name with other const array items",
"inputFiles": [
"shared_name_with_consts.ts"
],
"expectations": [
{
"files": [
{
"templatePipelineExpected": "shared_name_with_consts_template.pipeline.js",
"expected": "shared_name_with_consts_template.js",
"generated": "shared_name_with_consts.js"
}
],
"failureMessage": "Incorrect template"
}
]
}
]
}

View file

@ -0,0 +1,18 @@
import {Component} from '@angular/core';
@Component({
selector: 'my-component',
standalone: true,
template: `
<div attr class="attr"></div>
<div ngProjectAs="selector" class="selector"></div>
<div style="width:0px" class="width"></div>
<div [tabindex]="tabIndex" class="tabindex"></div>
<div *ngIf="cond" class="ngIf"></div>
<div aria-label="label" i18n-aria-label class="aria-label"></div>
<div all ngProjectAs="all" style="all:all" [all]="all" *all="all" i18n-all class="all"></div>
`
})
export class MyComponent {
tabIndex = 0;
}

View file

@ -0,0 +1,30 @@
consts: () => {
__i18nMsg__('label', [], {}, {})
return [
["attr", "", __AttributeMarker.Classes__, "attr"],
["ngProjectAs", "selector", __AttributeMarker.ProjectAs__, ["selector"], __AttributeMarker.Classes__, "selector"],
[__AttributeMarker.Classes__, "width", __AttributeMarker.Styles__, "width", "0px"],
[__AttributeMarker.Classes__, "tabindex", __AttributeMarker.Bindings__, "tabindex"],
["class", "ngIf", __AttributeMarker.Template__, "ngIf"],
["aria-label", i18n_0, __AttributeMarker.Classes__, "aria-label"],
["all", "", "ngProjectAs", "all", "style", "all:all", "class", "all", __AttributeMarker.ProjectAs__, ["all"], __AttributeMarker.Bindings__, "all", __AttributeMarker.Template__],
[__AttributeMarker.Classes__, "ngIf"],
["all", "", "ngProjectAs", "all", __AttributeMarker.ProjectAs__, ["all"], __AttributeMarker.Classes__, "all", __AttributeMarker.Styles__, "all", "all", __AttributeMarker.Bindings__, "all"]
];
},
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
i0.ɵɵelement(0, "div", 0)(1, "div", 1)(2, "div", 2)(3, "div", 3);
i0.ɵɵtemplate(4, MyComponent_div_4_Template, 1, 0, "div", 4);
i0.ɵɵelement(5, "div", 5);
i0.ɵɵtemplate(6, MyComponent_div_6_Template, 1, 1, "div", 6);
}
if (rf & 2) {
i0.ɵɵadvance(3);
i0.ɵɵproperty("tabindex", ctx.tabIndex);
i0.ɵɵadvance(1);
i0.ɵɵproperty("ngIf", ctx.cond);
i0.ɵɵadvance(2);
i0.ɵɵproperty("all", ctx.all);
}
}

View file

@ -0,0 +1,32 @@
consts: () => {
__i18nMsg__('label', [], {}, {})
return [
["attr", "", __AttributeMarker.Classes__, "attr"],
["ngProjectAs", "selector", __AttributeMarker.ProjectAs__, ["selector"], __AttributeMarker.Classes__, "selector"],
[__AttributeMarker.Classes__, "width", __AttributeMarker.Styles__, "width", "0px"],
[__AttributeMarker.Classes__, "tabindex", __AttributeMarker.Bindings__, "tabindex"],
["class", "ngIf", __AttributeMarker.Template__, "ngIf"],
["aria-label", i18n_0, __AttributeMarker.Classes__, "aria-label"],
// NOTE: We tolerate a slight difference -- we emit `all` as a Template binding
["all", "", "ngProjectAs", "all", "style", "all:all", "class", "all", __AttributeMarker.ProjectAs__, ["all"], __AttributeMarker.Bindings__, "all", __AttributeMarker.Template__, "all"],
[__AttributeMarker.Classes__, "ngIf"],
["all", "", "ngProjectAs", "all", __AttributeMarker.ProjectAs__, ["all"], __AttributeMarker.Classes__, "all", __AttributeMarker.Styles__, "all", "all", __AttributeMarker.Bindings__, "all"]
];
},
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
i0.ɵɵelement(0, "div", 0)(1, "div", 1)(2, "div", 2)(3, "div", 3);
i0.ɵɵtemplate(4, MyComponent_div_4_Template, 1, 0, "div", 4);
i0.ɵɵelement(5, "div", 5);
i0.ɵɵtemplate(6, MyComponent_div_6_Template, 1, 1, "div", 6);
}
if (rf & 2) {
i0.ɵɵadvance(3);
i0.ɵɵproperty("tabindex", ctx.tabIndex);
i0.ɵɵadvance(1);
i0.ɵɵproperty("ngIf", ctx.cond);
i0.ɵɵadvance(2);
i0.ɵɵproperty("all", ctx.all);
}
}

View file

@ -73,7 +73,7 @@ const FLYWEIGHT_ARRAY: ReadonlyArray<o.Expression> = Object.freeze<o.Expression[
* Container for all of the various kinds of attributes which are applied on an element.
*/
class ElementAttributes {
private known = new Set<string>();
private known = new Map<ir.BindingKind, Set<string>>();
private byKind = new Map<ir.BindingKind, o.Expression[]>;
projectAs: string|null = null;
@ -102,12 +102,22 @@ class ElementAttributes {
return this.byKind.get(ir.BindingKind.I18n) ?? FLYWEIGHT_ARRAY;
}
isKnown(kind: ir.BindingKind, name: string, value: o.Expression|null) {
const nameToValue = this.known.get(kind) ?? new Set<string>();
this.known.set(kind, nameToValue);
if (nameToValue.has(name)) {
return true;
}
nameToValue.add(name);
return false;
}
add(kind: ir.BindingKind, name: string, value: o.Expression|null,
trustedValueFn: o.Expression|null): void {
if (this.known.has(name)) {
if (this.isKnown(kind, name, value)) {
return;
}
this.known.add(name);
// TODO: Can this be its own phase
if (name === 'ngProjectAs') {
if (value === null || !(value instanceof o.LiteralExpr) || (value.value == null) ||

View file

@ -18,10 +18,32 @@ import type {CompilationJob} from '../compilation';
* class property.
*/
export function parseExtractedStyles(job: CompilationJob) {
const elements = new Map<ir.XrefId, ir.CreateOp>();
for (const unit of job.units) {
for (const op of unit.create) {
if (ir.isElementOrContainerOp(op)) {
elements.set(op.xref, op);
}
}
}
for (const unit of job.units) {
for (const op of unit.create) {
if (op.kind === ir.OpKind.ExtractedAttribute && op.bindingKind === ir.BindingKind.Attribute &&
ir.isStringLiteral(op.expression!)) {
const target = elements.get(op.target)!;
if (target !== undefined && target.kind === ir.OpKind.Template &&
target.templateKind === ir.TemplateKind.Structural) {
// TemplateDefinitionBuilder will not apply class and style bindings to structural
// directives; instead, it will leave them as attributes.
// (It's not clear what that would mean, anyway -- classes and styles on a structural
// element should probably be a parse error.)
// TODO: We may be able to remove this once Template Pipeline is the default.
continue;
}
if (op.name === 'style') {
const parsedStyles = parseStyle(op.expression.value);
for (let i = 0; i < parsedStyles.length - 1; i += 2) {