Skip to content

Commit

Permalink
fix(compiler): project control flow root elements into correct slot (#…
Browse files Browse the repository at this point in the history
…52414)

With the directive-based control flow users were able to conditionally project content using the `*` syntax. E.g. `<div *ngIf="expr" projectMe></div>` will be projected into `<ng-content select="[projectMe]"/>`, because the attributes and tag name from the `div` are copied to the template via the template creation instruction. With `@if` and `@for` that is not the case, because the conditional is placed *around* elements, rather than *on* them. The result is that content projection won't work in the same way if a user converts from `*ngIf` to `@if`.

These changes aim to cover the most common case by doing the same copying when a control flow node has *one and only one* root element or template node.

This approach comes with some caveats:
1. As soon as any other node is added to the root, the copying behavior won't work anymore. A diagnostic will be added to flag cases like this and to explain how to work around it.
2. If `preserveWhitespaces` is enabled, it's very likely that indentation will break this workaround, because it'll include an additional text node as the first child. We can work around it here, but in a discussion it was decided not to, because the user explicitly opted into preserving the whitespace and we would have to drop it from the generated code. The diagnostic mentioned point #1 will flag such cases to users.

Fixes #52277.

PR Close #52414
  • Loading branch information
crisbeto authored and alxhub committed Oct 31, 2023
1 parent 696f003 commit 1f5039b
Show file tree
Hide file tree
Showing 30 changed files with 883 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1652,3 +1652,77 @@ export declare class MyApp {
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
}

/****************************************************************************************************
* PARTIAL FILE: if_element_root_node.js
****************************************************************************************************/
import { Component } from '@angular/core';
import * as i0 from "@angular/core";
export class MyApp {
constructor() {
this.expr = true;
}
}
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: `
@if (expr) {
<div foo="1" bar="2">{{expr}}</div>
}
`, isInline: true });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
type: Component,
args: [{
template: `
@if (expr) {
<div foo="1" bar="2">{{expr}}</div>
}
`,
}]
}] });

/****************************************************************************************************
* PARTIAL FILE: if_element_root_node.d.ts
****************************************************************************************************/
import * as i0 from "@angular/core";
export declare class MyApp {
expr: boolean;
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, false, never>;
}

