Skip to content

Commit 7fc7f3f

Browse files
crisbetodylhunn
authored andcommitted
fix(compiler): capture all control flow branches for content projection in if blocks (#54921)
Previously only the first branch of an `if` block was captured for content projection. This was done because of some planned refactors in the future. Since we've decided not to apply those refactors to conditionals, these changes update the compiler to capture each branch individually for content projection purposes. PR Close #54921
1 parent 1c0ec56 commit 7fc7f3f

File tree

12 files changed

+240
-121
lines changed

12 files changed

+240
-121
lines changed

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

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -395,15 +395,7 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
395395
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
396396
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty,
397397
preservesWhitespaces: boolean): void {
398-
let blockName: string;
399-
if (controlFlowNode instanceof TmplAstForLoopBlockEmpty) {
400-
blockName = '@empty';
401-
} else if (controlFlowNode instanceof TmplAstForLoopBlock) {
402-
blockName = '@for';
403-
} else {
404-
blockName = '@if';
405-
}
406-
398+
const blockName = controlFlowNode.nameSpan.toString().trim();
407399
const lines = [
408400
`Node matches the "${slotSelector}" slot of the "${
409401
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: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,9 +1002,6 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
10021002
const result: Array<TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty> = [];
10031003

10041004
for (const child of this.element.children) {
1005-
let eligibleNode: TmplAstForLoopBlock|TmplAstIfBlockBranch|null = null;
1006-
1007-
// Only `@for` blocks and the first branch of `@if` blocks participate in content projection.
10081005
if (child instanceof TmplAstForLoopBlock) {
10091006
if (this.shouldCheck(child)) {
10101007
result.push(child);
@@ -1013,33 +1010,11 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
10131010
result.push(child.empty);
10141011
}
10151012
} else if (child instanceof TmplAstIfBlock) {
1016-
eligibleNode = child.branches[0]; // @if blocks are guaranteed to have at least one branch.
1017-
}
1018-
1019-
// Skip nodes with less than two children since it's impossible
1020-
// for them to run into the issue that we're checking for.
1021-
if (eligibleNode === null || eligibleNode.children.length < 2) {
1022-
continue;
1023-
}
1024-
1025-
// Count the number of root nodes while skipping empty text where relevant.
1026-
const rootNodeCount = eligibleNode.children.reduce((count, node) => {
1027-
// Normally `preserveWhitspaces` would have been accounted for during parsing, however
1028-
// in `ngtsc/annotations/component/src/resources.ts#parseExtractedTemplate` we enable
1029-
// `preserveWhitespaces` to preserve the accuracy of source maps diagnostics. This means
1030-
// that we have to account for it here since the presence of text nodes affects the
1031-
// content projection behavior.
1032-
if (!(node instanceof TmplAstText) || this.tcb.hostPreserveWhitespaces ||
1033-
node.value.trim().length > 0) {
1034-
count++;
1013+
for (const branch of child.branches) {
1014+
if (this.shouldCheck(branch)) {
1015+
result.push(branch);
1016+
}
10351017
}
1036-
1037-
return count;
1038-
}, 0);
1039-
1040-
// Content projection can only be affected if there is more than one root node.
1041-
if (rootNodeCount > 1) {
1042-
result.push(eligibleNode);
10431018
}
10441019
}
10451020

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

