Skip to content

Commit 560282a

Browse files
crisbetothePunderWoman
crisbeto
authored and
thePunderWoman
committed
fix(compiler): control flow nodes with root at the end projected incorrectly (#58607)
Fixes an edge case where a control flow node that has non-projectable nodes followed by an element node at the end would cause the entire control flow node to be project. For example if we have a projection target of `Main: <ng-content/> Slot: <ng-content select="[foo]"/>`, inserting a node of `@if (true) {Hello <span foo>world</span>}` would project the entire `Hello world` into the `[foo]` slot. In the process of working on the issue, I also found that `@let` declarations at the root of the control flow node would prevent content projection as well. PR Close #58607
1 parent 51933ef commit 560282a

File tree

12 files changed

+460
-3
lines changed

12 files changed

+460
-3
lines changed

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

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1964,6 +1964,70 @@ export declare class MyApp {
19641964
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
19651965
}
19661966

1967+
/****************************************************************************************************
1968+
* PARTIAL FILE: if_element_root_node_at_end.js
1969+
****************************************************************************************************/
1970+
import { Component, Directive, Input } from '@angular/core';
1971+
import * as i0 from "@angular/core";
1972+
export class Binding {
1973+
constructor() {
1974+
this.binding = 0;
1975+
}
1976+
}
1977+
Binding.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, deps: [], target: i0.ɵɵFactoryTarget.Directive });
1978+
Binding.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: Binding, isStandalone: true, selector: "[binding]", inputs: { binding: "binding" }, ngImport: i0 });
1979+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, decorators: [{
1980+
type: Directive,
1981+
args: [{ selector: '[binding]' }]
1982+
}], propDecorators: { binding: [{
1983+
type: Input
1984+
}] } });
1985+
export class MyApp {
1986+
constructor() {
1987+
this.expr = 0;
1988+
}
1989+
}
1990+
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
1991+
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
1992+
@if (expr === 0) {
1993+
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
1994+
} @else if (expr === 1) {
1995+
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
1996+
} @else {
1997+
Hello <div foo="7" bar="8" [binding]="9">{{expr}}</div>
1998+
}
1999+
`, isInline: true, dependencies: [{ kind: "directive", type: Binding, selector: "[binding]", inputs: ["binding"] }] });
2000+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
2001+
type: Component,
2002+
args: [{
2003+
template: `
2004+
@if (expr === 0) {
2005+
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
2006+
} @else if (expr === 1) {
2007+
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
2008+
} @else {
2009+
Hello <div foo="7" bar="8" [binding]="9">{{expr}}</div>
2010+
}
2011+
`,
2012+
imports: [Binding],
2013+
}]
2014+
}] });
2015+
2016+
/****************************************************************************************************
2017+
* PARTIAL FILE: if_element_root_node_at_end.d.ts
2018+
****************************************************************************************************/
2019+
import * as i0 from "@angular/core";
2020+
export declare class Binding {
2021+
binding: number;
2022+
static ɵfac: i0.ɵɵFactoryDeclaration<Binding, never>;
2023+
static ɵdir: i0.ɵɵDirectiveDeclaration<Binding, "[binding]", never, { "binding": { "alias": "binding"; "required": false; }; }, {}, never, never, true, never>;
2024+
}
2025+
export declare class MyApp {
2026+
expr: number;
2027+
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
2028+
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
2029+
}
2030+
19672031
/****************************************************************************************************
19682032
* PARTIAL FILE: for_element_root_node.js
19692033
****************************************************************************************************/
@@ -2084,6 +2148,66 @@ export declare class MyApp {
20842148
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
20852149
}
20862150

2151+
/****************************************************************************************************
2152+
* PARTIAL FILE: for_element_root_node_at_end.js
2153+
****************************************************************************************************/
2154+
import { Component, Directive, Input } from '@angular/core';
2155+
import * as i0 from "@angular/core";
2156+
export class Binding {
2157+
constructor() {
2158+
this.binding = 0;
2159+
}
2160+
}
2161+
Binding.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, deps: [], target: i0.ɵɵFactoryTarget.Directive });
2162+
Binding.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: Binding, isStandalone: true, selector: "[binding]", inputs: { binding: "binding" }, ngImport: i0 });
2163+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, decorators: [{
2164+
type: Directive,
2165+
args: [{ selector: '[binding]' }]
2166+
}], propDecorators: { binding: [{
2167+
type: Input
2168+
}] } });
2169+
export class MyApp {
2170+
constructor() {
2171+
this.items = [1, 2, 3];
2172+
}
2173+
}
2174+
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
2175+
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
2176+
@for (item of items; track item) {
2177+
Hello <div foo="1" bar="2" [binding]="3">{{item}}</div>
2178+
} @empty {
2179+
Hello <span empty-foo="1" empty-bar="2" [binding]="3">Empty!</span>
2180+
}
2181+
`, isInline: true, dependencies: [{ kind: "directive", type: Binding, selector: "[binding]", inputs: ["binding"] }] });
2182+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
2183+
type: Component,
2184+
args: [{
2185+
template: `
2186+
@for (item of items; track item) {
2187+
Hello <div foo="1" bar="2" [binding]="3">{{item}}</div>
2188+
} @empty {
2189+
Hello <span empty-foo="1" empty-bar="2" [binding]="3">Empty!</span>
2190+
}
2191+
`,
2192+
imports: [Binding],
2193+
}]
2194+
}] });
2195+
2196+
/****************************************************************************************************
2197+
* PARTIAL FILE: for_element_root_node_at_end.d.ts
2198+
****************************************************************************************************/
2199+
import * as i0 from "@angular/core";
2200+
export declare class Binding {
2201+
binding: number;
2202+
static ɵfac: i0.ɵɵFactoryDeclaration<Binding, never>;
2203+
static ɵdir: i0.ɵɵDirectiveDeclaration<Binding, "[binding]", never, { "binding": { "alias": "binding"; "required": false; }; }, {}, never, never, true, never>;
2204+
}
2205+
export declare class MyApp {
2206+
items: number[];
2207+
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
2208+
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
2209+
}
2210+
20872211
/****************************************************************************************************
20882212
* PARTIAL FILE: switch_element_root_node.js
20892213
****************************************************************************************************/
@@ -2156,6 +2280,78 @@ export declare class MyApp {
21562280
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
21572281
}
21582282

