From 05393cb54527295140ecf8dfcaa4092053e14563 Mon Sep 17 00:00:00 2001 From: Dylan Hunn Date: Mon, 28 Aug 2023 20:22:47 -0700 Subject: [PATCH] refactor(compiler): Support content projection in template pipeline (#51544) Supporting content projection requires us to emit three new kinds of output: 1. An `ngContentSelectors` field on the component metadata, which points to an array in the constant pool with all of the `select` attributes from `` elements. 2. One `projectionDef` instruction at the beginning of each root view template function for a component. That `projectionDef` points to a constant pool expression, which contains *parsed* selectors for all `` elements in the root's entire view tree. 3. A `projection` instruction for each `` slot in the view tree. These each get a data slot, a monotonically increasing "content slot", and a pointer to the tag's attributes in the component const array. We support the first two features entirely within a new compilation phase. The third feature, collection of processed attributes, is a bit trickier. We now treat `` tags as element-like ops, and use the normal attribute ingestion pipeline to process any attributes, and assign the appropriate `ConstIndex`. **Note**: We also split up a number of the tests into two expectations files, one for the view functions, and one for other const listerals from the constant pool. This is because `TemplateDefinitionBuilder` emits the literals in a quirky order (mixed in with the view functions) due to how it lazily generates view functions. Our eager ordering is totally different, but by splitting the expectations, we can still share the same tests with `TemplateDefinitionBuilder`. PR Close #51544 --- .../content_projection/TEST_CASES.json | 32 +++++++--- .../content_projection/nested_template.js | 1 - .../nested_template_consts.js | 1 + .../content_projection/root_and_nested.js | 2 - .../root_and_nested_consts.js | 2 + .../compiler/src/render3/view/compiler.ts | 5 ++ .../src/template/pipeline/ir/src/enums.ts | 16 +++-- .../template/pipeline/ir/src/expression.ts | 2 + .../template/pipeline/ir/src/ops/create.ts | 59 +++++++++++++++++-- .../src/template/pipeline/src/compilation.ts | 6 ++ .../src/template/pipeline/src/conversion.ts | 7 +++ .../src/template/pipeline/src/emit.ts | 2 + .../src/template/pipeline/src/ingest.ts | 21 ++++++- .../src/template/pipeline/src/instruction.ts | 16 +++++ .../pipeline/src/phases/const_collection.ts | 3 +- .../src/phases/generate_projection_def.ts | 53 +++++++++++++++++ .../src/template/pipeline/src/phases/reify.ts | 10 ++++ 17 files changed, 214 insertions(+), 24 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/nested_template_consts.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/root_and_nested_consts.js create mode 100644 packages/compiler/src/template/pipeline/src/phases/generate_projection_def.ts diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/TEST_CASES.json index 1970c55b248..251c9d9d1c4 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/TEST_CASES.json @@ -25,8 +25,7 @@ } ] } - ], - "skipForTemplatePipeline": true + ] }, { "description": "should support multi-slot content projection with multiple wildcard slots", @@ -40,8 +39,7 @@ "multiple_wildcards.js" ] } - ], - "skipForTemplatePipeline": true + ] }, { "description": "should support content projection in nested templates", @@ -54,9 +52,17 @@ "files": [ "nested_template.js" ] + }, + { + "failureMessage": "Invalid content projection instructions generated", + "files": [ + { + "expected": "nested_template_consts.js", + "generated": "nested_template.js" + } + ] } - ], - "skipForTemplatePipeline": true + ] }, { "description": "should support content projection in both the root and nested templates", @@ -69,9 +75,17 @@ "files": [ "root_and_nested.js" ] + }, + { + "failureMessage": "Invalid content projection instructions generated", + "files": [ + { + "expected": "root_and_nested_consts.js", + "generated": "root_and_nested.js" + } + ] } - ], - "skipForTemplatePipeline": true + ] }, { "description": "should parse the selector that is passed into ngProjectAs", @@ -134,4 +148,4 @@ "skipForTemplatePipeline": true } ] -} +} \ No newline at end of file diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/nested_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/nested_template.js index 3f5298d42df..540470ee58b 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/nested_template.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/nested_template.js @@ -16,7 +16,6 @@ function Cmp_ng_template_2_Template(rf, ctx) { $r3$.ɵɵprojection(1, 1); } } -const $_c4$ = [[["span", "title", "tofirst"]], "*"]; // ... consts: [["id", "second", __AttributeMarker.Template__, "ngIf"], ["id", "third", __AttributeMarker.Template__, "ngIf"], ["id", "second"], ["id", "third"]], template: function Cmp_Template(rf, ctx) { diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/nested_template_consts.js b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/nested_template_consts.js new file mode 100644 index 00000000000..1f0f9ca9e49 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/nested_template_consts.js @@ -0,0 +1 @@ +const $_c4$ = [[["span", "title", "tofirst"]], "*"]; diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/root_and_nested.js b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/root_and_nested.js index d7642a6e2a3..e390121fe1f 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/root_and_nested.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/root_and_nested.js @@ -15,8 +15,6 @@ function Cmp_ng_template_2_Template(rf, ctx) { $r3$.ɵɵprojection(1, 4); } } -const $_c0$ = [[["", "id", "tomainbefore"]], [["", "id", "tomainafter"]], [["", "id", "totemplate"]], [["", "id", "tonestedtemplate"]], "*"]; -const $_c1$ = ["[id=toMainBefore]", "[id=toMainAfter]", "[id=toTemplate]", "[id=toNestedTemplate]", "*"]; // ... template: function Cmp_Template(rf, ctx) { if (rf & 1) { diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/root_and_nested_consts.js b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/root_and_nested_consts.js new file mode 100644 index 00000000000..234c3b6359f --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/content_projection/root_and_nested_consts.js @@ -0,0 +1,2 @@ +const $_c0$ = [[["", "id", "tomainbefore"]], [["", "id", "tomainafter"]], [["", "id", "totemplate"]], [["", "id", "tonestedtemplate"]], "*"]; +const $_c1$ = ["[id=toMainBefore]", "[id=toMainAfter]", "[id=toTemplate]", "[id=toNestedTemplate]", "*"]; \ No newline at end of file diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index 21a5ea562f6..4c015ea3561 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -244,6 +244,11 @@ export function compileComponentFromMetadata( // Finally we emit the template function: const templateFn = emitTemplateFn(tpl, constantPool); + + if (tpl.contentSelectors !== null) { + definitionMap.set('ngContentSelectors', tpl.contentSelectors); + } + definitionMap.set('decls', o.literal(tpl.root.decls as number)); definitionMap.set('vars', o.literal(tpl.root.vars as number)); if (tpl.consts.length > 0) { diff --git a/packages/compiler/src/template/pipeline/ir/src/enums.ts b/packages/compiler/src/template/pipeline/ir/src/enums.ts index 8bd62caba52..96293f44286 100644 --- a/packages/compiler/src/template/pipeline/ir/src/enums.ts +++ b/packages/compiler/src/template/pipeline/ir/src/enums.ts @@ -160,6 +160,16 @@ export enum OpKind { */ Namespace, + /** + * Configure a content projeciton definition for the view. + */ + ProjectionDef, + + /** + * Create a content projection slot. + */ + Projection, + /** * The start of an i18n block. */ @@ -174,8 +184,6 @@ export enum OpKind { * The end of an i18n block. */ I18nEnd, - - // TODO: Add Host Listeners, and possibly other host ops also. } /** @@ -310,8 +318,8 @@ export enum SemanticVariableKind { /** * Whether to compile in compatibilty mode. In compatibility mode, the template pipeline will - * attempt to match the output of `TemplateDefinitionBuilder` as exactly as possible, at the cost of - * producing quirky or larger code in some cases. + * attempt to match the output of `TemplateDefinitionBuilder` as exactly as possible, at the cost + * of producing quirky or larger code in some cases. */ export enum CompatibilityMode { Normal, diff --git a/packages/compiler/src/template/pipeline/ir/src/expression.ts b/packages/compiler/src/template/pipeline/ir/src/expression.ts index 3f3a28524c0..35cd03a4824 100644 --- a/packages/compiler/src/template/pipeline/ir/src/expression.ts +++ b/packages/compiler/src/template/pipeline/ir/src/expression.ts @@ -858,6 +858,8 @@ export function transformExpressionsInOp( transformExpressionsInExpression(op.tagNameParams[placeholder], transform, flags); } break; + case OpKind.Projection: + case OpKind.ProjectionDef: case OpKind.Element: case OpKind.ElementStart: case OpKind.ElementEnd: diff --git a/packages/compiler/src/template/pipeline/ir/src/ops/create.ts b/packages/compiler/src/template/pipeline/ir/src/ops/create.ts index a02d0db7948..99bfb212f76 100644 --- a/packages/compiler/src/template/pipeline/ir/src/ops/create.ts +++ b/packages/compiler/src/template/pipeline/ir/src/ops/create.ts @@ -22,20 +22,21 @@ import type {UpdateOp} from './update'; */ export type CreateOp = ListEndOp|StatementOp|ElementOp|ElementStartOp| ElementEndOp|ContainerOp|ContainerStartOp|ContainerEndOp|TemplateOp|EnableBindingsOp| - DisableBindingsOp|TextOp|ListenerOp|PipeOp|VariableOp|NamespaceOp| - ExtractedAttributeOp|ExtractedMessageOp|I18nOp|I18nStartOp|I18nEndOp; + DisableBindingsOp|TextOp|ListenerOp|PipeOp|VariableOp|NamespaceOp|ProjectionDefOp| + ProjectionOp|ExtractedAttributeOp|ExtractedMessageOp|I18nOp|I18nStartOp|I18nEndOp; /** * An operation representing the creation of an element or container. */ export type ElementOrContainerOps = - ElementOp|ElementStartOp|ContainerOp|ContainerStartOp|TemplateOp; + ElementOp|ElementStartOp|ContainerOp|ContainerStartOp|TemplateOp|ProjectionOp; /** * The set of OpKinds that represent the creation of an element or container */ const elementContainerOpKinds = new Set([ - OpKind.Element, OpKind.ElementStart, OpKind.Container, OpKind.ContainerStart, OpKind.Template + OpKind.Element, OpKind.ElementStart, OpKind.Container, OpKind.ContainerStart, OpKind.Template, + OpKind.Projection ]); /** @@ -445,6 +446,56 @@ export function createNamespaceOp(namespace: Namespace): NamespaceOp { }; } +/** + * An op that creates a content projection slot. + */ +export interface ProjectionDefOp extends Op { + kind: OpKind.ProjectionDef; + + // The parsed selector information for this projection def. + def: o.Expression|null; +} + +export function createProjectionDefOp(def: o.Expression|null): ProjectionDefOp { + return { + kind: OpKind.ProjectionDef, + def, + ...NEW_OP, + }; +} + +/** + * An op that creates a content projection slot. + */ +export interface ProjectionOp extends Op, ConsumesSlotOpTrait, ElementOrContainerOpBase { + kind: OpKind.Projection; + + xref: XrefId; + + slot: number|null; + + projectionSlotIndex: number; + + selector: string; +} + +export function createProjectionOp(xref: XrefId, selector: string): ProjectionOp { + return { + kind: OpKind.Projection, + xref, + selector, + projectionSlotIndex: 0, + attributes: null, + localRefs: [], + nonBindable: false, + i18n: undefined, // TODO + sourceSpan: null!, // TODO + ...NEW_OP, + ...TRAIT_CONSUMES_SLOT, + ...TRAIT_USES_SLOT_INDEX, + }; +} + /** * Represents an attribute that has been extracted for inclusion in the consts array. */ diff --git a/packages/compiler/src/template/pipeline/src/compilation.ts b/packages/compiler/src/template/pipeline/src/compilation.ts index bba92d50101..b89fc3a3cbd 100644 --- a/packages/compiler/src/template/pipeline/src/compilation.ts +++ b/packages/compiler/src/template/pipeline/src/compilation.ts @@ -81,6 +81,12 @@ export class ComponentCompilationJob extends CompilationJob { readonly views = new Map(); + /** + * Causes ngContentSelectors to be emitted, for content projection slots in the view. Possibly a + * reference into the constant pool. + */ + public contentSelectors: o.Expression|null = null; + /** * Add a `ViewCompilation` for a new embedded view to this compilation. */ diff --git a/packages/compiler/src/template/pipeline/src/conversion.ts b/packages/compiler/src/template/pipeline/src/conversion.ts index 7e44208273f..731dd98475f 100644 --- a/packages/compiler/src/template/pipeline/src/conversion.ts +++ b/packages/compiler/src/template/pipeline/src/conversion.ts @@ -53,3 +53,10 @@ export function prefixWithNamespace(strippedTag: string, namespace: ir.Namespace } return `:${keyForNamespace(namespace)}:${strippedTag}`; } + +export function literalOrArrayLiteral(value: any): o.Expression { + if (Array.isArray(value)) { + return o.literalArr(value.map(literalOrArrayLiteral)); + } + return o.literal(value, o.INFERRED_TYPE); +} diff --git a/packages/compiler/src/template/pipeline/src/emit.ts b/packages/compiler/src/template/pipeline/src/emit.ts index 0f25dc8c5aa..ae57144d428 100644 --- a/packages/compiler/src/template/pipeline/src/emit.ts +++ b/packages/compiler/src/template/pipeline/src/emit.ts @@ -54,6 +54,7 @@ import {phaseStyleBindingSpecialization} from './phases/style_binding_specializa import {phaseTemporaryVariables} from './phases/temporary_variables'; import {phaseVarCounting} from './phases/var_counting'; import {phaseVariableOptimization} from './phases/variable_optimization'; +import {phaseGenerateProjectionDef} from './phases/generate_projection_def'; type Phase = { fn: (job: CompilationJob) => void; kind: Kind.Both | Kind.Host | Kind.Tmpl; @@ -81,6 +82,7 @@ const phases: Phase[] = [ {kind: Kind.Tmpl, fn: phasePipeCreation}, {kind: Kind.Tmpl, fn: phasePipeVariadic}, {kind: Kind.Both, fn: phasePureLiteralStructures}, + {kind: Kind.Tmpl, fn: phaseGenerateProjectionDef}, {kind: Kind.Tmpl, fn: phaseGenerateVariables}, {kind: Kind.Tmpl, fn: phaseSaveRestoreView}, {kind: Kind.Tmpl, fn: phaseFindAnyCasts}, diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index 039b568ab08..57f39dfa8cb 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -115,6 +115,8 @@ function ingestNodes(unit: ViewCompilationUnit, template: t.Node[]): void { ingestElement(unit, node); } else if (node instanceof t.Template) { ingestTemplate(unit, node); + } else if (node instanceof t.Content) { + ingestContent(unit, node); } else if (node instanceof t.Text) { ingestText(unit, node); } else if (node instanceof t.BoundText) { @@ -127,8 +129,6 @@ function ingestNodes(unit: ViewCompilationUnit, template: t.Node[]): void { } } - - /** * Ingest an element AST from the template into the given `ViewCompilation`. */ @@ -180,6 +180,23 @@ function ingestTemplate(unit: ViewCompilationUnit, tmpl: t.Template): void { } } +/** + * Ingest a literal text node from the AST into the given `ViewCompilation`. + */ +function ingestContent(unit: ViewCompilationUnit, content: t.Content): void { + const op = ir.createProjectionOp(unit.job.allocateXrefId(), content.selector); + for (const attr of content.attributes) { + // Attributes named 'select' are specifically excluded, because they control which content + // matches as a property of the `projection`, and are not a plain attribute. + if (attr.name.toLowerCase() !== 'select') { + ingestBinding( + unit, op.xref, attr.name, o.literal(attr.value), e.BindingType.Attribute, null, + SecurityContext.NONE, attr.sourceSpan, true, false); + } + } + unit.create.push(op); +} + /** * Ingest a literal text node from the AST into the given `ViewCompilation`. */ diff --git a/packages/compiler/src/template/pipeline/src/instruction.ts b/packages/compiler/src/template/pipeline/src/instruction.ts index 79061b71af3..387058a97e6 100644 --- a/packages/compiler/src/template/pipeline/src/instruction.ts +++ b/packages/compiler/src/template/pipeline/src/instruction.ts @@ -181,6 +181,22 @@ export function text( return call(Identifiers.text, args, sourceSpan); } +export function projectionDef(def: o.Expression|null): ir.CreateOp { + return call(Identifiers.projectionDef, def ? [def] : [], null); +} + +export function projection( + slot: number, projectionSlotIndex: number, attributes: number|null): ir.CreateOp { + const args = [o.literal(slot)]; + if (projectionSlotIndex !== 0 || attributes !== null) { + args.push(o.literal(projectionSlotIndex)); + if (attributes != null) { + args.push(o.literal(attributes)); + } + } + return call(Identifiers.projection, args, null); +} + export function i18nStart(slot: number, constIndex: number): ir.CreateOp { return call(Identifiers.i18nStart, [o.literal(slot), o.literal(constIndex)], null); } 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 901627c9cc7..7fee026f5ed 100644 --- a/packages/compiler/src/template/pipeline/src/phases/const_collection.ts +++ b/packages/compiler/src/template/pipeline/src/phases/const_collection.ts @@ -57,8 +57,7 @@ export function phaseConstCollection(job: CompilationJob): void { 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) { + if (ir.isElementOrContainerOp(op)) { const attributes = elementAttributes.get(op.xref); if (attributes !== undefined) { const attrArray = serializeAttributes(attributes); diff --git a/packages/compiler/src/template/pipeline/src/phases/generate_projection_def.ts b/packages/compiler/src/template/pipeline/src/phases/generate_projection_def.ts new file mode 100644 index 00000000000..88edcc40408 --- /dev/null +++ b/packages/compiler/src/template/pipeline/src/phases/generate_projection_def.ts @@ -0,0 +1,53 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {parseSelectorToR3Selector} from '../../../../core'; +import * as o from '../../../../output/output_ast'; +import * as ir from '../../ir'; +import type {ComponentCompilationJob} from '../compilation'; +import {literalOrArrayLiteral} from '../conversion'; + +/** + * Locate projection slots, populate the each component's `ngContentSelectors` literal field, + * populate `project` arguments, and generate the required `projectionDef` instruction for the job's + * root view. + */ +export function phaseGenerateProjectionDef(job: ComponentCompilationJob): void { + // TODO: Why does TemplateDefinitionBuilder force a shared constant? + const share = job.compatibility === ir.CompatibilityMode.TemplateDefinitionBuilder; + + // Collect all selectors from this component, and its nested views. Also, assign each projection a + // unique ascending projection slot index. + const selectors = []; + let projectionSlotIndex = 0; + for (const unit of job.units) { + for (const op of unit.create) { + if (op.kind === ir.OpKind.Projection) { + selectors.push(op.selector); + op.projectionSlotIndex = projectionSlotIndex++; + } + } + } + + if (selectors.length > 0) { + // Create the projectionDef array. If we only found a single wildcard selector, then we use the + // default behavior with no arguments instead. + let defExpr: o.Expression|null = null; + if (selectors.length > 1 || selectors[0] !== '*') { + const def = selectors.map(s => s === '*' ? s : parseSelectorToR3Selector(s)); + defExpr = job.pool.getConstLiteral(literalOrArrayLiteral(def), share); + } + + // Create the ngContentSelectors constant. + job.contentSelectors = job.pool.getConstLiteral(literalOrArrayLiteral(selectors), share); + + // The projection def instruction goes at the beginning of the root view, before any + // `projection` instructions. + job.root.create.prepend([ir.createProjectionDefOp(defExpr)]); + } +} diff --git a/packages/compiler/src/template/pipeline/src/phases/reify.ts b/packages/compiler/src/template/pipeline/src/phases/reify.ts index bf8ee0182ab..ab5f764fa03 100644 --- a/packages/compiler/src/template/pipeline/src/phases/reify.ts +++ b/packages/compiler/src/template/pipeline/src/phases/reify.ts @@ -140,6 +140,16 @@ function reifyCreateOperations(unit: CompilationUnit, ops: ir.OpList(op, ng.projectionDef(op.def)); + break; + case ir.OpKind.Projection: + if (op.slot === null) { + throw new Error('No slot was assigned for project instruction'); + } + ir.OpList.replace( + op, ng.projection(op.slot, op.projectionSlotIndex, op.attributes)); + break; case ir.OpKind.Statement: // Pass statement operations directly through. break;