From bf21792ca1379f17cbfb397bb6fcaaafe78c1ed6 Mon Sep 17 00:00:00 2001 From: Dylan Hunn Date: Thu, 14 Dec 2023 15:43:18 -0800 Subject: [PATCH] refactor(compiler): Host attribute bindings should always be extracted into hostAttrs (#53574) Host attribute literal bindings should not result in an `attribute` update instruction. PR Close #53574 --- .../host_bindings/TEST_CASES.json | 1 - ...with_ts_expression_node_template.pipeline.js | 5 ----- .../src/template/pipeline/src/ingest.ts | 17 +++++++++-------- .../pipeline/src/phases/attribute_extraction.ts | 4 ++-- 4 files changed, 11 insertions(+), 16 deletions(-) delete mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_with_ts_expression_node_template.pipeline.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 5835b76e337..68f7914f131 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 @@ -362,7 +362,6 @@ "files": [ { "expected": "host_with_ts_expression_node_template.js", - "templatePipelineExpected": "host_with_ts_expression_node_template.pipeline.js", "generated": "host_with_ts_expression_node.js" } ] diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_with_ts_expression_node_template.pipeline.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_with_ts_expression_node_template.pipeline.js deleted file mode 100644 index 39c443d3d86..00000000000 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/host_with_ts_expression_node_template.pipeline.js +++ /dev/null @@ -1,5 +0,0 @@ -function MyComponent_HostBindings(rf, ctx) { - if (rf & 2) { - i0.ɵɵattribute("foo", BAR_CONST); - } -} \ No newline at end of file diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index 9ac8e6e5c86..7dbc486ae02 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -78,7 +78,7 @@ export function ingestHostBinding( .calcPossibleSecurityContexts( input.componentSelector, property.name, bindingKind === ir.BindingKind.Attribute) .filter(context => context !== SecurityContext.NONE); - ingestHostProperty(job, property, bindingKind, false, securityContexts); + ingestHostProperty(job, property, bindingKind, securityContexts); } for (const [name, expr] of Object.entries(input.attributes) ?? []) { const securityContexts = @@ -96,7 +96,7 @@ export function ingestHostBinding( // with ordinary components. This would allow us to share a lot more ingestion code. export function ingestHostProperty( job: HostBindingCompilationJob, property: e.ParsedProperty, bindingKind: ir.BindingKind, - isTextAttribute: boolean, securityContexts: SecurityContext[]): void { + securityContexts: SecurityContext[]): void { let expression: o.Expression|ir.Interpolation; const ast = property.expression.ast; if (ast instanceof e.Interpolation) { @@ -106,19 +106,20 @@ export function ingestHostProperty( expression = convertAst(ast, job, property.sourceSpan); } job.root.update.push(ir.createBindingOp( - job.root.xref, bindingKind, property.name, expression, null, securityContexts, - isTextAttribute, false, null, /* TODO: How do Host bindings handle i18n attrs? */ null, - property.sourceSpan)); + job.root.xref, bindingKind, property.name, expression, null, securityContexts, false, false, + null, /* TODO: How do Host bindings handle i18n attrs? */ null, property.sourceSpan)); } export function ingestHostAttribute( job: HostBindingCompilationJob, name: string, value: o.Expression, securityContexts: SecurityContext[]): void { const attrBinding = ir.createBindingOp( - job.root.xref, ir.BindingKind.Attribute, name, value, null, securityContexts, true, false, - null, + job.root.xref, ir.BindingKind.Attribute, name, value, null, securityContexts, + /* Host attributes should always be extracted to const hostAttrs, even if they are not + *strictly* text literals */ + true, false, null, /* TODO */ null, - /* TODO: host attribute source spans */ null!); + /** TODO: May be null? */ value.sourceSpan!); job.root.update.push(attrBinding); } diff --git a/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts b/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts index 0bf2d914110..2bc529c408c 100644 --- a/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts +++ b/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts @@ -106,10 +106,10 @@ function extractAttributeOp( return; } - let extractable = op.expression.isConstant(); + let extractable = op.isTextAttribute || op.expression.isConstant(); if (unit.job.compatibility === ir.CompatibilityMode.TemplateDefinitionBuilder) { // TemplateDefinitionBuilder only extracted attributes that were string literals. - extractable = ir.isStringLiteral(op.expression); + extractable = op.isTextAttribute || ir.isStringLiteral(op.expression); if (op.name === 'style' || op.name === 'class') { // For style and class attributes, TemplateDefinitionBuilder only extracted them if they were // text attributes. For example, `[attr.class]="'my-class'"` was not extracted despite being a