2283+
/****************************************************************************************************
2284+
* PARTIAL FILE: switch_element_root_node_at_end.js
2285+
****************************************************************************************************/
2286+
import { Component, Directive, Input } from '@angular/core';
2287+
import * as i0 from "@angular/core";
2288+
export class Binding {
2289+
constructor() {
2290+
this.binding = 0;
2291+
}
2292+
}
2293+
Binding.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, deps: [], target: i0.ɵɵFactoryTarget.Directive });
2294+
Binding.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: Binding, isStandalone: true, selector: "[binding]", inputs: { binding: "binding" }, ngImport: i0 });
2295+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, decorators: [{
2296+
type: Directive,
2297+
args: [{ selector: '[binding]' }]
2298+
}], propDecorators: { binding: [{
2299+
type: Input
2300+
}] } });
2301+
export class MyApp {
2302+
constructor() {
2303+
this.expr = 0;
2304+
}
2305+
}
2306+
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
2307+
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
2308+
@switch (expr) {
2309+
@case (0) {
2310+
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
2311+
}
2312+
@case (1) {
2313+
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
2314+
}
2315+
@default {
2316+
Hello <div foo="7" bar="8" [binding]="9">{{expr}}</div>
2317+
}
2318+
}
2319+
`, isInline: true, dependencies: [{ kind: "directive", type: Binding, selector: "[binding]", inputs: ["binding"] }] });
2320+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
2321+
type: Component,
2322+
args: [{
2323+
template: `
2324+
@switch (expr) {
2325+
@case (0) {
2326+
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
2327+
}
2328+
@case (1) {
2329+
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
2330+
}
2331+
@default {
2332+
Hello <div foo="7" bar="8" [binding]="9">{{expr}}</div>
2333+
}
2334+
}
2335+
`,
2336+
imports: [Binding],
2337+
}]
2338+
}] });
2339+
2340+
/****************************************************************************************************
2341+
* PARTIAL FILE: switch_element_root_node_at_end.d.ts
2342+
****************************************************************************************************/
2343+
import * as i0 from "@angular/core";
2344+
export declare class Binding {
2345+
binding: number;
2346+
static ɵfac: i0.ɵɵFactoryDeclaration<Binding, never>;
2347+
static ɵdir: i0.ɵɵDirectiveDeclaration<Binding, "[binding]", never, { "binding": { "alias": "binding"; "required": false; }; }, {}, never, never, true, never>;
2348+
}
2349+
export declare class MyApp {
2350+
expr: number;
2351+
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
2352+
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
2353+
}
2354+
21592355
/****************************************************************************************************
21602356
* PARTIAL FILE: switch_template_root_node.js
21612357
****************************************************************************************************/

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,21 @@
526526
}
527527
]
528528
},
529+
{
530+
"description": "should generate an if block with an element root node preceded by non-element node",
531+
"inputFiles": ["if_element_root_node_at_end.ts"],
532+
"expectations": [
533+
{
534+
"files": [
535+
{
536+
"expected": "if_element_root_node_at_end_template.js",
537+
"generated": "if_element_root_node_at_end.js"
538+
}
539+
],
540+
"failureMessage": "Incorrect template"
541+
}
542+
]
543+
},
529544
{
530545
"description": "should generate a for block with an element root node",
531546
"inputFiles": ["for_element_root_node.ts"],
@@ -556,6 +571,21 @@
556571
}
557572
]
558573
},
574+
{
575+
"description": "should generate a for block with an element root node preceded by non-element node",
576+
"inputFiles": ["for_element_root_node_at_end.ts"],
577+
"expectations": [
578+
{
579+
"files": [
580+
{
581+
"expected": "for_element_root_node_at_end_template.js",
582+
"generated": "for_element_root_node_at_end.js"
583+
}
584+
],
585+
"failureMessage": "Incorrect template"
586+
}
587+
]
588+
},
559589
{
560590
"description": "should generate a switch block with cases that have element root nodes",
561591
"inputFiles": ["switch_element_root_node.ts"],
@@ -571,6 +601,21 @@
571601
}
572602
]
573603
},
604+
{
605+
"description": "should generate a switch block with cases that have element root nodes preceded by non-element node",
606+
"inputFiles": ["switch_element_root_node_at_end.ts"],
607+
"expectations": [
608+
{
609+
"files": [
610+
{
611+
"expected": "switch_element_root_node_at_end_template.js",
612+
"generated": "switch_element_root_node_at_end.js"
613+
}
614+
],
615+
"failureMessage": "Incorrect template"
616+
}
617+
]
618+
},
574619
{
575620
"description": "should generate a switch block with cases that have ng-template root nodes",
576621
"inputFiles": ["switch_template_root_node.ts"],

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import {Component, Directive, Input} from '@angular/core';
2+
3+
@Directive({selector: '[binding]'})
4+
export class Binding {
5+
@Input() binding = 0;
6+
}
7+
8+
@Component({
9+
template: `
10+
@for (item of items; track item) {
11+
Hello <div foo="1" bar="2" [binding]="3">{{item}}</div>
12+
} @empty {
13+
Hello <span empty-foo="1" empty-bar="2" [binding]="3">Empty!</span>
14+
}
15+
`,
16+
imports: [Binding],
17+
})
18+
export class MyApp {
19+
items = [1, 2, 3];
20+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 3, 2, null, null, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 3, 1);

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import {Component, Directive, Input} from '@angular/core';
2+
3+
@Directive({selector: '[binding]'})
4+
export class Binding {
5+
@Input() binding = 0;
6+
}
7+
8+
@Component({
9+
template: `
10+
@if (expr === 0) {
11+
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
12+
} @else if (expr === 1) {
13+
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
14+
} @else {
15+
Hello <div foo="7" bar="8" [binding]="9">{{expr}}</div>
16+
}
17+
`,
18+
imports: [Binding],
19+
})
20+
export class MyApp {
21+
expr = 0;
22+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 3, 2)(1, MyApp_Conditional_1_Template, 3, 2)(2, MyApp_Conditional_2_Template, 3, 2);

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import {Component, Directive, Input} from '@angular/core';
2+
3+
@Directive({selector: '[binding]'})
4+
export class Binding {
5+
@Input() binding = 0;
6+
}
7+
8+
@Component({
9+
template: `
10+
@switch (expr) {
11+
@case (0) {
12+
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
13+
}
14+
@case (1) {
15+
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
16+
}
17+
@default {
18+
Hello <div foo="7" bar="8" [binding]="9">{{expr}}</div>
19+
}
20+
}
21+
`,
22+
imports: [Binding],
23+
})
24+
export class MyApp {
25+
expr = 0;
26+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
$r3$.ɵɵtemplate(0, MyApp_Case_0_Template, 3, 2)(1, MyApp_Case_1_Template, 3, 2)(2, MyApp_Case_2_Template, 3, 2);

‎packages/compiler/src/template/pipeline/src/ingest.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1734,7 +1734,7 @@ function convertSourceSpan(
17341734
* workaround, because it'll include an additional text node as the first child. We can work
17351735
* around it here, but in a discussion it was decided not to, because the user explicitly opted
17361736
* into preserving the whitespace and we would have to drop it from the generated code.
1737-
* The diagnostic mentioned point #1 will flag such cases to users.
1737+
* The diagnostic mentioned point in #1 will flag such cases to users.
17381738
*
17391739
* @returns Tag name to be used for the control flow template.
17401740
*/
@@ -1746,8 +1746,9 @@ function ingestControlFlowInsertionPoint(
17461746
let root: t.Element | t.Template | null = null;
17471747

17481748
for (const child of node.children) {
1749-
// Skip over comment nodes.
1750-
if (child instanceof t.Comment) {
1749+
// Skip over comment nodes and @let declarations since
1750+
// it doesn't matter where they end up in the DOM.
1751+
if (child instanceof t.Comment || child instanceof t.LetDeclaration) {
17511752
continue;
17521753
}
17531754

@@ -1759,6 +1760,8 @@ function ingestControlFlowInsertionPoint(
17591760
// Root nodes can only elements or templates with a tag name (e.g. `<div *foo></div>`).
17601761
if (child instanceof t.Element || (child instanceof t.Template && child.tagName !== null)) {
17611762
root = child;
1763+
} else {
1764+
return null;
17621765
}
17631766
}
17641767

0 commit comments

Comments
 (0)