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
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.
  • Loading branch information
crisbeto committed Oct 31, 2023
1 parent c5980d6 commit f6e6783
Show file tree
Hide file tree
Showing 30 changed files with 883 additions and 35 deletions.
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>;
}

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"
}
]
}
]
}
}
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
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
@@ -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
@@ -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];
}
@@ -0,0 +1,3 @@
consts: [["foo", "1", "bar", "2"]]
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 2, 1, "div", 0, i0.ɵɵrepeaterTrackByIdentity);
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
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
@@ -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);
@@ -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);
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
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
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
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
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
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
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
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
@@ -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;
}
@@ -0,0 +1,3 @@
consts: [["foo", "1", "bar", "2"]]
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 2, 1, "div", 0);
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
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
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
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

0 comments on commit f6e6783

Please sign in to comment.