Lines changed: 78 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1782,21 +1782,29 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
17821782
}] } });
17831783
export class MyApp {
17841784
constructor() {
1785-
this.expr = true;
1785+
this.expr = 0;
17861786
}
17871787
}
17881788
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
17891789
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
1790-
@if (expr) {
1790+
@if (expr === 0) {
17911791
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
1792+
} @else if (expr === 1) {
1793+
<div foo="4" bar="5" [binding]="6">{{expr}}</div>
1794+
} @else {
1795+
<div foo="7" bar="8" [binding]="9">{{expr}}</div>
17921796
}
17931797
`, isInline: true, dependencies: [{ kind: "directive", type: Binding, selector: "[binding]", inputs: ["binding"] }] });
17941798
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
17951799
type: Component,
17961800
args: [{
17971801
template: `
1798-
@if (expr) {
1802+
@if (expr === 0) {
17991803
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
1804+
} @else if (expr === 1) {
1805+
<div foo="4" bar="5" [binding]="6">{{expr}}</div>
1806+
} @else {
1807+
<div foo="7" bar="8" [binding]="9">{{expr}}</div>
18001808
}
18011809
`,
18021810
standalone: true,
@@ -1814,46 +1822,74 @@ export declare class Binding {
18141822
static ɵdir: i0.ɵɵDirectiveDeclaration<Binding, "[binding]", never, { "binding": { "alias": "binding"; "required": false; }; }, {}, never, never, true, never>;
18151823
}
18161824
export declare class MyApp {
1817-
expr: boolean;
1825+
expr: number;
18181826
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
18191827
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
18201828
}
18211829

18221830
/****************************************************************************************************
18231831
* PARTIAL FILE: if_template_root_node.js
18241832
****************************************************************************************************/
1825-
import { Component } from '@angular/core';
1833+
import { Component, Directive, Input } from '@angular/core';
18261834
import * as i0 from "@angular/core";
1835+
export class Binding {
1836+
constructor() {
1837+
this.binding = 0;
1838+
}
1839+
}
1840+
Binding.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, deps: [], target: i0.ɵɵFactoryTarget.Directive });
1841+
Binding.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: Binding, isStandalone: true, selector: "[binding]", inputs: { binding: "binding" }, ngImport: i0 });
1842+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, decorators: [{
1843+
type: Directive,
1844+
args: [{ standalone: true, selector: '[binding]' }]
1845+
}], propDecorators: { binding: [{
1846+
type: Input
1847+
}] } });
18271848
export class MyApp {
18281849
constructor() {
1829-
this.expr = true;
1850+
this.expr = 0;
18301851
}
18311852
}
18321853
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
1833-
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: `
1834-
@if (expr) {
1835-
<ng-template foo="1" bar="2">{{expr}}</ng-template>
1854+
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
1855+
@if (expr === 0) {
1856+
<ng-template foo="1" bar="2" [binding]="3">{{expr}}</ng-template>
1857+
} @else if (expr === 1) {
1858+
<ng-template foo="4" bar="5" [binding]="6">{{expr}}</ng-template>
1859+
} @else {
1860+
<ng-template foo="7" bar="8" [binding]="9">{{expr}}</ng-template>
18361861
}
1837-
`, isInline: true });
1862+
`, isInline: true, dependencies: [{ kind: "directive", type: Binding, selector: "[binding]", inputs: ["binding"] }] });
18381863
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
18391864
type: Component,
18401865
args: [{
18411866
template: `
1842-
@if (expr) {
1843-
<ng-template foo="1" bar="2">{{expr}}</ng-template>
1867+
@if (expr === 0) {
1868+
<ng-template foo="1" bar="2" [binding]="3">{{expr}}</ng-template>
1869+
} @else if (expr === 1) {
1870+
<ng-template foo="4" bar="5" [binding]="6">{{expr}}</ng-template>
1871+
} @else {
1872+
<ng-template foo="7" bar="8" [binding]="9">{{expr}}</ng-template>
18441873
}
18451874
`,
1875+
standalone: true,
1876+
imports: [Binding],
18461877
}]
18471878
}] });
18481879

18491880
/****************************************************************************************************
18501881
* PARTIAL FILE: if_template_root_node.d.ts
18511882
****************************************************************************************************/
18521883
import * as i0 from "@angular/core";
1884+
export declare class Binding {
1885+
binding: number;
1886+
static ɵfac: i0.ɵɵFactoryDeclaration<Binding, never>;
1887+
static ɵdir: i0.ɵɵDirectiveDeclaration<Binding, "[binding]", never, { "binding": { "alias": "binding"; "required": false; }; }, {}, never, never, true, never>;
1888+
}
18531889
export declare class MyApp {
1854-
expr: boolean;
1890+
expr: number;
18551891
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
1856-
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, false, never>;
1892+
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
18571893
}
18581894

