Skip to content

Commit df8a825

Browse files
crisbetoatscott
crisbeto
authored and
atscott
committed
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
1 parent 6c8faaa commit df8a825

File tree

12 files changed

+180
-16
lines changed

12 files changed

+180
-16
lines changed

‎packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {BindingPipe, PropertyRead, PropertyWrite, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstReference, TmplAstTemplate, TmplAstVariable, TmplAstViewportDeferredTrigger} from '@angular/compiler';
9+
import {BindingPipe, PropertyRead, PropertyWrite, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstForLoopBlock, TmplAstForLoopBlockEmpty, TmplAstHoverDeferredTrigger, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstReference, TmplAstTemplate, TmplAstVariable, TmplAstViewportDeferredTrigger} from '@angular/compiler';
1010
import ts from 'typescript';
1111

1212
import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '../../diagnostics';
@@ -107,7 +107,7 @@ export interface OutOfBandDiagnosticRecorder {
107107
controlFlowPreventingContentProjection(
108108
templateId: TemplateId, category: ts.DiagnosticCategory,
109109
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
110-
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
110+
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty,
111111
preservesWhitespaces: boolean): void;
112112
}
113113

@@ -353,9 +353,17 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
353353
controlFlowPreventingContentProjection(
354354
templateId: TemplateId, category: ts.DiagnosticCategory,
355355
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
356-
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
356+
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty,
357357
preservesWhitespaces: boolean): void {
358-
const blockName = controlFlowNode instanceof TmplAstIfBlockBranch ? '@if' : '@for';
358+
let blockName: string;
359+
if (controlFlowNode instanceof TmplAstForLoopBlockEmpty) {
360+
blockName = '@empty';
361+
} else if (controlFlowNode instanceof TmplAstForLoopBlock) {
362+
blockName = '@for';
363+
} else {
364+
blockName = '@if';
365+
}
366+
359367
const lines = [
360368
`Node matches the "${slotSelector}" slot of the "${
361369
componentName}" component, but will not be projected into the specific slot because the surrounding ${

‎packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
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';
9+
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';
1010
import ts from 'typescript';
1111

1212
import {Reference} from '../../imports';
@@ -992,14 +992,19 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
992992
}
993993

994994
private findPotentialControlFlowNodes() {
995-
const result: Array<TmplAstIfBlockBranch|TmplAstForLoopBlock> = [];
995+
const result: Array<TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty> = [];
996996

997997
for (const child of this.element.children) {
998998
let eligibleNode: TmplAstForLoopBlock|TmplAstIfBlockBranch|null = null;
999999

10001000
// Only `@for` blocks and the first branch of `@if` blocks participate in content projection.
10011001
if (child instanceof TmplAstForLoopBlock) {
1002-
eligibleNode = child;
1002+
if (this.shouldCheck(child)) {
1003+
result.push(child);
1004+
}
1005+
if (child.empty !== null && this.shouldCheck(child.empty)) {
1006+
result.push(child.empty);
1007+
}
10031008
} else if (child instanceof TmplAstIfBlock) {
10041009
eligibleNode = child.branches[0]; // @if blocks are guaranteed to have at least one branch.
10051010
}
@@ -1033,6 +1038,32 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
10331038

10341039
return result;
10351040
}
1041+
1042+
private shouldCheck(node: TmplAstNode&{children: TmplAstNode[]}): boolean {
1043+
// Skip nodes with less than two children since it's impossible
1044+
// for them to run into the issue that we're checking for.
1045+
if (node.children.length < 2) {
1046+
return false;
1047+
}
1048+
1049+
// Count the number of root nodes while skipping empty text where relevant.
1050+
const rootNodeCount = node.children.reduce((count, node) => {
1051+
// Normally `preserveWhitspaces` would have been accounted for during parsing, however
1052+
// in `ngtsc/annotations/component/src/resources.ts#parseExtractedTemplate` we enable
1053+
// `preserveWhitespaces` to preserve the accuracy of source maps diagnostics. This means
1054+
// that we have to account for it here since the presence of text nodes affects the
1055+
// content projection behavior.
1056+
if (!(node instanceof TmplAstText) || this.tcb.hostPreserveWhitespaces ||
1057+
node.value.trim().length > 0) {
1058+
count++;
1059+
}
1060+
1061+
return count;
1062+
}, 0);
1063+
1064+
// Content projection can only be affected if there is more than one root node.
1065+
return rootNodeCount > 1;
1066+
}
10361067
}
10371068

