test(compiler): Add a test for handling of duplicate bindings (#53596)

Adds a test for handling of duplicate bindings. Fow now we replicate the
TDB behavior in template pipeline, which is: For style and class text
attributes, only keep the last one. For all other text attributes, add
all of the values to the consts array.

PR Close #53596
This commit is contained in:
Miles Malerba 2023-12-15 15:07:49 -08:00 committed by Jessica Janiuk
parent 4c8e8e3714
commit 2ef1bb960e
6 changed files with 103 additions and 2 deletions

View file

@ -110,6 +110,18 @@
]
}
]
},
{
"description": "should handle duplicate bindings",
"inputFiles": [
"duplicate_bindings.ts"
],
"expectations": [
{
"failureMessage": "Incorrect handling of duplicate bindings"
}
],
"focusTest": true
}
]
}

View file

@ -0,0 +1,19 @@
consts: [["aria-label", "hello", "aria-label", "hi"], [2, "height", "0"], [1, "cls2"], [3, "tabindex"]],
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
i0.ɵɵelement(0, "div", 0);
i0.ɵɵelementStart(1, "div", 1);
i0.ɵɵelement(2, "div", 2)(3, "div")(4, "div", 3)(5, "div")(6, "div");
i0.ɵɵelementEnd();
}
if (rf & 2) {
i0.ɵɵadvance(3);
i0.ɵɵattribute("aria-label", ctx.value1)("aria-label", ctx.value2);
i0.ɵɵadvance(1);
i0.ɵɵproperty("tabindex", ctx.value1)("tabindex", ctx.value2);
i0.ɵɵadvance(1);
i0.ɵɵclassMap(ctx.value2);
i0.ɵɵadvance(1);
i0.ɵɵstyleMap(ctx.value2);
}
}

View file

@ -0,0 +1,19 @@
import {Component} from '@angular/core';
@Component({
selector: 'my-component',
standalone: true,
template: `
<div aria-label="hello" aria-label="hi"></div>
<div style="width: 0" style="height: 0">
<div class="cls1" class="cls2"></div>
<div [attr.aria-label]="value1" [attr.aria-label]="value2"></div>
<div [tabindex]="value1" [tabindex]="value2"></div>
<div [class]="value1" [class]="value2"></div>
<div [style]="value1" [style]="value2"></div>
`,
})
export class MyComponent {
value1: any;
value2: any;
}

View file

@ -25,6 +25,7 @@ import {collectElementConsts} from './phases/const_collection';
import {convertI18nBindings} from './phases/convert_i18n_bindings';
import {createDeferDepsFns} from './phases/create_defer_deps_fns';
import {createI18nContexts} from './phases/create_i18n_contexts';
import {deduplicateTextBindings} from './phases/deduplicate_text_bindings';
import {configureDeferInstructions} from './phases/defer_configs';
import {resolveDeferTargetNames} from './phases/defer_resolve_targets';
import {collapseEmptyInstructions} from './phases/empty_elements';
@ -92,6 +93,7 @@ const phases: Phase[] = [
{kind: Kind.Tmpl, fn: emitNamespaceChanges},
{kind: Kind.Tmpl, fn: propagateI18nBlocks},
{kind: Kind.Tmpl, fn: wrapI18nIcus},
{kind: Kind.Both, fn: deduplicateTextBindings},
{kind: Kind.Both, fn: specializeStyleBindings},
{kind: Kind.Both, fn: specializeBindings},
{kind: Kind.Both, fn: extractAttributes},

View file

@ -25,7 +25,8 @@ export function collectElementConsts(job: CompilationJob): void {
for (const unit of job.units) {
for (const op of unit.create) {
if (op.kind === ir.OpKind.ExtractedAttribute) {
const attributes = allElementAttributes.get(op.target) || new ElementAttributes();
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);
ir.OpList.remove<ir.CreateOp>(op);
@ -102,6 +103,8 @@ class ElementAttributes {
return this.byKind.get(ir.BindingKind.I18n) ?? FLYWEIGHT_ARRAY;
}
constructor(private compatibility: ir.CompatibilityMode) {}
isKnown(kind: ir.BindingKind, name: string, value: o.Expression|null) {
const nameToValue = this.known.get(kind) ?? new Set<string>();
this.known.set(kind, nameToValue);
@ -114,7 +117,12 @@ class ElementAttributes {
add(kind: ir.BindingKind, name: string, value: o.Expression|null,
trustedValueFn: o.Expression|null): void {
if (this.isKnown(kind, name, value)) {
// In compatibility mode, we allow duplicates for attributes to replicate the behavior in
// TemplateDefinitionBuilder.
const allowDuplicates = this.compatibility === ir.CompatibilityMode.TemplateDefinitionBuilder ?
kind === ir.BindingKind.Attribute :
false;
if (this.isKnown(kind, name, value) && !allowDuplicates) {
return;
}

View file

@ -0,0 +1,41 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import * as ir from '../../ir';
import type {CompilationJob} from '../compilation';
/**
* Deduplicate text bindings, e.g. <div class="cls1" class="cls2">
*/
export function deduplicateTextBindings(job: CompilationJob): void {
const seen = new Map<ir.XrefId, Set<string>>();
for (const unit of job.units) {
for (const op of unit.update.reversed()) {
if (op.kind === ir.OpKind.Binding && op.isTextAttribute) {
const seenForElement = seen.get(op.target) || new Set();
if (seenForElement.has(op.name)) {
if (job.compatibility === ir.CompatibilityMode.TemplateDefinitionBuilder) {
// For most duplicated attributes, TemplateDefinitionBuilder lists all of the values in
// the consts array. However, for style and class attributes it only keeps the last one.
// We replicate that behavior here since it has actual consequences for apps with
// duplicate class or style attrs.
if (op.name === 'style' || op.name === 'class') {
ir.OpList.remove<ir.UpdateOp>(op);
}
} else {
// TODO: Determine the correct behavior. It would probably make sense to merge multiple
// style and class attributes. Alternatively we could just throw an error, as HTML
// doesn't permit duplicate attributes.
}
}
seenForElement.add(op.name);
seen.set(op.target, seenForElement);
}
}
}
}