fix(compiler): project empty block root node (#53620)

Expands the workaround from #52414 to allow for the root nodes of `@empty` blocks to be content projected.

Fixes #53570.

PR Close #53620
This commit is contained in:
Kristiyan Kostadinov 2023-12-18 18:02:39 +02:00 committed by Andrew Scott
parent 6c8faaa769
commit df8a825910
12 changed files with 180 additions and 16 deletions

View file

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {BindingPipe, PropertyRead, PropertyWrite, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstReference, TmplAstTemplate, TmplAstVariable, TmplAstViewportDeferredTrigger} from '@angular/compiler';
import {BindingPipe, PropertyRead, PropertyWrite, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstForLoopBlock, TmplAstForLoopBlockEmpty, TmplAstHoverDeferredTrigger, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstReference, TmplAstTemplate, TmplAstVariable, TmplAstViewportDeferredTrigger} from '@angular/compiler';
import ts from 'typescript';
import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '../../diagnostics';
@ -107,7 +107,7 @@ export interface OutOfBandDiagnosticRecorder {
controlFlowPreventingContentProjection(
templateId: TemplateId, category: ts.DiagnosticCategory,
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty,
preservesWhitespaces: boolean): void;
}
@ -353,9 +353,17 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
controlFlowPreventingContentProjection(
templateId: TemplateId, category: ts.DiagnosticCategory,
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty,
preservesWhitespaces: boolean): void {
const blockName = controlFlowNode instanceof TmplAstIfBlockBranch ? '@if' : '@for';
let blockName: string;
if (controlFlowNode instanceof TmplAstForLoopBlockEmpty) {
blockName = '@empty';
} else if (controlFlowNode instanceof TmplAstForLoopBlock) {
blockName = '@for';
} else {
blockName = '@if';
}
const lines = [
`Node matches the "${slotSelector}" slot of the "${
componentName}" component, but will not be projected into the specific slot because the surrounding ${

View file

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {AST, BindingPipe, BindingType, BoundTarget, Call, createCssSelectorFromNode, CssSelector, DYNAMIC_TYPE, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, R3Identifiers, SafeCall, SafePropertyRead, SchemaMetadata, SelectorMatcher, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstSwitchBlockCase, TmplAstTemplate, TmplAstText, TmplAstTextAttribute, TmplAstVariable, TmplAstViewportDeferredTrigger, TransplantedType} from '@angular/compiler';
import {AST, BindingPipe, BindingType, BoundTarget, Call, createCssSelectorFromNode, CssSelector, DYNAMIC_TYPE, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, R3Identifiers, SafeCall, SafePropertyRead, SchemaMetadata, SelectorMatcher, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstElement, TmplAstForLoopBlock, TmplAstForLoopBlockEmpty, TmplAstHoverDeferredTrigger, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstSwitchBlockCase, TmplAstTemplate, TmplAstText, TmplAstTextAttribute, TmplAstVariable, TmplAstViewportDeferredTrigger, TransplantedType} from '@angular/compiler';
import ts from 'typescript';
import {Reference} from '../../imports';
@ -992,14 +992,19 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
}
private findPotentialControlFlowNodes() {
const result: Array<TmplAstIfBlockBranch|TmplAstForLoopBlock> = [];
const result: Array<TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty> = [];
for (const child of this.element.children) {
let eligibleNode: TmplAstForLoopBlock|TmplAstIfBlockBranch|null = null;
// Only `@for` blocks and the first branch of `@if` blocks participate in content projection.
if (child instanceof TmplAstForLoopBlock) {
eligibleNode = child;
if (this.shouldCheck(child)) {
result.push(child);
}
if (child.empty !== null && this.shouldCheck(child.empty)) {
result.push(child.empty);
}
} else if (child instanceof TmplAstIfBlock) {
eligibleNode = child.branches[0]; // @if blocks are guaranteed to have at least one branch.
}
@ -1033,6 +1038,32 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
return result;
}
private shouldCheck(node: TmplAstNode&{children: TmplAstNode[]}): boolean {
// Skip nodes with less than two children since it's impossible
// for them to run into the issue that we're checking for.
if (node.children.length < 2) {
return false;
}
// Count the number of root nodes while skipping empty text where relevant.
const rootNodeCount = node.children.reduce((count, node) => {
// Normally `preserveWhitspaces` would have been accounted for during parsing, however
// in `ngtsc/annotations/component/src/resources.ts#parseExtractedTemplate` we enable
// `preserveWhitespaces` to preserve the accuracy of source maps diagnostics. This means
// that we have to account for it here since the presence of text nodes affects the
// content projection behavior.
if (!(node instanceof TmplAstText) || this.tcb.hostPreserveWhitespaces ||
node.value.trim().length > 0) {
count++;
}
return count;
}, 0);
// Content projection can only be affected if there is more than one root node.
return rootNodeCount > 1;
}
}
/**

View file

@ -1740,6 +1740,8 @@ MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PL
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: `
@for (item of items; track item) {
<div foo="1" bar="2">{{item}}</div>
} @empty {
<span empty-foo="1" empty-bar="2">Empty!</span>
}
`, isInline: true });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
@ -1748,6 +1750,8 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
template: `
@for (item of items; track item) {
<div foo="1" bar="2">{{item}}</div>
} @empty {
<span empty-foo="1" empty-bar="2">Empty!</span>
}
`,
}]
@ -1777,6 +1781,8 @@ MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PL
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: `
@for (item of items; track item) {
<ng-template foo="1" bar="2">{{item}}</ng-template>
} @empty {
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
}
`, isInline: true });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
@ -1785,6 +1791,8 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
template: `
@for (item of items; track item) {
<ng-template foo="1" bar="2">{{item}}</ng-template>
} @empty {
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
}
`,
}]

View file

@ -550,6 +550,7 @@
},
{
"description": "should generate a for block with an element root node",
"skipForTemplatePipeline": true,
"inputFiles": [
"for_element_root_node.ts"
],
@ -567,6 +568,7 @@
},
{
"description": "should generate a for block with an ng-template root node",
"skipForTemplatePipeline": true,
"inputFiles": [
"for_template_root_node.ts"
],

View file

@ -4,6 +4,8 @@ import {Component} from '@angular/core';
template: `
@for (item of items; track item) {
<div foo="1" bar="2">{{item}}</div>
} @empty {
<span empty-foo="1" empty-bar="2">Empty!</span>
}
`,
})

View file

@ -1,3 +1,3 @@
consts: [["foo", "1", "bar", "2"]]
consts: [["foo", "1", "bar", "2"], ["empty-foo", "1", "empty-bar", "2"]]
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 2, 1, "div", 0, i0.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 2, 1, "div", 0, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 2, 0, "span", 1);

View file

@ -4,6 +4,8 @@ import {Component} from '@angular/core';
template: `
@for (item of items; track item) {
<ng-template foo="1" bar="2">{{item}}</ng-template>
} @empty {
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
}
`,
})

View file

@ -1,3 +1,3 @@
consts: [["foo", "1", "bar", "2"]]
consts: [["foo", "1", "bar", "2"], ["empty-foo", "1", "empty-bar", "2"]]
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 0, null, 0, i0.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 0, null, 0, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 1, 0, null, 1);

View file

@ -5196,6 +5196,42 @@ suppress
`not be projected into the specific slot because the surrounding @for has more than one node at its root.`);
});
it('should report when an @empty block prevents content from being projected', () => {
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'comp',
template: '<ng-content/> <ng-content select="bar, [foo]"/>',
standalone: true,
})
class Comp {}
@Component({
standalone: true,
imports: [Comp],
template: \`
<comp>
@for (i of [1, 2, 3]; track i) {
} @empty {
<div foo></div>
breaks projection
}
</comp>
\`,
})
class TestCmp {}
`);
const diags =
env.driveDiagnostics().map(d => ts.flattenDiagnosticMessageText(d.messageText, ''));
expect(diags.length).toBe(1);
expect(diags[0]).toContain(
`Node matches the "bar, [foo]" slot of the "Comp" component, but will ` +
`not be projected into the specific slot because the surrounding @empty has more than one node at its root.`);
});
it('should report nodes that are targeting different slots but cannot be projected', () => {
env.write('test.ts', `
import {Component} from '@angular/core';

View file

@ -1498,7 +1498,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
* node.
* @param node Node for which to infer the projection data.
*/
private inferProjectionDataFromInsertionPoint(node: t.IfBlockBranch|t.ForLoopBlock) {
private inferProjectionDataFromInsertionPoint(node: t.IfBlockBranch|t.ForLoopBlock|
t.ForLoopBlockEmpty) {
let root: t.Element|t.Template|null = null;
let tagName: string|null = null;
let attrsExprs: o.Expression[]|undefined;
@ -1560,8 +1561,13 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
const {expression: trackByExpression, usesComponentInstance: trackByUsesComponentInstance} =
this.createTrackByFunction(block);
let emptyData: TemplateData|null = null;
let emptyTagName: string|null = null;
let emptyAttrsExprs: o.Expression[]|undefined;
if (block.empty !== null) {
const emptyInferred = this.inferProjectionDataFromInsertionPoint(block.empty);
emptyTagName = emptyInferred.tagName;
emptyAttrsExprs = emptyInferred.attrsExprs;
emptyData = this.prepareEmbeddedTemplateFn(
block.empty.children, '_ForEmpty', undefined, block.empty.i18n);
// Allocate an extra slot for the empty block tracking.
@ -1585,13 +1591,14 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
if (emptyData !== null) {
params.push(
o.literal(trackByUsesComponentInstance), o.variable(emptyData.name),
o.literal(emptyData.getConstCount()), o.literal(emptyData.getVarCount()));
o.literal(emptyData.getConstCount()), o.literal(emptyData.getVarCount()),
o.literal(emptyTagName), this.addAttrsToConsts(emptyAttrsExprs || null));
} else if (trackByUsesComponentInstance) {
// If the tracking function doesn't use the component instance, we can omit the flag.
params.push(o.literal(trackByUsesComponentInstance));
}
return params;
return trimTrailingNulls(params);
});
// Note: the expression needs to be processed *after* the template,