/****************************************************************************************************
* PARTIAL FILE: for_element_root_node.js
****************************************************************************************************/
import { Component } from '@angular/core';
import * as i0 from "@angular/core";
export class MyApp {
constructor() {
this.items = [1, 2, 3];
}
}
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
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>
}
`, isInline: true });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
type: Component,
args: [{
template: `
@for (item of items; track item) {
<div foo="1" bar="2">{{item}}</div>
}
`,
}]
}] });

/****************************************************************************************************
* PARTIAL FILE: for_element_root_node.d.ts
****************************************************************************************************/
import * as i0 from "@angular/core";
export declare class MyApp {
items: number[];
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, false, never>;
}

Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,40 @@
"failureMessage": "Incorrect template"
}
]
},
{
"description": "should generate an if block with an element root node",
"inputFiles": [
"if_element_root_node.ts"
],
"expectations": [
{
"files": [
{
"expected": "if_element_root_node_template.js",
"generated": "if_element_root_node.js"
}
],
"failureMessage": "Incorrect template"
}
]
},
{
"description": "should generate a for block with an element root node",
"inputFiles": [
"for_element_root_node.ts"
],
"expectations": [
{
"files": [
{
"expected": "for_element_root_node_template.js",
"generated": "for_element_root_node.js"
}
],
"failureMessage": "Incorrect template"
}
]
}
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵtext(1);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, null, null, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵtext(1);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 6, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 6, null, null, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵtemplate(0, MyApp_ng_template_0_Template, 0, 0, "ng-template");
$r3$.ɵɵrepeaterCreate(1, MyApp_For_2_Template, 1, 1, $r3$.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_3_Template, 1, 0);
$r3$.ɵɵrepeaterCreate(1, MyApp_For_2_Template, 1, 1, null, null, $r3$.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_3_Template, 1, 0);
$r3$.ɵɵtemplate(4, MyApp_ng_template_4_Template, 0, 0, "ng-template");
}
if (rf & 2) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import {Component} from '@angular/core';

@Component({
template: `
@for (item of items; track item) {
<div foo="1" bar="2">{{item}}</div>
}
`,
})
export class MyApp {
items = [1, 2, 3];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
consts: [["foo", "1", "bar", "2"]]
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 2, 1, "div", 0, i0.ɵɵrepeaterTrackByIdentity);
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ function $_forTrack0$($index, $item) {
function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 1, $_forTrack0$, true);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, $_forTrack0$, true);
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 1, null, null, $_forTrack0$, true);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, null, null, $_forTrack0$, true);
}
if (rf & 2) {
$r3$.ɵɵrepeater(0, ctx.items);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ const $_forTrack0$ = ($index, $item) => $item.name[0].toUpperCase();
function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 1, $_forTrack0$);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, $_forTrack0$);
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 1, null, null, $_forTrack0$);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, null, null, $_forTrack0$);
}
if (rf & 2) {
$r3$.ɵɵrepeater(0, ctx.items);
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
$r3$.ɵɵrepeaterCreate(0, MyApp_ng_template_2_For_1_Template, 0, 0, $r3$.ɵɵcomponentInstance().trackFn);
$r3$.ɵɵrepeaterCreate(0, MyApp_ng_template_2_For_1_Template, 0, 0, null, null, $r3$.ɵɵcomponentInstance().trackFn);
Original file line number Diff line number Diff line change
@@ -1 +1 @@
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 0, 0, ctx.trackFn);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 0, 0, null, null, ctx.trackFn);
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵtext(1);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 0, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 0, "div", null, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function MyApp_For_2_Template(rf, ctx) {
function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵtext(0);
$r3$.ɵɵrepeaterCreate(1, MyApp_For_2_Template, 1, 4, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵrepeaterCreate(1, MyApp_For_2_Template, 1, 4, null, null, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵtext(3);
}
if (rf & 2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵtext(1);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 6, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 6, null, null, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵtext(1);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, $_forTrack0$);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, null, null, $_forTrack0$);
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵtext(1);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, $r3$.ɵɵrepeaterTrackByIndex);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, null, null, $r3$.ɵɵrepeaterTrackByIndex);
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ function $_forTrack0$($index, $item) {
function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 1, $_forTrack0$, true);
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 1, null, null, $_forTrack0$, true);
}
if (rf & 2) {
$r3$.ɵɵrepeater(0, ctx.items);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵtext(1);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, $r3$.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_4_Template, 1, 0);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, null, null, $r3$.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_4_Template, 1, 0);
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵtext(1);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 1, 1, null, null, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵpipe(4, "test");
$r3$.ɵɵelementEnd();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import {Component} from '@angular/core';

@Component({
template: `
@if (expr) {
<div foo="1" bar="2">{{expr}}</div>
}
`,
})
export class MyApp {
expr = true;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
consts: [["foo", "1", "bar", "2"]]
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 2, 1, "div", 0);
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function MyApp_Conditional_0_Conditional_1_Template(rf, ctx) {
return $r3$.ɵɵresetView($ctx_r10$.log($ctx_r10$.value(), $root_r1$, $inner_r3$));
});
$r3$.ɵɵelementEnd();
$r3$.ɵɵtemplate(1, MyApp_Conditional_0_Conditional_1_Conditional_1_Template, 1, 0);
$r3$.ɵɵtemplate(1, MyApp_Conditional_0_Conditional_1_Conditional_1_Template, 1, 0, "button");
}
if (rf & 2) {
const $ctx_r2$ = $r3$.ɵɵnextContext(2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function MyApp_Conditional_0_Conditional_1_Conditional_1_Template(rf, ctx) {
return $r3$.ɵɵresetView($ctx_r10$.log($ctx_r10$.value(), $root_r1$, $inner_r3$));
});
$r3$.ɵɵelementEnd();
$r3$.ɵɵtemplate(1, MyApp_Conditional_0_Conditional_1_Conditional_1_Template, 1, 0);
$r3$.ɵɵtemplate(1, MyApp_Conditional_0_Conditional_1_Conditional_1_Template, 1, 0, "button");
}
if (rf & 2) {
let $MyApp_Conditional_0_Conditional_1_contFlowTmp$;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function MyApp_For_3_For_2_Template(rf, ctx) {
function MyApp_For_3_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵtext(0);
$r3$.ɵɵrepeaterCreate(1, MyApp_For_3_For_2_Template, 1, 2, $r3$.ɵɵrepeaterTrackByIndex);
$r3$.ɵɵrepeaterCreate(1, MyApp_For_3_For_2_Template, 1, 2, null, null, $r3$.ɵɵrepeaterTrackByIndex);
}
if (rf & 2) {
const $item_r1$ = ctx.$implicit;
Expand All @@ -25,7 +25,7 @@ function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵtext(1);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 3, 1, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 3, 1, null, null, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function MyApp_For_3_For_2_Template(rf, ctx) {
function MyApp_For_3_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵtext(0);
$r3$.ɵɵrepeaterCreate(1, MyApp_For_3_For_2_Template, 1, 2, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵrepeaterCreate(1, MyApp_For_3_For_2_Template, 1, 2, null, null, $r3$.ɵɵrepeaterTrackByIdentity);
}
if (rf & 2) {
const $item_r1$ = ctx.$implicit;
Expand All @@ -25,7 +25,7 @@ function MyApp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵtext(1);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 3, 1, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵrepeaterCreate(2, MyApp_For_3_Template, 3, 1, null, null, $r3$.ɵɵrepeaterTrackByIdentity);
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
Expand Down
Loading

0 comments on commit 1f5039b

Please sign in to comment.