Skip to content

Commit

Permalink
fix(compiler): project empty block root node in template pipeline (#5…
Browse files Browse the repository at this point in the history
…3620)

Updates the template pipeline to allow for the root node of an `@empty` block to be content projected.

PR Close #53620
  • Loading branch information
crisbeto authored and atscott committed Dec 19, 2023
1 parent df8a825 commit 478d622
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 13 deletions.
Expand Up @@ -550,7 +550,6 @@
},
{
"description": "should generate a for block with an element root node",
"skipForTemplatePipeline": true,
"inputFiles": [
"for_element_root_node.ts"
],
Expand All @@ -568,7 +567,6 @@
},
{
"description": "should generate a for block with an ng-template root node",
"skipForTemplatePipeline": true,
"inputFiles": [
"for_template_root_node.ts"
],
Expand Down
16 changes: 15 additions & 1 deletion packages/compiler/src/template/pipeline/ir/src/ops/create.ts
Expand Up @@ -281,6 +281,17 @@ export interface RepeaterCreateOp extends ElementOpBase, ConsumesVarsTrait {
*/
functionNameSuffix: string;

/**
* Tag name for the empty block.
*/
emptyTag: string|null;

/**
* Attributes of various kinds on the empty block. Represented as a `ConstIndex` pointer into the
* shared `consts` array of the component compilation.
*/
emptyAttributes: ConstIndex|null;

/**
* The i18n placeholder for the repeated item template.
*/
Expand All @@ -305,7 +316,8 @@ export interface RepeaterVarNames {

export function createRepeaterCreateOp(
primaryView: XrefId, emptyView: XrefId|null, tag: string|null, track: o.Expression,
varNames: RepeaterVarNames, i18nPlaceholder: i18n.BlockPlaceholder|undefined,
varNames: RepeaterVarNames, emptyTag: string|null,
i18nPlaceholder: i18n.BlockPlaceholder|undefined,
emptyI18nPlaceholder: i18n.BlockPlaceholder|undefined, startSourceSpan: ParseSourceSpan,
wholeSourceSpan: ParseSourceSpan): RepeaterCreateOp {
return {
Expand All @@ -317,6 +329,8 @@ export function createRepeaterCreateOp(
track,
trackByFn: null,
tag,
emptyTag,
emptyAttributes: null,
functionNameSuffix: 'For',
namespace: Namespace.HTML,
nonBindable: false,
Expand Down
9 changes: 6 additions & 3 deletions packages/compiler/src/template/pipeline/src/ingest.ts
Expand Up @@ -603,9 +603,11 @@ function ingestForBlock(unit: ViewCompilationUnit, forBlock: t.ForLoopBlock): vo
ingestNodes(repeaterView, forBlock.children);

let emptyView: ViewCompilationUnit|null = null;
let emptyTagName: string|null = null;
if (forBlock.empty !== null) {
emptyView = unit.job.allocateView(unit.xref);
ingestNodes(emptyView, forBlock.empty.children);
emptyTagName = ingestControlFlowInsertionPoint(unit, emptyView.xref, forBlock.empty);
}

const varNames: ir.RepeaterVarNames = {
Expand All @@ -630,8 +632,8 @@ function ingestForBlock(unit: ViewCompilationUnit, forBlock: t.ForLoopBlock): vo

const tagName = ingestControlFlowInsertionPoint(unit, repeaterView.xref, forBlock);
const repeaterCreate = ir.createRepeaterCreateOp(
repeaterView.xref, emptyView?.xref ?? null, tagName, track, varNames, i18nPlaceholder,
emptyI18nPlaceholder, forBlock.startSourceSpan, forBlock.sourceSpan);
repeaterView.xref, emptyView?.xref ?? null, tagName, track, varNames, emptyTagName,
i18nPlaceholder, emptyI18nPlaceholder, forBlock.startSourceSpan, forBlock.sourceSpan);
unit.create.push(repeaterCreate);

const expression = convertAst(
Expand Down Expand Up @@ -1124,7 +1126,8 @@ function convertSourceSpan(
* @returns Tag name to be used for the control flow template.
*/
function ingestControlFlowInsertionPoint(
unit: ViewCompilationUnit, xref: ir.XrefId, node: t.IfBlockBranch|t.ForLoopBlock): string|null {
unit: ViewCompilationUnit, xref: ir.XrefId,
node: t.IfBlockBranch|t.ForLoopBlock|t.ForLoopBlockEmpty): string|null {
let root: t.Element|t.Template|null = null;

for (const child of node.children) {
Expand Down
7 changes: 7 additions & 0 deletions packages/compiler/src/template/pipeline/src/instruction.ts
Expand Up @@ -264,6 +264,7 @@ export function repeaterCreate(
slot: number, viewFnName: string, decls: number, vars: number, tag: string|null,
constIndex: number|null, trackByFn: o.Expression, trackByUsesComponentInstance: boolean,
emptyViewFnName: string|null, emptyDecls: number|null, emptyVars: number|null,
emptyTag: string|null, emptyConstIndex: number|null,
sourceSpan: ParseSourceSpan|null): ir.CreateOp {
const args = [
o.literal(slot),
Expand All @@ -278,6 +279,12 @@ export function repeaterCreate(
args.push(o.literal(trackByUsesComponentInstance));
if (emptyViewFnName !== null) {
args.push(o.variable(emptyViewFnName), o.literal(emptyDecls), o.literal(emptyVars));
if (emptyTag !== null || emptyConstIndex !== null) {
args.push(o.literal(emptyTag));
}
if (emptyConstIndex !== null) {
args.push(o.literal(emptyConstIndex));
}
}
}
return call(Identifiers.repeaterCreate, args, sourceSpan);
Expand Down
Expand Up @@ -39,12 +39,14 @@ export function collectElementConsts(job: CompilationJob): void {
for (const unit of job.units) {
for (const op of unit.create) {
if (ir.isElementOrContainerOp(op)) {
const attributes = allElementAttributes.get(op.xref);
if (attributes !== undefined) {
const attrArray = serializeAttributes(attributes);
if (attrArray.entries.length > 0) {
op.attributes = job.addConst(attrArray);
}
op.attributes = getConstIndex(job, allElementAttributes, op.xref);

// TODO(dylhunn): `@for` loops with `@empty` blocks need to be special-cased here,
// because the slot consumer trait currently only supports one slot per consumer and we
// need two. This should be revisited when making the refactors mentioned in:
// https://github.com/angular/angular/pull/53620#discussion_r1430918822
if (op.kind === ir.OpKind.RepeaterCreate && op.emptyView !== null) {
op.emptyAttributes = getConstIndex(job, allElementAttributes, op.emptyView);
}
}
}
Expand All @@ -65,6 +67,19 @@ export function collectElementConsts(job: CompilationJob): void {
}
}

function getConstIndex(
job: ComponentCompilationJob, allElementAttributes: Map<ir.XrefId, ElementAttributes>,
xref: ir.XrefId): ir.ConstIndex|null {
const attributes = allElementAttributes.get(xref);
if (attributes !== undefined) {
const attrArray = serializeAttributes(attributes);
if (attrArray.entries.length > 0) {
return job.addConst(attrArray);
}
}
return null;
}

/**
* Shared instance of an empty array to avoid unnecessary array allocations.
*/
Expand Down
Expand Up @@ -239,7 +239,7 @@ function reifyCreateOperations(unit: CompilationUnit, ops: ir.OpList<ir.CreateOp
ng.repeaterCreate(
op.handle.slot, repeaterView.fnName, op.decls!, op.vars!, op.tag, op.attributes,
op.trackByFn!, op.usesComponentInstance, emptyViewFnName, emptyDecls, emptyVars,
op.wholeSourceSpan));
op.emptyTag, op.emptyAttributes, op.wholeSourceSpan));
break;
case ir.OpKind.Statement:
// Pass statement operations directly through.
Expand Down
8 changes: 8 additions & 0 deletions packages/compiler/src/template/pipeline/src/util/elements.ts
Expand Up @@ -20,6 +20,14 @@ export function createOpXrefMap(unit: CompilationUnit):
continue;
}
map.set(op.xref, op);

// TODO(dylhunn): `@for` loops with `@empty` blocks need to be special-cased here,
// because the slot consumer trait currently only supports one slot per consumer and we
// need two. This should be revisited when making the refactors mentioned in:
// https://github.com/angular/angular/pull/53620#discussion_r1430918822
if (op.kind === ir.OpKind.RepeaterCreate && op.emptyView !== null) {
map.set(op.emptyView, op);
}
}
return map;
}

0 comments on commit 478d622

Please sign in to comment.