18591895
/****************************************************************************************************
@@ -1920,42 +1956,62 @@ export declare class MyApp {
19201956
/****************************************************************************************************
19211957
* PARTIAL FILE: for_template_root_node.js
19221958
****************************************************************************************************/
1923-
import { Component } from '@angular/core';
1959+
import { Component, Directive, Input } from '@angular/core';
19241960
import * as i0 from "@angular/core";
1961+
export class Binding {
1962+
constructor() {
1963+
this.binding = 0;
1964+
}
1965+
}
1966+
Binding.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, deps: [], target: i0.ɵɵFactoryTarget.Directive });
1967+
Binding.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: Binding, isStandalone: true, selector: "[binding]", inputs: { binding: "binding" }, ngImport: i0 });
1968+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, decorators: [{
1969+
type: Directive,
1970+
args: [{ standalone: true, selector: '[binding]' }]
1971+
}], propDecorators: { binding: [{
1972+
type: Input
1973+
}] } });
19251974
export class MyApp {
19261975
constructor() {
19271976
this.items = [1, 2, 3];
19281977
}
19291978
}
19301979
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
1931-
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: `
1980+
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
19321981
@for (item of items; track item) {
1933-
<ng-template foo="1" bar="2">{{item}}</ng-template>
1982+
<ng-template foo="1" bar="2" [binding]="3">{{item}}</ng-template>
19341983
} @empty {
1935-
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
1984+
<ng-template empty-foo="1" empty-bar="2" [binding]="3">Empty!</ng-template>
19361985
}
1937-
`, isInline: true });
1986+
`, isInline: true, dependencies: [{ kind: "directive", type: Binding, selector: "[binding]", inputs: ["binding"] }] });
19381987
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
19391988
type: Component,
19401989
args: [{
19411990
template: `
19421991
@for (item of items; track item) {
1943-
<ng-template foo="1" bar="2">{{item}}</ng-template>
1992+
<ng-template foo="1" bar="2" [binding]="3">{{item}}</ng-template>
19441993
} @empty {
1945-
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
1994+
<ng-template empty-foo="1" empty-bar="2" [binding]="3">Empty!</ng-template>
19461995
}
19471996
`,
1997+
standalone: true,
1998+
imports: [Binding],
19481999
}]
19492000
}] });
19502001

19512002
/****************************************************************************************************
19522003
* PARTIAL FILE: for_template_root_node.d.ts
19532004
****************************************************************************************************/
19542005
import * as i0 from "@angular/core";
2006+
export declare class Binding {
2007+
binding: number;
2008+
static ɵfac: i0.ɵɵFactoryDeclaration<Binding, never>;
2009+
static ɵdir: i0.ɵɵDirectiveDeclaration<Binding, "[binding]", never, { "binding": { "alias": "binding"; "required": false; }; }, {}, never, never, true, never>;
2010+
}
19552011
export declare class MyApp {
19562012
items: number[];
19572013
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
1958-
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, false, never>;
2014+
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
19592015
}
19602016

19612017
/****************************************************************************************************

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
1-
import {Component} from '@angular/core';
1+
import {Component, Directive, Input} from '@angular/core';
2+
3+
@Directive({standalone: true, selector: '[binding]'})
4+
export class Binding {
5+
@Input() binding = 0;
6+
}
27

38
@Component({
49
template: `
510
@for (item of items; track item) {
6-
<ng-template foo="1" bar="2">{{item}}</ng-template>
11+
<ng-template foo="1" bar="2" [binding]="3">{{item}}</ng-template>
712
} @empty {
8-
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
13+
<ng-template empty-foo="1" empty-bar="2" [binding]="3">Empty!</ng-template>
914
}
1015
`,
16+
standalone: true,
17+
imports: [Binding],
1118
})
1219
export class MyApp {
1320
items = [1, 2, 3];
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
consts: [["foo", "1", "bar", "2"], ["empty-foo", "1", "empty-bar", "2"]]
1+
consts: [["foo", "1", "bar", "2", 3, "binding"], ["empty-foo", "1", "empty-bar", "2", 3, "binding"]]
22
3-
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 0, null, 0, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 1, 0, null, 1);
3+
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 1, null, 0, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 1, 1, null, 1);

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_element_root_node.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,17 @@ export class Binding {
77

88
@Component({
99
template: `
10-
@if (expr) {
10+
@if (expr === 0) {
1111
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
12+
} @else if (expr === 1) {
13+
<div foo="4" bar="5" [binding]="6">{{expr}}</div>
14+
} @else {
15+
<div foo="7" bar="8" [binding]="9">{{expr}}</div>
1216
}
1317
`,
1418
standalone: true,
1519
imports: [Binding],
1620
})
1721
export class MyApp {
18-
expr = true;
22+
expr = 0;
1923
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
consts: [["foo", "1", "bar", "2", 3, "binding"]]
1+
consts: [["foo", "1", "bar", "2", 3, "binding"], ["foo", "4", "bar", "5", 3, "binding"], ["foo", "7", "bar", "8", 3, "binding"]],
22
3-
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 2, 2, "div", 0);
3+
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 2, 2, "div", 0)(1, MyApp_Conditional_1_Template, 2, 2, "div", 1)(2, MyApp_Conditional_2_Template, 2, 2, "div", 2);
Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,24 @@
1-
import {Component} from '@angular/core';
1+
import {Component, Directive, Input} from '@angular/core';
2+
3+
@Directive({standalone: true, selector: '[binding]'})
4+
export class Binding {
5+
@Input() binding = 0;
6+
}
7+
28

39
@Component({
410
template: `
5-
@if (expr) {
6-
<ng-template foo="1" bar="2">{{expr}}</ng-template>
11+
@if (expr === 0) {
12+
<ng-template foo="1" bar="2" [binding]="3">{{expr}}</ng-template>
13+
} @else if (expr === 1) {
14+
<ng-template foo="4" bar="5" [binding]="6">{{expr}}</ng-template>
15+
} @else {
16+
<ng-template foo="7" bar="8" [binding]="9">{{expr}}</ng-template>
717
}
818
`,
19+
standalone: true,
20+
imports: [Binding],
921
})
1022
export class MyApp {
11-
expr = true;
23+
expr = 0;
1224
}
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", 3, "binding"], ["foo", "4", "bar", "5", 3, "binding"], ["foo", "7", "bar", "8", 3, "binding"]],
22
3-
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 1, 0, null, 0);
3+
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 1, 1, null, 0)(1, MyApp_Conditional_1_Template, 1, 1, null, 1)(2, MyApp_Conditional_2_Template, 1, 1, null, 2);

0 commit comments

Comments
 (0)