From 0df78286370654d07d7eb83f822f051d4048dbb2 Mon Sep 17 00:00:00 2001 From: Dylan Hunn Date: Thu, 24 Aug 2023 20:58:30 -0700 Subject: [PATCH] refactor(compiler): Extract host binding static attributes to `hostAttrs` (#51498) Host bindings can apply static attributes. These will be extracted to a `hostAttrs` field on the host binding function's metadata. In order to achieve this, we add an `attributes` field to the host binding job. Then, we peform attribute exraction on host bindings. We finally populate the `attributes` field directly, instead of relying on a `consts` array. PR Close #51498 --- .../host_bindings/TEST_CASES.json | 3 +- .../compiler/src/render3/view/compiler.ts | 2 + .../src/template/pipeline/src/emit.ts | 2 + .../src/template/pipeline/src/ingest.ts | 12 ++++ .../src/phases/attribute_extraction.ts | 26 ++++---- .../pipeline/src/phases/const_collection.ts | 66 ++++++++++++------- .../template/pipeline/src/util/elements.ts | 6 +- 7 files changed, 77 insertions(+), 40 deletions(-) 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 37cc78b519b..51ac7295d9b 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 @@ -70,8 +70,7 @@ "host_attributes.js" ] } - ], - "skipForTemplatePipeline": true + ] }, { "description": "should support host attributes together with host classes and styles", diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index c7324dd0ccb..a30fe6d1c39 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -580,6 +580,7 @@ function createHostBindingsFunction( componentName: name, properties: bindings, events: eventBindings, + attributes: hostBindingsMetadata.attributes, }, bindingParser, constantPool); transformHostBinding(hostJob); @@ -588,6 +589,7 @@ function createHostBindingsFunction( if (varCount !== null && varCount > 0) { definitionMap.set('hostVars', o.literal(varCount)); } + definitionMap.set('hostAttrs', hostJob.root.attributes); return emitHostBindingFunction(hostJob); } diff --git a/packages/compiler/src/template/pipeline/src/emit.ts b/packages/compiler/src/template/pipeline/src/emit.ts index 581d52e23ba..368298d259f 100644 --- a/packages/compiler/src/template/pipeline/src/emit.ts +++ b/packages/compiler/src/template/pipeline/src/emit.ts @@ -109,7 +109,9 @@ export function transformHostBinding(job: HostBindingCompilationJob): void { phaseHostStylePropertyParsing(job); phaseStyleBindingSpecialization(job); phaseBindingSpecialization(job); + phaseAttributeExtraction(job); phasePureLiteralStructures(job); + phaseConstCollection(job); phaseNullishCoalescing(job); phaseExpandSafeReads(job); phaseTemporaryVariables(job); diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index 4054b02d18b..3ec0819a676 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -37,6 +37,7 @@ export function ingestComponent( export interface HostBindingInput { componentName: string; properties: e.ParsedProperty[]|null; + attributes: {[key: string]: o.Expression}; events: e.ParsedEvent[]|null; } @@ -51,6 +52,9 @@ export function ingestHostBinding( for (const property of input.properties ?? []) { ingestHostProperty(job, property, false); } + for (const [name, expr] of Object.entries(input.attributes) ?? []) { + ingestHostAttribute(job, name, expr); + } for (const event of input.events ?? []) { ingestHostEvent(job, event); } @@ -82,6 +86,14 @@ export function ingestHostProperty( isTextAttribute, false, property.sourceSpan)); } +export function ingestHostAttribute( + job: HostBindingCompilationJob, name: string, value: o.Expression): void { + const attrBinding = ir.createBindingOp( + job.root.xref, ir.BindingKind.Attribute, name, value, null, SecurityContext.NONE, true, false, + /* TODO: host attribute source spans */ null!); + job.root.update.push(attrBinding); +} + export function ingestHostEvent(job: HostBindingCompilationJob, event: e.ParsedEvent) {} /** 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 fe5f6f52358..2147d6480e7 100644 --- a/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts +++ b/packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts @@ -8,14 +8,14 @@ import * as ir from '../../ir'; -import {ComponentCompilationJob, ViewCompilationUnit} from '../compilation'; +import {HostBindingCompilationJob, type CompilationJob, type CompilationUnit} from '../compilation'; import {getElementsByXrefId} from '../util/elements'; /** * Find all extractable attribute and binding ops, and create ExtractedAttributeOps for them. * In cases where no instruction needs to be generated for the attribute or binding, it is removed. */ -export function phaseAttributeExtraction(cpl: ComponentCompilationJob): void { +export function phaseAttributeExtraction(cpl: CompilationJob): void { for (const unit of cpl.units) { const elements = getElementsByXrefId(unit); for (const op of unit.ops()) { @@ -72,15 +72,13 @@ function lookupElement( * Extracts an attribute binding. */ function extractAttributeOp( - view: ViewCompilationUnit, op: ir.AttributeOp, - elements: Map) { + unit: CompilationUnit, op: ir.AttributeOp, elements: Map) { if (op.expression instanceof ir.Interpolation) { return; } - const ownerOp = lookupElement(elements, op.target); let extractable = op.expression.isConstant(); - if (view.job.compatibility === ir.CompatibilityMode.TemplateDefinitionBuilder) { + if (unit.job.compatibility === ir.CompatibilityMode.TemplateDefinitionBuilder) { // TemplateDefinitionBuilder only extracted attributes that were string literals. extractable = ir.isStringLiteral(op.expression); if (op.name === 'style' || op.name === 'class') { @@ -92,11 +90,17 @@ function extractAttributeOp( } if (extractable) { - ir.OpList.insertBefore( - ir.createExtractedAttributeOp( - op.target, op.isTemplate ? ir.BindingKind.Template : ir.BindingKind.Attribute, op.name, - op.expression), - ownerOp); + const extractedAttributeOp = ir.createExtractedAttributeOp( + op.target, op.isTemplate ? ir.BindingKind.Template : ir.BindingKind.Attribute, op.name, + op.expression); + if (unit.job instanceof HostBindingCompilationJob) { + // This attribute will apply to the enclosing host binding compilation unit, so order doesn't + // matter. + unit.create.push(extractedAttributeOp); + } else { + const ownerOp = lookupElement(elements, op.target); + ir.OpList.insertBefore(extractedAttributeOp, ownerOp); + } ir.OpList.remove(op); } } diff --git a/packages/compiler/src/template/pipeline/src/phases/const_collection.ts b/packages/compiler/src/template/pipeline/src/phases/const_collection.ts index 841b50f50d6..901627c9cc7 100644 --- a/packages/compiler/src/template/pipeline/src/phases/const_collection.ts +++ b/packages/compiler/src/template/pipeline/src/phases/const_collection.ts @@ -10,29 +10,32 @@ import * as core from '../../../../core'; import {splitNsName} from '../../../../ml_parser/tags'; import * as o from '../../../../output/output_ast'; import * as ir from '../../ir'; -import {ComponentCompilationJob} from '../compilation'; +import {HostBindingCompilationJob, type CompilationJob, ComponentCompilationJob} from '../compilation'; +import {element} from '../instruction'; /** * Converts the semantic attributes of element-like operations (elements, templates) into constant * array expressions, and lifts them into the overall component `consts`. */ -export function phaseConstCollection(job: ComponentCompilationJob): void { - // Serialize the extracted messages into the const array. - const messageConstIndices: {[id: ir.XrefId]: ir.ConstIndex} = {}; - for (const unit of job.units) { - for (const op of unit.create) { - if (op.kind === ir.OpKind.ExtractedMessage) { - messageConstIndices[op.owner] = job.addConst(op.expression, op.statements); - ir.OpList.remove(op); +export function phaseConstCollection(job: CompilationJob): void { + if (job instanceof ComponentCompilationJob) { + // Serialize the extracted messages into the const array. + const messageConstIndices: {[id: ir.XrefId]: ir.ConstIndex} = {}; + for (const unit of job.units) { + for (const op of unit.create) { + if (op.kind === ir.OpKind.ExtractedMessage) { + messageConstIndices[op.owner] = job.addConst(op.expression, op.statements); + ir.OpList.remove(op); + } } } - } - // Assign const index to i18n ops that messages were extracted from. - for (const unit of job.units) { - for (const op of unit.create) { - if (op.kind === ir.OpKind.I18nStart && messageConstIndices[op.xref] !== undefined) { - op.messageIndex = messageConstIndices[op.xref]; + // Assign const index to i18n ops that messages were extracted from. + for (const unit of job.units) { + for (const op of unit.create) { + if (op.kind === ir.OpKind.I18nStart && messageConstIndices[op.xref] !== undefined) { + op.messageIndex = messageConstIndices[op.xref]; + } } } } @@ -51,19 +54,34 @@ export function phaseConstCollection(job: ComponentCompilationJob): void { } // Serialize the extracted attributes into the const array. - for (const unit of job.units) { - for (const op of unit.create) { - if (op.kind === ir.OpKind.Element || op.kind === ir.OpKind.ElementStart || - op.kind === ir.OpKind.Template) { - const attributes = elementAttributes.get(op.xref); - if (attributes !== undefined) { - const attrArray = serializeAttributes(attributes); - if (attrArray.entries.length > 0) { - op.attributes = job.addConst(attrArray); + if (job instanceof ComponentCompilationJob) { + for (const unit of job.units) { + for (const op of unit.create) { + if (op.kind === ir.OpKind.Element || op.kind === ir.OpKind.ElementStart || + op.kind === ir.OpKind.Template) { + const attributes = elementAttributes.get(op.xref); + if (attributes !== undefined) { + const attrArray = serializeAttributes(attributes); + if (attrArray.entries.length > 0) { + op.attributes = job.addConst(attrArray); + } } } } } + } else if (job instanceof HostBindingCompilationJob) { + // TODO: If the host binding case further diverges, we may want to split it into its own + // phase. + for (const [xref, attributes] of elementAttributes.entries()) { + if (xref !== job.root.xref) { + throw new Error( + `An attribute would be const collected into the host binding's template function, but is not associated with the root xref.`); + } + const attrArray = serializeAttributes(attributes); + if (attrArray.entries.length > 0) { + job.root.attributes = attrArray; + } + } } } diff --git a/packages/compiler/src/template/pipeline/src/util/elements.ts b/packages/compiler/src/template/pipeline/src/util/elements.ts index 3c422bc79a0..767ae00d33f 100644 --- a/packages/compiler/src/template/pipeline/src/util/elements.ts +++ b/packages/compiler/src/template/pipeline/src/util/elements.ts @@ -7,14 +7,14 @@ */ import * as ir from '../../ir'; -import {CompilationUnit} from '../compilation'; +import type {CompilationUnit} from '../compilation'; /** * Gets a map of all elements in the given view by their xref id. */ -export function getElementsByXrefId(view: CompilationUnit) { +export function getElementsByXrefId(unit: CompilationUnit) { const elements = new Map(); - for (const op of view.create) { + for (const op of unit.create) { if (!ir.isElementOrContainerOp(op)) { continue; }