From 00014ed68aa179fad4071e3bb3bafb1480b6b4f1 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Thu, 30 Nov 2023 18:47:56 -0800 Subject: [PATCH] fix(compiler): generate proper code for nullish coalescing in styling host bindings This commit fixes an issue where having an expression with nullish coalescing in styling host bindings leads to JS errors due to the fact that a declaration for a temporary variable was not included into the generated code. Resolves #53295. --- .../nullish_coalescing_host_bindings.js | 8 +-- .../host_bindings/GOLDEN_PARTIAL.js | 68 +++++++++++++++++++ .../host_bindings/TEST_CASES.json | 28 ++++++++ .../host_class_bindings_with_temporaries.js | 8 +++ .../host_class_bindings_with_temporaries.ts | 13 ++++ .../host_style_bindings_with_temporaries.js | 8 +++ .../host_style_bindings_with_temporaries.ts | 13 ++++ .../compiler/src/render3/view/compiler.ts | 30 ++++++-- 8 files changed, 165 insertions(+), 11 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_class_bindings_with_temporaries.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_class_bindings_with_temporaries.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_style_bindings_with_temporaries.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_style_bindings_with_temporaries.ts diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/nullish_coalescing/nullish_coalescing_host_bindings.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/nullish_coalescing/nullish_coalescing_host_bindings.js index 0205b73b7889..648a56b0e672 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/nullish_coalescing/nullish_coalescing_host_bindings.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/nullish_coalescing/nullish_coalescing_host_bindings.js @@ -1,12 +1,12 @@ hostBindings: function MyApp_HostBindings(rf, ctx) { if (rf & 1) { i0.ɵɵlistener("click", function MyApp_click_HostBindingHandler() { - let $tmp$; - return ctx.logLastName(($tmp$ = ($tmp$ = ctx.lastName) !== null && $tmp$ !== undefined ? $tmp$ : ctx.lastNameFallback) !== null && $tmp$ !== undefined ? $tmp$ : "unknown"); + let $tmp_0$; + return ctx.logLastName(($tmp_0$ = ($tmp_0$ = ctx.lastName) !== null && $tmp_0$ !== undefined ? $tmp_0$ : ctx.lastNameFallback) !== null && $tmp_0$ !== undefined ? $tmp_0$ : "unknown"); }); } if (rf & 2) { - let $tmp$; - i0.ɵɵattribute("first-name", "Hello, " + (($tmp$ = ctx.firstName) !== null && $tmp$ !== undefined ? $tmp$ : "Frodo") + "!"); + let $tmp_1$; + i0.ɵɵattribute("first-name", "Hello, " + (($tmp_1$ = ctx.firstName) !== null && $tmp_1$ !== undefined ? $tmp_1$ : "Frodo") + "!"); } } 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 180390ab16ee..d61174d7b294 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 @@ -82,6 +82,74 @@ export declare class MyModule { static ɵinj: i0.ɵɵInjectorDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: host_class_bindings_with_temporaries.js + ****************************************************************************************************/ +import { Directive } from '@angular/core'; +import * as i0 from "@angular/core"; +export class HostBindingDir { + constructor() { + this.value = null; + } +} +HostBindingDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingDir, deps: [], target: i0.ɵɵFactoryTarget.Directive }); +HostBindingDir.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingDir, isStandalone: true, selector: "[hostBindingDir]", host: { properties: { "class.a": "value ?? \"class-a\"", "class.b": "value ?? \"class-b\"" } }, ngImport: i0 }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingDir, decorators: [{ + type: Directive, + args: [{ + standalone: true, + selector: '[hostBindingDir]', + host: { + '[class.a]': 'value ?? "class-a"', + '[class.b]': 'value ?? "class-b"', + }, + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: host_class_bindings_with_temporaries.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class HostBindingDir { + value: number | null; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} + +/**************************************************************************************************** + * PARTIAL FILE: host_style_bindings_with_temporaries.js + ****************************************************************************************************/ +import { Directive } from '@angular/core'; +import * as i0 from "@angular/core"; +export class HostBindingDir { + constructor() { + this.value = null; + } +} +HostBindingDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingDir, deps: [], target: i0.ɵɵFactoryTarget.Directive }); +HostBindingDir.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingDir, isStandalone: true, selector: "[hostBindingDir]", host: { properties: { "style.fontSize": "value ?? \"15px\"", "style.fontWeight": "value ?? \"bold\"" } }, ngImport: i0 }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingDir, decorators: [{ + type: Directive, + args: [{ + standalone: true, + selector: '[hostBindingDir]', + host: { + '[style.fontSize]': 'value ?? "15px"', + '[style.fontWeight]': 'value ?? "bold"', + }, + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: host_style_bindings_with_temporaries.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class HostBindingDir { + value: number | null; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} + /**************************************************************************************************** * PARTIAL FILE: host_bindings_with_pure_functions.js ****************************************************************************************************/ 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 385745f2cdb9..fd2a96a43a4e 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 @@ -29,6 +29,34 @@ } ] }, + { + "description": "should support host class bindings with temporary expressions", + "inputFiles": [ + "host_class_bindings_with_temporaries.ts" + ], + "expectations": [ + { + "failureMessage": "Invalid host binding code", + "files": [ + "host_class_bindings_with_temporaries.js" + ] + } + ] + }, + { + "description": "should support host style bindings with temporary expressions", + "inputFiles": [ + "host_style_bindings_with_temporaries.ts" + ], + "expectations": [ + { + "failureMessage": "Invalid host binding code", + "files": [ + "host_style_bindings_with_temporaries.js" + ] + } + ] + }, { "description": "should support host bindings with pure functions", "inputFiles": [ diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_class_bindings_with_temporaries.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_class_bindings_with_temporaries.js new file mode 100644 index 000000000000..d8df7871dec5 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_class_bindings_with_temporaries.js @@ -0,0 +1,8 @@ +hostBindings: function HostBindingDir_HostBindings(rf, ctx) { + if (rf & 2) { + let $tmp_0$; + let $tmp_1$; + $r3$.ɵɵclassProp("a", ($tmp_0$ = ctx.value) !== null && $tmp_0$ !== undefined ? $tmp_0$ : "class-a") + ("b", ($tmp_1$ = ctx.value) !== null && $tmp_1$ !== undefined ? $tmp_1$ : "class-b"); + } +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_class_bindings_with_temporaries.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_class_bindings_with_temporaries.ts new file mode 100644 index 000000000000..291a189cf78f --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_class_bindings_with_temporaries.ts @@ -0,0 +1,13 @@ +import {Directive} from '@angular/core'; + +@Directive({ + standalone: true, + selector: '[hostBindingDir]', + host: { + '[class.a]': 'value ?? "class-a"', + '[class.b]': 'value ?? "class-b"', + }, +}) +export class HostBindingDir { + value: number|null = null; +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_style_bindings_with_temporaries.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_style_bindings_with_temporaries.js new file mode 100644 index 000000000000..419d87b28985 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_style_bindings_with_temporaries.js @@ -0,0 +1,8 @@ +hostBindings: function HostBindingDir_HostBindings(rf, ctx) { + if (rf & 2) { + let $tmp_0$; + let $tmp_1$; + $r3$.ɵɵstyleProp("font-size", ($tmp_0$ = ctx.value) !== null && $tmp_0$ !== undefined ? $tmp_0$ : "15px") + ("font-weight", ($tmp_1$ = ctx.value) !== null && $tmp_1$ !== undefined ? $tmp_1$ : "bold"); + } +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_style_bindings_with_temporaries.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_style_bindings_with_temporaries.ts new file mode 100644 index 000000000000..0c2db506dec3 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_style_bindings_with_temporaries.ts @@ -0,0 +1,13 @@ +import {Directive} from '@angular/core'; + +@Directive({ + standalone: true, + selector: '[hostBindingDir]', + host: { + '[style.fontSize]': 'value ?? "15px"', + '[style.fontWeight]': 'value ?? "bold"', + }, +}) +export class HostBindingDir { + value: number|null = null; +} diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index f0141d8e1984..2eebafee7689 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -618,6 +618,10 @@ function createHostBindingsFunction( return emitHostBindingFunction(hostJob); } + + let bindingId = 0; + const getNextBindingId = () => `${bindingId++}`; + const bindingContext = o.variable(CONTEXT_NAME); const styleBuilder = new StylingBuilder(bindingContext); @@ -681,7 +685,7 @@ function createHostBindingsFunction( for (const binding of allOtherBindings) { // resolve literal arrays and literal objects const value = binding.expression.visit(getValueConverter()); - const bindingExpr = bindingFn(bindingContext, value); + const bindingExpr = bindingFn(bindingContext, value, getNextBindingId); const {bindingName, instruction, isAttribute} = getBindingNameAndInstruction(binding); @@ -768,10 +772,13 @@ function createHostBindingsFunction( totalHostVarsCount += Math.max(call.allocateBindingSlots - MIN_STYLING_BINDING_SLOTS_REQUIRED, 0); + const {params, stmts} = + convertStylingCall(call, bindingContext, bindingFn, getNextBindingId); + updateVariables.push(...stmts); updateInstructions.push({ reference: instruction.reference, - paramsOrFn: convertStylingCall(call, bindingContext, bindingFn), - span: null + paramsOrFn: params, + span: null, }); } }); @@ -801,13 +808,22 @@ function createHostBindingsFunction( return null; } -function bindingFn(implicit: any, value: AST) { - return convertPropertyBinding(null, implicit, value, 'b'); +function bindingFn(implicit: any, value: AST, getNextBindingIdFn: () => string) { + return convertPropertyBinding(null, implicit, value, getNextBindingIdFn()); } function convertStylingCall( - call: StylingInstructionCall, bindingContext: any, bindingFn: Function) { - return call.params(value => bindingFn(bindingContext, value).currValExpr); + call: StylingInstructionCall, bindingContext: any, bindingFn: Function, + getNextBindingIdFn: () => string) { + const stmts: o.Statement[] = []; + const params = call.params(value => { + const result = bindingFn(bindingContext, value, getNextBindingIdFn); + if (Array.isArray(result.stmts) && result.stmts.length > 0) { + stmts.push(...result.stmts); + } + return result.currValExpr; + }); + return {params, stmts}; } function getBindingNameAndInstruction(binding: ParsedProperty):