10381069
/**

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1740,6 +1740,8 @@ MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PL
17401740
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: `
17411741
@for (item of items; track item) {
17421742
<div foo="1" bar="2">{{item}}</div>
1743+
} @empty {
1744+
<span empty-foo="1" empty-bar="2">Empty!</span>
17431745
}
17441746
`, isInline: true });
17451747
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
17481750
template: `
17491751
@for (item of items; track item) {
17501752
<div foo="1" bar="2">{{item}}</div>
1753+
} @empty {
1754+
<span empty-foo="1" empty-bar="2">Empty!</span>
17511755
}
17521756
`,
17531757
}]
@@ -1777,6 +1781,8 @@ MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PL
17771781
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: `
17781782
@for (item of items; track item) {
17791783
<ng-template foo="1" bar="2">{{item}}</ng-template>
1784+
} @empty {
1785+
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
17801786
}
17811787
`, isInline: true });
17821788
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
17851791
template: `
17861792
@for (item of items; track item) {
17871793
<ng-template foo="1" bar="2">{{item}}</ng-template>
1794+
} @empty {
1795+
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
17881796
}
17891797
`,
17901798
}]

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,7 @@
550550
},
551551
{
552552
"description": "should generate a for block with an element root node",
553+
"skipForTemplatePipeline": true,
553554
"inputFiles": [
554555
"for_element_root_node.ts"
555556
],
@@ -567,6 +568,7 @@
567568
},
568569
{
569570
"description": "should generate a for block with an ng-template root node",
571+
"skipForTemplatePipeline": true,
570572
"inputFiles": [
571573
"for_template_root_node.ts"
572574
],

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_element_root_node.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import {Component} from '@angular/core';
44
template: `
55
@for (item of items; track item) {
66
<div foo="1" bar="2">{{item}}</div>
7+
} @empty {
8+
<span empty-foo="1" empty-bar="2">Empty!</span>
79
}
810
`,
911
})

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_element_root_node_template.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
consts: [["foo", "1", "bar", "2"]]
1+
consts: [["foo", "1", "bar", "2"], ["empty-foo", "1", "empty-bar", "2"]]
22
…
3-
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 2, 1, "div", 0, i0.ɵɵrepeaterTrackByIdentity);
3+
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 2, 1, "div", 0, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 2, 0, "span", 1);

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_root_node.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import {Component} from '@angular/core';
44
template: `
55
@for (item of items; track item) {
66
<ng-template foo="1" bar="2">{{item}}</ng-template>
7+
} @empty {
8+
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
79
}
810
`,
911
})

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_template_root_node_template.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
consts: [["foo", "1", "bar", "2"]]
1+
consts: [["foo", "1", "bar", "2"], ["empty-foo", "1", "empty-bar", "2"]]
22
…
3-
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 0, null, 0, i0.ɵɵrepeaterTrackByIdentity);
3+
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 0, null, 0, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 1, 0, null, 1);

‎packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5196,6 +5196,42 @@ suppress
51965196
`not be projected into the specific slot because the surrounding @for has more than one node at its root.`);
51975197
});
51985198

5199+
it('should report when an @empty block prevents content from being projected', () => {
5200+
env.write('test.ts', `
5201+
import {Component} from '@angular/core';
5202+
5203+
@Component({
5204+
selector: 'comp',
5205+
template: '<ng-content/> <ng-content select="bar, [foo]"/>',
5206+
standalone: true,
5207+
})
5208+
class Comp {}
5209+
5210+
@Component({
5211+
standalone: true,
5212+
imports: [Comp],
5213+
template: \`
5214+
<comp>
5215+
@for (i of [1, 2, 3]; track i) {
5216+
5217+
} @empty {
5218+
<div foo></div>
5219+
breaks projection
5220+
}
5221+
</comp>
5222+
\`,
5223+
})
5224+
class TestCmp {}
5225+
`);
5226+
5227+
const diags =
5228+
env.driveDiagnostics().map(d => ts.flattenDiagnosticMessageText(d.messageText, ''));
5229+
expect(diags.length).toBe(1);
5230+
expect(diags[0]).toContain(
5231+
`Node matches the "bar, [foo]" slot of the "Comp" component, but will ` +
5232+
`not be projected into the specific slot because the surrounding @empty has more than one node at its root.`);
5233+
});
5234+
51995235
it('should report nodes that are targeting different slots but cannot be projected', () => {
52005236
env.write('test.ts', `
52015237
import {Component} from '@angular/core';

‎packages/compiler/src/render3/view/template.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1498,7 +1498,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
14981498
* node.
14991499
* @param node Node for which to infer the projection data.
15001500
*/
1501-
private inferProjectionDataFromInsertionPoint(node: t.IfBlockBranch|t.ForLoopBlock) {
1501+
private inferProjectionDataFromInsertionPoint(node: t.IfBlockBranch|t.ForLoopBlock|
1502+
t.ForLoopBlockEmpty) {
15021503
let root: t.Element|t.Template|null = null;
15031504
let tagName: string|null = null;
15041505
let attrsExprs: o.Expression[]|undefined;
@@ -1560,8 +1561,13 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
15601561
const {expression: trackByExpression, usesComponentInstance: trackByUsesComponentInstance} =
15611562
this.createTrackByFunction(block);
15621563
let emptyData: TemplateData|null = null;
1564+
let emptyTagName: string|null = null;
1565+
let emptyAttrsExprs: o.Expression[]|undefined;
15631566

15641567
if (block.empty !== null) {
1568+
const emptyInferred = this.inferProjectionDataFromInsertionPoint(block.empty);
1569+
emptyTagName = emptyInferred.tagName;
1570+
emptyAttrsExprs = emptyInferred.attrsExprs;
15651571
emptyData = this.prepareEmbeddedTemplateFn(
15661572
block.empty.children, '_ForEmpty', undefined, block.empty.i18n);
15671573
// Allocate an extra slot for the empty block tracking.
@@ -1585,13 +1591,14 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
15851591
if (emptyData !== null) {
15861592
params.push(
15871593
o.literal(trackByUsesComponentInstance), o.variable(emptyData.name),
1588-
o.literal(emptyData.getConstCount()), o.literal(emptyData.getVarCount()));
1594+
o.literal(emptyData.getConstCount()), o.literal(emptyData.getVarCount()),
1595+
o.literal(emptyTagName), this.addAttrsToConsts(emptyAttrsExprs || null));
15891596
} else if (trackByUsesComponentInstance) {
15901597
// If the tracking function doesn't use the component instance, we can omit the flag.
15911598
params.push(o.literal(trackByUsesComponentInstance));
15921599
}
15931600

1594-
return params;
1601+
return trimTrailingNulls(params);
15951602
});
15961603

15971604
// Note: the expression needs to be processed *after* the template,

0 commit comments

Comments
 (0)