From 104546569ed195570042553d37bfc0d5f668fe4d Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 31 Dec 2020 18:32:24 +0200 Subject: [PATCH] fix(compiler): incorrectly interpreting some HostBinding names (#40233) Currently when analyzing the metadata of a directive, we bundle together the bindings from `host` and the `HostBinding` and `HostListener` together. This can become a problem later on in the compilation pipeline, because we try to evaluate the value of the binding, causing something like `@HostBinding('class.foo') public true = 1;` to be treated the same as `host: {'[class.foo]': 'true'}`. While looking into the issue, I noticed another one that is closely related: we weren't treating quoted property names correctly. E.g. `@HostBinding('class.foo') public "foo-bar" = 1;` was being interpreted as `classProp('foo', ctx.foo - ctx.bar)` due to the same issue where property names were being evaluated. These changes resolve both of the issues by treating all `HostBinding` instance as if they're reading the property from `this`. E.g. the `@HostBinding('class.foo') public true = 1;` from above is now being treated as `host: {'[class.foo]': 'this.true'}` which further down the pipeline becomes `classProp('foo', ctx.true)`. This doesn't have any payload size implications for existing code, because we've always been prefixing implicit property reads with `ctx.`. If the property doesn't have an identifier that can be read using dotted access, we convert it to a quoted one (e.g. `classProp('foo', ctx['is-foo']))`. Fixes #40220. Fixes #40230. Fixes #18698. PR Close #40233 --- .../src/ngtsc/annotations/src/directive.ts | 8 +- .../host_bindings/GOLDEN_PARTIAL.js | 108 +++++++++++++++++- .../host_bindings/TEST_CASES.json | 28 +++++ .../host_bindings_primitive_names.js | 11 ++ .../host_bindings_primitive_names.ts | 18 +++ .../host_bindings_quoted_names.js | 11 ++ .../host_bindings_quoted_names.ts | 12 ++ .../binding_slots/GOLDEN_PARTIAL.js | 4 +- .../chaining/GOLDEN_PARTIAL.js | 2 +- .../host_bindings/GOLDEN_PARTIAL.js | 12 +- packages/compiler/src/compiler.ts | 2 +- packages/compiler/src/jit_compiler_facade.ts | 8 +- packages/compiler/src/render3/util.ts | 6 + .../core/test/acceptance/host_binding_spec.ts | 59 ++++++++++ 14 files changed, 272 insertions(+), 17 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_primitive_names.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_primitive_names.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_quoted_names.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_quoted_names.ts diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index c0214ad069..5159305c42 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {compileDeclareDirectiveFromMetadata, compileDirectiveFromMetadata, ConstantPool, Expression, ExternalExpr, Identifiers, makeBindingParser, ParsedHostBindings, ParseError, parseHostBindings, R3DependencyMetadata, R3DirectiveDef, R3DirectiveMetadata, R3FactoryTarget, R3QueryMetadata, R3ResolvedDependencyType, Statement, verifyHostBindings, WrappedNodeExpr} from '@angular/compiler'; +import {compileDeclareDirectiveFromMetadata, compileDirectiveFromMetadata, ConstantPool, Expression, ExternalExpr, getSafePropertyAccessString, Identifiers, makeBindingParser, ParsedHostBindings, ParseError, parseHostBindings, R3DependencyMetadata, R3DirectiveDef, R3DirectiveMetadata, R3FactoryTarget, R3QueryMetadata, R3ResolvedDependencyType, Statement, verifyHostBindings, WrappedNodeExpr} from '@angular/compiler'; import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; @@ -738,7 +738,11 @@ export function extractHostBindings( hostPropertyName = resolved; } - bindings.properties[hostPropertyName] = member.name; + // Since this is a decorator, we know that the value is a class member. Always access it + // through `this` so that further down the line it can't be confused for a literal value + // (e.g. if there's a property called `true`). There is no size penalty, because all + // values (except literals) are converted to `ctx.propName` eventually. + bindings.properties[hostPropertyName] = getSafePropertyAccessString('this', member.name); }); }); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/GOLDEN_PARTIAL.js index 616903d2a9..46243c0a86 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/GOLDEN_PARTIAL.js @@ -9,7 +9,7 @@ export class HostBindingDir { } } HostBindingDir.ɵfac = function HostBindingDir_Factory(t) { return new (t || HostBindingDir)(); }; -HostBindingDir.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: HostBindingDir, selector: "[hostBindingDir]", host: { properties: { "id": "dirId" } }, ngImport: i0 }); +HostBindingDir.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: HostBindingDir, selector: "[hostBindingDir]", host: { properties: { "id": "this.dirId" } }, ngImport: i0 }); (function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(HostBindingDir, [{ type: Directive, args: [{ selector: '[hostBindingDir]' }] @@ -298,7 +298,7 @@ export class MyDirective { } } MyDirective.ɵfac = function MyDirective_Factory(t) { return new (t || MyDirective)(); }; -MyDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: MyDirective, selector: "[my-dir]", host: { properties: { "tabindex": "1", "title": "myTitle", "id": "myId" } }, ngImport: i0 }); +MyDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: MyDirective, selector: "[my-dir]", host: { properties: { "tabindex": "1", "title": "this.myTitle", "id": "this.myId" } }, ngImport: i0 }); (function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyDirective, [{ type: Directive, args: [{ selector: '[my-dir]', host: { '[tabindex]': '1' } }] @@ -423,7 +423,7 @@ export class MyDirective { } } MyDirective.ɵfac = function MyDirective_Factory(t) { return new (t || MyDirective)(); }; -MyDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: MyDirective, selector: "[my-dir]", host: { properties: { "attr.tabindex": "1", "attr.title": "myTitle", "attr.id": "myId" } }, ngImport: i0 }); +MyDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: MyDirective, selector: "[my-dir]", host: { properties: { "attr.tabindex": "1", "attr.title": "this.myTitle", "attr.id": "this.myId" } }, ngImport: i0 }); (function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyDirective, [{ type: Directive, args: [{ selector: '[my-dir]', host: { '[attr.tabindex]': '1' } }] @@ -589,3 +589,105 @@ export declare class MyComponent { static ɵcmp: i0.ɵɵComponentDefWithMeta; } +/**************************************************************************************************** + * PARTIAL FILE: host_bindings_primitive_names.js + ****************************************************************************************************/ +import { Directive, HostBinding, NgModule } from '@angular/core'; +import * as i0 from "@angular/core"; +export class HostBindingDir { +} +HostBindingDir.ɵfac = function HostBindingDir_Factory(t) { return new (t || HostBindingDir)(); }; +HostBindingDir.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: HostBindingDir, selector: "[hostBindingDir]", host: { properties: { "class.a": "true", "class.b": "false", "class.c": "this.true", "class.d": "this.false", "class.e": "this.other" } }, ngImport: i0 }); +(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(HostBindingDir, [{ + type: Directive, + args: [{ + selector: '[hostBindingDir]', + host: { + '[class.a]': 'true', + '[class.b]': 'false', + } + }] + }], null, { true: [{ + type: HostBinding, + args: ['class.c'] + }], false: [{ + type: HostBinding, + args: ['class.d'] + }], other: [{ + type: HostBinding, + args: ['class.e'] + }] }); })(); +export class MyModule { +} +MyModule.ɵmod = i0.ɵɵdefineNgModule({ type: MyModule }); +MyModule.ɵinj = i0.ɵɵdefineInjector({ factory: function MyModule_Factory(t) { return new (t || MyModule)(); } }); +(function () { (typeof ngJitMode === "undefined" || ngJitMode) && i0.ɵɵsetNgModuleScope(MyModule, { declarations: [HostBindingDir] }); })(); +(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyModule, [{ + type: NgModule, + args: [{ declarations: [HostBindingDir] }] + }], null, null); })(); + +/**************************************************************************************************** + * PARTIAL FILE: host_bindings_primitive_names.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class HostBindingDir { + true: any; + false: any; + other: any; + static ɵfac: i0.ɵɵFactoryDef; + static ɵdir: i0.ɵɵDirectiveDefWithMeta; +} +export declare class MyModule { + static ɵmod: i0.ɵɵNgModuleDefWithMeta; + static ɵinj: i0.ɵɵInjectorDef; +} + +/**************************************************************************************************** + * PARTIAL FILE: host_bindings_quoted_names.js + ****************************************************************************************************/ +import { Directive, HostBinding, NgModule } from '@angular/core'; +import * as i0 from "@angular/core"; +export class HostBindingDir { +} +HostBindingDir.ɵfac = function HostBindingDir_Factory(t) { return new (t || HostBindingDir)(); }; +HostBindingDir.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: HostBindingDir, selector: "[hostBindingDir]", host: { properties: { "class.a": "this['is-a']", "class.b": "this['is-\"b\"']", "class.c": "this['\"is-c\"']" } }, ngImport: i0 }); +(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(HostBindingDir, [{ + type: Directive, + args: [{ selector: '[hostBindingDir]' }] + }], null, { 'is-a': [{ + type: HostBinding, + args: ['class.a'] + }], 'is-"b"': [{ + type: HostBinding, + args: ['class.b'] + }], '"is-c"': [{ + type: HostBinding, + args: ['class.c'] + }] }); })(); +export class MyModule { +} +MyModule.ɵmod = i0.ɵɵdefineNgModule({ type: MyModule }); +MyModule.ɵinj = i0.ɵɵdefineInjector({ factory: function MyModule_Factory(t) { return new (t || MyModule)(); } }); +(function () { (typeof ngJitMode === "undefined" || ngJitMode) && i0.ɵɵsetNgModuleScope(MyModule, { declarations: [HostBindingDir] }); })(); +(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyModule, [{ + type: NgModule, + args: [{ declarations: [HostBindingDir] }] + }], null, null); })(); + +/**************************************************************************************************** + * PARTIAL FILE: host_bindings_quoted_names.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class HostBindingDir { + 'is-a': any; + 'is-"b"': any; + '"is-c"': any; + static ɵfac: i0.ɵɵFactoryDef; + static ɵdir: i0.ɵɵDirectiveDefWithMeta; +} +export declare class MyModule { + static ɵmod: i0.ɵɵNgModuleDefWithMeta; + static ɵinj: i0.ɵɵInjectorDef; +} + diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/TEST_CASES.json index 8ae3757e64..c986e0867f 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/TEST_CASES.json @@ -224,6 +224,34 @@ ] } ] + }, + { + "description": "should handle host bindings with the same name as a primitive value", + "inputFiles": [ + "host_bindings_primitive_names.ts" + ], + "expectations": [ + { + "failureMessage": "Invalid host binding code", + "files": [ + "host_bindings_primitive_names.js" + ] + } + ] + }, + { + "description": "should handle host bindings with quoted names", + "inputFiles": [ + "host_bindings_quoted_names.ts" + ], + "expectations": [ + { + "failureMessage": "Invalid host binding code", + "files": [ + "host_bindings_quoted_names.js" + ] + } + ] } ] } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_primitive_names.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_primitive_names.js new file mode 100644 index 0000000000..0a6f5e4a72 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_primitive_names.js @@ -0,0 +1,11 @@ + +HostBindingDir.ɵdir = $r3$.ɵɵdefineDirective({ + type: HostBindingDir, + selectors: [["", "hostBindingDir", ""]], + hostVars: 10, + hostBindings: function HostBindingDir_HostBindings(rf, ctx) { + if (rf & 2) { + $r3$.ɵɵclassProp("a", true)("b", false)("c", ctx.true)("d", ctx.false)("e", ctx.other); + } + } + }); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_primitive_names.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_primitive_names.ts new file mode 100644 index 0000000000..d5a19f26b9 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_primitive_names.ts @@ -0,0 +1,18 @@ +import {Directive, HostBinding, NgModule} from '@angular/core'; + +@Directive({ + selector: '[hostBindingDir]', + host: { + '[class.a]': 'true', + '[class.b]': 'false', + } +}) +export class HostBindingDir { + @HostBinding('class.c') true: any; + @HostBinding('class.d') false: any; + @HostBinding('class.e') other: any; +} + +@NgModule({declarations: [HostBindingDir]}) +export class MyModule { +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_quoted_names.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_quoted_names.js new file mode 100644 index 0000000000..bffac31d5c --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_quoted_names.js @@ -0,0 +1,11 @@ + +HostBindingDir.ɵdir = $r3$.ɵɵdefineDirective({ + type: HostBindingDir, + selectors: [["", "hostBindingDir", ""]], + hostVars: 6, + hostBindings: function HostBindingDir_HostBindings(rf, ctx) { + if (rf & 2) { + $r3$.ɵɵclassProp("a", ctx["is-a"])("b", ctx["is-\"b\""])("c", ctx["\"is-c\""]); + } + } + }); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_quoted_names.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_quoted_names.ts new file mode 100644 index 0000000000..f911a13b97 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_bindings_quoted_names.ts @@ -0,0 +1,12 @@ +import {Directive, HostBinding, NgModule} from '@angular/core'; + +@Directive({selector: '[hostBindingDir]'}) +export class HostBindingDir { + @HostBinding('class.a') 'is-a': any; + @HostBinding('class.b') 'is-"b"': any; + @HostBinding('class.c') '"is-c"': any; +} + +@NgModule({declarations: [HostBindingDir]}) +export class MyModule { +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/binding_slots/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/binding_slots/GOLDEN_PARTIAL.js index 913dd682b0..f7366001de 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/binding_slots/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/binding_slots/GOLDEN_PARTIAL.js @@ -13,7 +13,7 @@ export class MyComponent { } } MyComponent.ɵfac = function MyComponent_Factory(t) { return new (t || MyComponent)(); }; -MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", inputs: { name: "name" }, host: { attributes: { "title": "foo title" }, properties: { "style": "myStyle", "class": "myClass", "id": "id", "title": "title" }, styleAttribute: "width:200px; height:500px", classAttribute: "foo baz" }, ngImport: i0, template: { source: '', isInline: true } }); +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", inputs: { name: "name" }, host: { attributes: { "title": "foo title" }, properties: { "style": "this.myStyle", "class": "this.myClass", "id": "this.id", "title": "this.title" }, styleAttribute: "width:200px; height:500px", classAttribute: "foo baz" }, ngImport: i0, template: { source: '', isInline: true } }); (function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyComponent, [{ type: Component, args: [{ @@ -83,7 +83,7 @@ export class WidthDirective { } } WidthDirective.ɵfac = function WidthDirective_Factory(t) { return new (t || WidthDirective)(); }; -WidthDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: WidthDirective, selector: "[myWidthDir]", host: { properties: { "style.width": "myWidth", "class.foo": "myFooClass", "id": "id", "title": "title" } }, ngImport: i0 }); +WidthDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: WidthDirective, selector: "[myWidthDir]", host: { properties: { "style.width": "this.myWidth", "class.foo": "this.myFooClass", "id": "this.id", "title": "this.title" } }, ngImport: i0 }); (function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(WidthDirective, [{ type: Directive, args: [{ selector: '[myWidthDir]' }] diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/chaining/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/chaining/GOLDEN_PARTIAL.js index f8d0ec57e2..400683ea63 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/chaining/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/chaining/GOLDEN_PARTIAL.js @@ -325,7 +325,7 @@ export class MyComponent { } } MyComponent.ɵfac = function MyComponent_Factory(t) { return new (t || MyComponent)(); }; -MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "ng-component", host: { properties: { "class.apple": "yesToApple", "style.color": "color", "class.tomato": "yesToTomato", "style.transition": "transition", "style.border": "border", "class.orange": "yesToOrange" } }, ngImport: i0, template: { source: '', isInline: true } }); +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "ng-component", host: { properties: { "class.apple": "yesToApple", "style.color": "color", "class.tomato": "yesToTomato", "style.transition": "transition", "style.border": "this.border", "class.orange": "this.yesToOrange" } }, ngImport: i0, template: { source: '', isInline: true } }); (function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyComponent, [{ type: Component, args: [{ diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/host_bindings/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/host_bindings/GOLDEN_PARTIAL.js index db6b05408e..f749cb54c7 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/host_bindings/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_styling/host_bindings/GOLDEN_PARTIAL.js @@ -12,7 +12,7 @@ export class MyComponent { } } MyComponent.ɵfac = function MyComponent_Factory(t) { return new (t || MyComponent)(); }; -MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", host: { properties: { "style": "myStyle", "class": "myClass", "style.color": "myColorProp", "class.foo": "myFooClass" }, styleAttribute: "width:200px; height:500px", classAttribute: "foo baz" }, ngImport: i0, template: { source: '', isInline: true } }); +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", host: { properties: { "style": "this.myStyle", "class": "this.myClass", "style.color": "this.myColorProp", "class.foo": "this.myFooClass" }, styleAttribute: "width:200px; height:500px", classAttribute: "foo baz" }, ngImport: i0, template: { source: '', isInline: true } }); (function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyComponent, [{ type: Component, args: [{ @@ -80,7 +80,7 @@ export class MyComponent { } } MyComponent.ɵfac = function MyComponent_Factory(t) { return new (t || MyComponent)(); }; -MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", host: { properties: { "style.height.pt": "myHeightProp", "class.bar": "myBarClass", "style": "myStyle", "style.width": "myWidthProp", "class.foo": "myFooClass", "class": "myClasses" } }, ngImport: i0, template: { source: '', isInline: true } }); +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", host: { properties: { "style.height.pt": "myHeightProp", "class.bar": "myBarClass", "style": "this.myStyle", "style.width": "this.myWidthProp", "class.foo": "this.myFooClass", "class": "this.myClasses" } }, ngImport: i0, template: { source: '', isInline: true } }); (function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyComponent, [{ type: Component, args: [{ @@ -149,7 +149,7 @@ export class MyComponent { } } MyComponent.ɵfac = function MyComponent_Factory(t) { return new (t || MyComponent)(); }; -MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", host: { properties: { "style!important": "myStyleExp", "class!important": "myClassExp", "class.foo!important": "myFooClassExp", "style.width!important": "myWidthExp" } }, ngImport: i0, template: { source: ` +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", host: { properties: { "style!important": "myStyleExp", "class!important": "myClassExp", "class.foo!important": "this.myFooClassExp", "style.width!important": "this.myWidthExp" } }, ngImport: i0, template: { source: `
`, isInline: true } }); @@ -368,7 +368,7 @@ export class ClassDirective { } } ClassDirective.ɵfac = function ClassDirective_Factory(t) { return new (t || ClassDirective)(); }; -ClassDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: ClassDirective, selector: "[myClassDir]", host: { properties: { "class": "myClassMap" } }, ngImport: i0 }); +ClassDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: ClassDirective, selector: "[myClassDir]", host: { properties: { "class": "this.myClassMap" } }, ngImport: i0 }); (function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(ClassDirective, [{ type: Directive, args: [{ selector: '[myClassDir]' }] @@ -383,7 +383,7 @@ export class WidthDirective { } } WidthDirective.ɵfac = function WidthDirective_Factory(t) { return new (t || WidthDirective)(); }; -WidthDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: WidthDirective, selector: "[myWidthDir]", host: { properties: { "style.width": "myWidth", "class.foo": "myFooClass" } }, ngImport: i0 }); +WidthDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: WidthDirective, selector: "[myWidthDir]", host: { properties: { "style.width": "this.myWidth", "class.foo": "this.myFooClass" } }, ngImport: i0 }); (function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(WidthDirective, [{ type: Directive, args: [{ selector: '[myWidthDir]' }] @@ -401,7 +401,7 @@ export class HeightDirective { } } HeightDirective.ɵfac = function HeightDirective_Factory(t) { return new (t || HeightDirective)(); }; -HeightDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: HeightDirective, selector: "[myHeightDir]", host: { properties: { "style.height": "myHeight", "class.bar": "myBarClass" } }, ngImport: i0 }); +HeightDirective.ɵdir = i0.ɵɵngDeclareDirective({ version: "0.0.0-PLACEHOLDER", type: HeightDirective, selector: "[myHeightDir]", host: { properties: { "style.height": "this.myHeight", "class.bar": "this.myBarClass" } }, ngImport: i0 }); (function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(HeightDirective, [{ type: Directive, args: [{ selector: '[myHeightDir]' }] diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index 5df78db770..95b0b4a9b2 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -101,7 +101,7 @@ export {R3DependencyMetadata, R3ResolvedDependencyType, compileFactoryFunction, export {compileInjector, compileNgModule, R3InjectorMetadata, R3NgModuleMetadata} from './render3/r3_module_compiler'; export {compilePipeFromMetadata, R3PipeMetadata} from './render3/r3_pipe_compiler'; export {makeBindingParser, ParsedTemplate, parseTemplate, ParseTemplateOptions} from './render3/view/template'; -export {R3Reference, devOnlyGuardedExpression} from './render3/util'; +export {R3Reference, devOnlyGuardedExpression, getSafePropertyAccessString} from './render3/util'; export {compileComponentFromMetadata, compileDirectiveFromMetadata, parseHostBindings, ParsedHostBindings, verifyHostBindings} from './render3/view/compiler'; export {compileDeclareComponentFromMetadata} from './render3/partial/component'; export {compileDeclareDirectiveFromMetadata} from './render3/partial/directive'; diff --git a/packages/compiler/src/jit_compiler_facade.ts b/packages/compiler/src/jit_compiler_facade.ts index 5ee1c6328c..f90c55d981 100644 --- a/packages/compiler/src/jit_compiler_facade.ts +++ b/packages/compiler/src/jit_compiler_facade.ts @@ -20,7 +20,7 @@ import {compileFactoryFunction, R3DependencyMetadata, R3FactoryTarget, R3Resolve import {R3JitReflector} from './render3/r3_jit'; import {compileInjector, compileNgModule, R3InjectorMetadata, R3NgModuleMetadata} from './render3/r3_module_compiler'; import {compilePipeFromMetadata, R3PipeMetadata} from './render3/r3_pipe_compiler'; -import {R3Reference} from './render3/util'; +import {getSafePropertyAccessString, R3Reference} from './render3/util'; import {DeclarationListEmitMode, R3ComponentMetadata, R3DirectiveMetadata, R3HostMetadata, R3QueryMetadata, R3UsedDirectiveMetadata} from './render3/view/api'; import {compileComponentFromMetadata, compileDirectiveFromMetadata, ParsedHostBindings, parseHostBindings, verifyHostBindings} from './render3/view/compiler'; import {makeBindingParser, parseTemplate} from './render3/view/template'; @@ -471,7 +471,11 @@ function extractHostBindings( if (propMetadata.hasOwnProperty(field)) { propMetadata[field].forEach(ann => { if (isHostBinding(ann)) { - bindings.properties[ann.hostPropertyName || field] = field; + // Since this is a decorator, we know that the value is a class member. Always access it + // through `this` so that further down the line it can't be confused for a literal value + // (e.g. if there's a property called `true`). + bindings.properties[ann.hostPropertyName || field] = + getSafePropertyAccessString('this', field); } else if (isHostListener(ann)) { bindings.listeners[ann.eventName || field] = `${field}(${(ann.args || []).join(',')})`; } diff --git a/packages/compiler/src/render3/util.ts b/packages/compiler/src/render3/util.ts index 8f1be8bd92..04083d07c5 100644 --- a/packages/compiler/src/render3/util.ts +++ b/packages/compiler/src/render3/util.ts @@ -7,6 +7,7 @@ */ import {StaticSymbol} from '../aot/static_symbol'; +import {escapeIdentifier} from '../output/abstract_emitter'; import * as o from '../output/output_ast'; import {OutputContext} from '../util'; @@ -84,6 +85,11 @@ export function getSyntheticPropertyName(name: string) { return name; } +export function getSafePropertyAccessString(accessor: string, name: string): string { + const escapedName = escapeIdentifier(name, false, false); + return escapedName !== name ? `${accessor}[${escapedName}]` : `${accessor}.${name}`; +} + export function prepareSyntheticListenerFunctionName(name: string, phase: string) { return `animation_${name}_${phase}`; } diff --git a/packages/core/test/acceptance/host_binding_spec.ts b/packages/core/test/acceptance/host_binding_spec.ts index 30f4166c9a..df4e19ce1d 100644 --- a/packages/core/test/acceptance/host_binding_spec.ts +++ b/packages/core/test/acceptance/host_binding_spec.ts @@ -1473,4 +1473,63 @@ describe('host bindings', () => { /Attempted to set attribute `dynamic` on a container node. Host bindings are not valid on ng-container or ng-template./); }); }); + + onlyInIvy('VE does not support this').describe('host bindings on edge case properties', () => { + it('should handle host bindings with the same name as a primitive value', () => { + @Directive({ + selector: '[dir]', + host: { + '[class.a]': 'true', + '[class.b]': 'false', + } + }) + class MyDirective { + @HostBinding('class.c') true: any; + @HostBinding('class.d') false: any; + } + + @Component({template: ''}) + class MyApp { + @ViewChild(MyDirective) dir!: MyDirective; + } + + TestBed.configureTestingModule({declarations: [MyApp, MyDirective]}); + const fixture = TestBed.createComponent(MyApp); + fixture.detectChanges(); + const span = fixture.nativeElement.querySelector('span'); + expect(span.className).toBe('a'); + + fixture.componentInstance.dir.true = 1; + fixture.componentInstance.dir.false = 2; + fixture.detectChanges(); + + expect(span.className).toBe('a c d'); + }); + + it('should handle host bindings with quoted names', () => { + @Directive({selector: '[dir]'}) + class MyDirective { + @HostBinding('class.a') 'is-a': any; + @HostBinding('class.b') 'is-"b"': any = true; + @HostBinding('class.c') '"is-c"': any; + } + + @Component({template: ''}) + class MyApp { + @ViewChild(MyDirective) dir!: MyDirective; + } + + TestBed.configureTestingModule({declarations: [MyApp, MyDirective]}); + const fixture = TestBed.createComponent(MyApp); + fixture.detectChanges(); + const span = fixture.nativeElement.querySelector('span'); + expect(span.className).toBe('b'); + + fixture.componentInstance.dir['is-a'] = 1; + fixture.componentInstance.dir['"is-c"'] = 2; + fixture.detectChanges(); + + expect(span.className).toBe('b a c'); + }); + }); });