From 8fbe558cbc2321739f4ed97e8fb93bc05629d3cc Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 2 Jul 2025 14:04:44 +0200 Subject: [PATCH] perf(core): move property remapping for dom properties to compiler (#62421) Since we know that DOM properties won't go to an inputs, we can move the remapping logic to the compiler, saving us some processing on the client. PR Close #62421 --- .../attribute_bindings/duplicate_bindings.js | 2 +- .../chain_attribute_bindings_mixed.js | 2 +- .../chain_multiple_property_bindings.js | 2 +- .../chain_property_bindings_all.js | 2 +- .../host_bindings/sanitization.js | 4 +- .../property_bindings/GOLDEN_PARTIAL.js | 41 +++++++++++++++---- .../property_bindings/TEST_CASES.json | 20 +++++++-- .../property_bindings/sanitization.js | 2 +- ...special_property_remapping_dom_property.js | 13 ++++++ ...special_property_remapping_dom_property.ts | 8 ++++ ...=> special_property_remapping_property.js} | 4 +- ...=> special_property_remapping_property.ts} | 10 ++--- .../shared_name_with_consts_template.js | 2 +- .../src/template/pipeline/src/phases/reify.ts | 27 +++++++++++- .../core/src/render3/instructions/shared.ts | 9 ++-- .../test/acceptance/property_binding_spec.ts | 6 +-- 16 files changed, 118 insertions(+), 36 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_dom_property.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_dom_property.ts rename packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/{special_property_remapping.js => special_property_remapping_property.js} (96%) rename packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/{special_property_remapping.ts => special_property_remapping_property.ts} (55%) diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/attribute_bindings/duplicate_bindings.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/attribute_bindings/duplicate_bindings.js index d83d2103caf..62defa6fd52 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/attribute_bindings/duplicate_bindings.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/attribute_bindings/duplicate_bindings.js @@ -12,7 +12,7 @@ template: function MyComponent_Template(rf, ctx) { $r3$.ɵɵadvance(3); $r3$.ɵɵattribute("aria-label", ctx.value1)("aria-label", ctx.value2); $r3$.ɵɵadvance(); - $r3$.ɵɵdomProperty("tabindex", ctx.value1)("tabindex", ctx.value2); + $r3$.ɵɵdomProperty("tabIndex", ctx.value1)("tabIndex", ctx.value2); $r3$.ɵɵadvance(); $r3$.ɵɵclassMap(ctx.value2); $r3$.ɵɵadvance(); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_attribute_bindings_mixed.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_attribute_bindings_mixed.js index bf431f24e9d..e948776e8df 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_attribute_bindings_mixed.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_attribute_bindings_mixed.js @@ -1,7 +1,7 @@ hostBindings: function MyDirective_HostBindings(rf, ctx) { … if (rf & 2) { - $r3$.ɵɵdomProperty("tabindex", 1); + $r3$.ɵɵdomProperty("tabIndex", 1); $r3$.ɵɵattribute("title", "my title")("id", "my-id"); } } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_multiple_property_bindings.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_multiple_property_bindings.js index 725902d22c7..fe55311c686 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_multiple_property_bindings.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_multiple_property_bindings.js @@ -1,6 +1,6 @@ hostBindings: function MyDirective_HostBindings(rf, ctx) { … if (rf & 2) { - $r3$.ɵɵdomProperty("title", ctx.myTitle)("tabindex", 1)("id", ctx.myId); + $r3$.ɵɵdomProperty("title", ctx.myTitle)("tabIndex", 1)("id", ctx.myId); } } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_property_bindings_all.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_property_bindings_all.js index 12c9eb1db48..642e7bb7910 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_property_bindings_all.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/chain_property_bindings_all.js @@ -1,6 +1,6 @@ hostBindings: function MyDirective_HostBindings(rf, ctx) { … if (rf & 2) { - $r3$.ɵɵdomProperty("tabindex", 1)("title", ctx.myTitle)("id", ctx.myId); + $r3$.ɵɵdomProperty("tabIndex", 1)("title", ctx.myTitle)("id", ctx.myId); } } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.js index 11a0bec968b..8af169be19a 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.js @@ -1,13 +1,13 @@ hostBindings: function HostBindingDir_HostBindings(rf, ctx) { if (rf & 2) { - $r3$.ɵɵdomProperty("innerHtml", ctx.evil, $r3$.ɵɵsanitizeHtml)("href", ctx.evil, $r3$.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.evil, $r3$.ɵɵsanitizeUrlOrResourceUrl)("sandbox", ctx.evil, $r3$.ɵɵvalidateIframeAttribute); + $r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("href", ctx.evil, $r3$.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.evil, $r3$.ɵɵsanitizeUrlOrResourceUrl)("sandbox", ctx.evil, $r3$.ɵɵvalidateIframeAttribute); $r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle); } } … hostBindings: function HostBindingDir2_HostBindings(rf, ctx) { if (rf & 2) { - $r3$.ɵɵdomProperty("innerHtml", ctx.evil, $r3$.ɵɵsanitizeHtml)("href", ctx.evil, $r3$.ɵɵsanitizeUrl)("src", ctx.evil)("sandbox", ctx.evil, $r3$.ɵɵvalidateIframeAttribute); + $r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("href", ctx.evil, $r3$.ɵɵsanitizeUrl)("src", ctx.evil)("sandbox", ctx.evil, $r3$.ɵɵvalidateIframeAttribute); $r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle); } } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/GOLDEN_PARTIAL.js index e8effb3e1af..c093572cd68 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/GOLDEN_PARTIAL.js @@ -166,7 +166,7 @@ export declare class MyModule { } /**************************************************************************************************** - * PARTIAL FILE: special_property_remapping.js + * PARTIAL FILE: special_property_remapping_property.js ****************************************************************************************************/ import { Component, NgModule } from '@angular/core'; import * as i0 from "@angular/core"; @@ -176,15 +176,13 @@ 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: false, selector: "my-component", ngImport: i0, template: ` - `, isInline: true }); +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, isStandalone: false, 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', - template: ` - `, - standalone: false + template: ``, + standalone: false, }] }] }); export class MyModule { @@ -198,7 +196,7 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE }] }); /**************************************************************************************************** - * PARTIAL FILE: special_property_remapping.d.ts + * PARTIAL FILE: special_property_remapping_property.d.ts ****************************************************************************************************/ import * as i0 from "@angular/core"; export declare class MyComponent { @@ -212,6 +210,35 @@ export declare class MyModule { static ɵinj: i0.ɵɵInjectorDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: special_property_remapping_dom_property.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyComponent { + constructor() { + this.forValue = 'some-input'; + } +} +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: "ng-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: [{ + template: ``, + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: special_property_remapping_dom_property.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyComponent { + forValue: string; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + /**************************************************************************************************** * PARTIAL FILE: temporary_variables.js ****************************************************************************************************/ diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/TEST_CASES.json index 027c58e0991..40935dd3527 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/TEST_CASES.json @@ -44,15 +44,29 @@ ] }, { - "description": "should not remap property names whose names do not correspond to their attribute names", + "description": "should not remap special property names when outputting property instructions", "inputFiles": [ - "special_property_remapping.ts" + "special_property_remapping_property.ts" ], "expectations": [ { "failureMessage": "Incorrect template", "files": [ - "special_property_remapping.js" + "special_property_remapping_property.js" + ] + } + ] + }, + { + "description": "should remap special property names when outputting domProperty instructions", + "inputFiles": [ + "special_property_remapping_dom_property.ts" + ], + "expectations": [ + { + "failureMessage": "Incorrect template", + "files": [ + "special_property_remapping_dom_property.js" ] } ] diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/sanitization.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/sanitization.js index f0eff761708..34000115284 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/sanitization.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/sanitization.js @@ -3,7 +3,7 @@ template: function MyComponent_Template(rf, ctx) { $r3$.ɵɵdomElement(0, "div", 0)(1, "link", 1)(2, "div")(3, "img", 2)(4, "iframe", 3)(5, "a", 1)(6, "div"); } if (rf & 2) { - $r3$.ɵɵdomProperty("innerHtml", ctx.evil, $r3$.ɵɵsanitizeHtml); + $r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml); $r3$.ɵɵadvance(); $r3$.ɵɵdomProperty("href", ctx.evil, $r3$.ɵɵsanitizeResourceUrl); $r3$.ɵɵadvance(); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_dom_property.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_dom_property.js new file mode 100644 index 00000000000..2f50136a723 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_dom_property.js @@ -0,0 +1,13 @@ + +consts: [[__AttributeMarker.Bindings__, "for"]] + +… + +function MyComponent_Template(rf, ctx) { + if (rf & 1) { + $i0$.ɵɵdomElement(0, "label", 0); + } + if (rf & 2) { + $i0$.ɵɵdomProperty("htmlFor", ctx.forValue); + } +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_dom_property.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_dom_property.ts new file mode 100644 index 00000000000..7618ff742d4 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_dom_property.ts @@ -0,0 +1,8 @@ +import {Component} from '@angular/core'; + +@Component({ + template: ``, +}) +export class MyComponent { + forValue = 'some-input'; +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_property.js similarity index 96% rename from packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping.js rename to packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_property.js index 09c572c2ea3..204192c89a7 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_property.js @@ -1,7 +1,7 @@ consts: [[__AttributeMarker.Bindings__, "for"]] -// ... +… function MyComponent_Template(rf, ctx) { if (rf & 1) { @@ -10,4 +10,4 @@ function MyComponent_Template(rf, ctx) { if (rf & 2) { $i0$.ɵɵproperty("for", ctx.forValue); } -} \ No newline at end of file +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_property.ts similarity index 55% rename from packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping.ts rename to packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_property.ts index be723097d61..6f5151a9f70 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping.ts +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/property_bindings/special_property_remapping_property.ts @@ -1,15 +1,13 @@ import {Component, NgModule} from '@angular/core'; @Component({ - selector: 'my-component', - template: ` - `, - standalone: false + selector: 'my-component', + template: ``, + standalone: false, }) export class MyComponent { forValue = 'some-input'; } @NgModule({declarations: [MyComponent]}) -export class MyModule { -} +export class MyModule {} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/class_bindings/shared_name_with_consts_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/class_bindings/shared_name_with_consts_template.js index 2cb737c4a26..1a11460166a 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/class_bindings/shared_name_with_consts_template.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/class_bindings/shared_name_with_consts_template.js @@ -22,7 +22,7 @@ consts: () => { } if (rf & 2) { $r3$.ɵɵadvance(3); - $r3$.ɵɵdomProperty("tabindex", ctx.tabIndex); + $r3$.ɵɵdomProperty("tabIndex", ctx.tabIndex); $r3$.ɵɵadvance(); $r3$.ɵɵdomProperty("ngIf", ctx.cond); $r3$.ɵɵadvance(2); diff --git a/packages/compiler/src/template/pipeline/src/phases/reify.ts b/packages/compiler/src/template/pipeline/src/phases/reify.ts index 926e14cf102..f0f1b68b829 100644 --- a/packages/compiler/src/template/pipeline/src/phases/reify.ts +++ b/packages/compiler/src/template/pipeline/src/phases/reify.ts @@ -26,6 +26,19 @@ const GLOBAL_TARGET_RESOLVERS = new Map([ ['body', Identifiers.resolveBody], ]); +/** + * DOM properties that need to be remapped on the compiler side. + * Note: this mapping has to be kept in sync with the equally named mapping in the runtime. + */ +const DOM_PROPERTY_REMAPPING = new Map([ + ['class', 'className'], + ['for', 'htmlFor'], + ['formaction', 'formAction'], + ['innerHtml', 'innerHTML'], + ['readonly', 'readOnly'], + ['tabindex', 'tabIndex'], +]); + /** * Compiles semantic operations across all views and generates output `o.Statement`s with actual * runtime calls in their place. @@ -549,7 +562,12 @@ function reifyUpdateOperations(unit: CompilationUnit, ops: ir.OpList( return; // Stop propcessing if we've matched at least one input. } + // If the property is going to a DOM node, we have to remap it. + if (tNode.type & TNodeType.AnyRNode) { + propName = mapPropName(propName); + } + setDomProperty(tNode, lView, propName, value, renderer, sanitizer); } @@ -287,7 +291,6 @@ export function setDomProperty( ) { if (tNode.type & TNodeType.AnyRNode) { const element = getNativeByTNode(tNode, lView) as RElement | RComment; - propName = mapPropName(propName); if (ngDevMode) { validateAgainstEventProperties(propName); diff --git a/packages/core/test/acceptance/property_binding_spec.ts b/packages/core/test/acceptance/property_binding_spec.ts index fa63b3c09cc..f8dcec2e42e 100644 --- a/packages/core/test/acceptance/property_binding_spec.ts +++ b/packages/core/test/acceptance/property_binding_spec.ts @@ -76,13 +76,11 @@ describe('property bindings', () => { it('should bind to properties whose names do not correspond to their attribute names', () => { @Component({ template: '', - standalone: false, }) class MyComp { forValue?: string; } - TestBed.configureTestingModule({declarations: [MyComp]}); const fixture = TestBed.createComponent(MyComp); const labelNode = fixture.debugElement.query(By.css('label')); @@ -104,7 +102,6 @@ describe('property bindings', () => { @Component({ template: '', selector: 'my-comp', - standalone: false, }) class MyComp { @Input() for!: string; @@ -112,13 +109,12 @@ describe('property bindings', () => { @Component({ template: '', - standalone: false, + imports: [MyComp], }) class App { forValue?: string; } - TestBed.configureTestingModule({declarations: [App, MyComp]}); const fixture = TestBed.createComponent(App); const myCompNode = fixture.debugElement.query(By.directive(MyComp)); fixture.componentInstance.forValue = 'hello';