View file

@ -138,6 +138,8 @@ class RepeaterMetadata {
* @param emptyTemplateFn Reference to the template function of the empty block.
* @param emptyDecls The number of nodes, local refs, and pipes for the empty block.
* @param emptyVars The number of bindings for the empty block.
* @param emptyTagName The name of the empty block container element, if applicable
* @param emptyAttrsIndex Index of the empty block template attributes in the `consts` array.
*
* @codeGenApi
*/
@ -145,7 +147,8 @@ export function ɵɵrepeaterCreate(
index: number, templateFn: ComponentTemplate<unknown>, decls: number, vars: number,
tagName: string|null, attrsIndex: number|null, trackByFn: TrackByFunction<unknown>,
trackByUsesComponentInstance?: boolean, emptyTemplateFn?: ComponentTemplate<unknown>,
emptyDecls?: number, emptyVars?: number): void {
emptyDecls?: number, emptyVars?: number, emptyTagName?: string|null,
emptyAttrsIndex?: number|null): void {
performanceMarkFeature('NgControlFlow');
const hasEmptyBlock = emptyTemplateFn !== undefined;
const hostLView = getLView();
@ -165,7 +168,7 @@ export function ɵɵrepeaterCreate(
ngDevMode &&
assertDefined(emptyVars, 'Missing number of bindings for the empty repeater block.');
ɵɵtemplate(index + 2, emptyTemplateFn, emptyDecls!, emptyVars!);
ɵɵtemplate(index + 2, emptyTemplateFn, emptyDecls!, emptyVars!, emptyTagName, emptyAttrsIndex);
}
}

View file

@ -388,6 +388,71 @@ describe('control flow - for', () => {
expect(fixture.nativeElement.textContent).toBe('Main: Before After Slot: 123');
});
it('should project an @empty block with a single root node into the root node slot', () => {
@Component({
standalone: true,
selector: 'test',
template: 'Main: <ng-content/> Slot: <ng-content select="[foo]"/>',
})
class TestComponent {
}
@Component({
standalone: true,
imports: [TestComponent],
template: `
<test>Before @for (item of items; track $index) {} @empty {
<span foo>Empty</span>
} After</test>
`
})
class App {
items = [];
}
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('Main: Before After Slot: Empty');
});
it('should allow @for and @empty blocks to be projected into different slots', () => {
@Component({
standalone: true,
selector: 'test',
template:
'Main: <ng-content/> Loop slot: <ng-content select="[loop]"/> Empty slot: <ng-content select="[empty]"/>',
})
class TestComponent {
}
@Component({
standalone: true,
imports: [TestComponent],
template: `
<test>Before @for (item of items; track $index) {
<span loop>{{item}}</span>
} @empty {
<span empty>Empty</span>
} After</test>
`
})
class App {
items = [1, 2, 3];
}
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.nativeElement.textContent)
.toBe('Main: Before After Loop slot: 123 Empty slot: ');
fixture.componentInstance.items = [];
fixture.detectChanges();
expect(fixture.nativeElement.textContent)
.toBe('Main: Before After Loop slot: Empty slot: Empty');
});
it('should project an @for with multiple root nodes into the catch-all slot', () => {
@Component({
standalone: true,