Skip to content

Commit

Permalink
fix(compiler): capture all control flow branches for content projecti…
Browse files Browse the repository at this point in the history
…on 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
  • Loading branch information
crisbeto authored and dylhunn committed Mar 22, 2024
1 parent 1c0ec56 commit 7fc7f3f
Show file tree
Hide file tree
Showing 12 changed files with 240 additions and 121 deletions.
10 changes: 1 addition & 9 deletions packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,15 +395,7 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty,
preservesWhitespaces: boolean): void {
let blockName: string;
if (controlFlowNode instanceof TmplAstForLoopBlockEmpty) {
blockName = '@empty';
} else if (controlFlowNode instanceof TmplAstForLoopBlock) {
blockName = '@for';
} else {
blockName = '@if';
}

const blockName = controlFlowNode.nameSpan.toString().trim();
const lines = [
`Node matches the "${slotSelector}" slot of the "${
componentName}" component, but will not be projected into the specific slot because the surrounding ${
Expand Down
33 changes: 4 additions & 29 deletions packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1002,9 +1002,6 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
const result: Array<TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty> = [];

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

// Only `@for` blocks and the first branch of `@if` blocks participate in content projection.
if (child instanceof TmplAstForLoopBlock) {
if (this.shouldCheck(child)) {
result.push(child);
Expand All @@ -1013,33 +1010,11 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
result.push(child.empty);
}
} else if (child instanceof TmplAstIfBlock) {
eligibleNode = child.branches[0]; // @if blocks are guaranteed to have at least one branch.
}

// Skip nodes with less than two children since it's impossible
// for them to run into the issue that we're checking for.
if (eligibleNode === null || eligibleNode.children.length < 2) {
continue;
}

// Count the number of root nodes while skipping empty text where relevant.
const rootNodeCount = eligibleNode.children.reduce((count, node) => {
// Normally `preserveWhitspaces` would have been accounted for during parsing, however
// in `ngtsc/annotations/component/src/resources.ts#parseExtractedTemplate` we enable
// `preserveWhitespaces` to preserve the accuracy of source maps diagnostics. This means
// that we have to account for it here since the presence of text nodes affects the
// content projection behavior.
if (!(node instanceof TmplAstText) || this.tcb.hostPreserveWhitespaces ||
node.value.trim().length > 0) {
count++;
for (const branch of child.branches) {
if (this.shouldCheck(branch)) {
result.push(branch);
}
}

return count;
}, 0);

// Content projection can only be affected if there is more than one root node.
if (rootNodeCount > 1) {
result.push(eligibleNode);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1782,21 +1782,29 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
}] } });
export class MyApp {
constructor() {
this.expr = true;
this.expr = 0;
}
}
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, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
@if (expr) {
@if (expr === 0) {
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
} @else if (expr === 1) {
<div foo="4" bar="5" [binding]="6">{{expr}}</div>
} @else {
<div foo="7" bar="8" [binding]="9">{{expr}}</div>
}
`, isInline: true, dependencies: [{ kind: "directive", type: Binding, selector: "[binding]", inputs: ["binding"] }] });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
type: Component,
args: [{
template: `
@if (expr) {
@if (expr === 0) {
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
} @else if (expr === 1) {
<div foo="4" bar="5" [binding]="6">{{expr}}</div>
} @else {
<div foo="7" bar="8" [binding]="9">{{expr}}</div>
}
`,
standalone: true,
Expand All @@ -1814,46 +1822,74 @@ export declare class Binding {
static ɵdir: i0.ɵɵDirectiveDeclaration<Binding, "[binding]", never, { "binding": { "alias": "binding"; "required": false; }; }, {}, never, never, true, never>;
}
export declare class MyApp {
expr: boolean;
expr: number;
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
}

/****************************************************************************************************
* PARTIAL FILE: if_template_root_node.js
****************************************************************************************************/
import { Component } from '@angular/core';
import { Component, Directive, Input } from '@angular/core';
import * as i0 from "@angular/core";
export class Binding {
constructor() {
this.binding = 0;
}
}
Binding.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, deps: [], target: i0.ɵɵFactoryTarget.Directive });
Binding.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: Binding, isStandalone: true, selector: "[binding]", inputs: { binding: "binding" }, ngImport: i0 });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, decorators: [{
type: Directive,
args: [{ standalone: true, selector: '[binding]' }]
}], propDecorators: { binding: [{
type: Input
}] } });
export class MyApp {
constructor() {
this.expr = true;
this.expr = 0;
}
}
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) {
<ng-template foo="1" bar="2">{{expr}}</ng-template>
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
@if (expr === 0) {
<ng-template foo="1" bar="2" [binding]="3">{{expr}}</ng-template>
} @else if (expr === 1) {
<ng-template foo="4" bar="5" [binding]="6">{{expr}}</ng-template>
} @else {
<ng-template foo="7" bar="8" [binding]="9">{{expr}}</ng-template>
}
`, isInline: true });
`, isInline: true, dependencies: [{ kind: "directive", type: Binding, selector: "[binding]", inputs: ["binding"] }] });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
type: Component,
args: [{
template: `
@if (expr) {
<ng-template foo="1" bar="2">{{expr}}</ng-template>
@if (expr === 0) {
<ng-template foo="1" bar="2" [binding]="3">{{expr}}</ng-template>
} @else if (expr === 1) {
<ng-template foo="4" bar="5" [binding]="6">{{expr}}</ng-template>
} @else {
<ng-template foo="7" bar="8" [binding]="9">{{expr}}</ng-template>
}
`,
standalone: true,
imports: [Binding],
}]
}] });

/****************************************************************************************************
* PARTIAL FILE: if_template_root_node.d.ts
****************************************************************************************************/
import * as i0 from "@angular/core";
export declare class Binding {
binding: number;
static ɵfac: i0.ɵɵFactoryDeclaration<Binding, never>;
static ɵdir: i0.ɵɵDirectiveDeclaration<Binding, "[binding]", never, { "binding": { "alias": "binding"; "required": false; }; }, {}, never, never, true, never>;
}
export declare class MyApp {
expr: boolean;
expr: number;
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, false, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
}

/****************************************************************************************************
Expand Down Expand Up @@ -1920,42 +1956,62 @@ export declare class MyApp {
/****************************************************************************************************
* PARTIAL FILE: for_template_root_node.js
****************************************************************************************************/
import { Component } from '@angular/core';
import { Component, Directive, Input } from '@angular/core';
import * as i0 from "@angular/core";
export class Binding {
constructor() {
this.binding = 0;
}
}
Binding.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, deps: [], target: i0.ɵɵFactoryTarget.Directive });
Binding.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: Binding, isStandalone: true, selector: "[binding]", inputs: { binding: "binding" }, ngImport: i0 });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Binding, decorators: [{
type: Directive,
args: [{ standalone: true, selector: '[binding]' }]
}], propDecorators: { binding: [{
type: Input
}] } });
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: `
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
@for (item of items; track item) {
<ng-template foo="1" bar="2">{{item}}</ng-template>
<ng-template foo="1" bar="2" [binding]="3">{{item}}</ng-template>
} @empty {
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
<ng-template empty-foo="1" empty-bar="2" [binding]="3">Empty!</ng-template>
}
`, isInline: true });
`, isInline: true, dependencies: [{ kind: "directive", type: Binding, selector: "[binding]", inputs: ["binding"] }] });
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) {
<ng-template foo="1" bar="2">{{item}}</ng-template>
<ng-template foo="1" bar="2" [binding]="3">{{item}}</ng-template>
} @empty {
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
<ng-template empty-foo="1" empty-bar="2" [binding]="3">Empty!</ng-template>
}
`,
standalone: true,
imports: [Binding],
}]
}] });

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

/****************************************************************************************************
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
import {Component} from '@angular/core';
import {Component, Directive, Input} from '@angular/core';

@Directive({standalone: true, selector: '[binding]'})
export class Binding {
@Input() binding = 0;
}

@Component({
template: `
@for (item of items; track item) {
<ng-template foo="1" bar="2">{{item}}</ng-template>
<ng-template foo="1" bar="2" [binding]="3">{{item}}</ng-template>
} @empty {
<ng-template empty-foo="1" empty-bar="2">Empty!</ng-template>
<ng-template empty-foo="1" empty-bar="2" [binding]="3">Empty!</ng-template>
}
`,
standalone: true,
imports: [Binding],
})
export class MyApp {
items = [1, 2, 3];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
consts: [["foo", "1", "bar", "2"], ["empty-foo", "1", "empty-bar", "2"]]
consts: [["foo", "1", "bar", "2", 3, "binding"], ["empty-foo", "1", "empty-bar", "2", 3, "binding"]]
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 0, null, 0, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 1, 0, null, 1);
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 1, 1, null, 0, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 1, 1, null, 1);
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ export class Binding {

@Component({
template: `
@if (expr) {
@if (expr === 0) {
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
} @else if (expr === 1) {
<div foo="4" bar="5" [binding]="6">{{expr}}</div>
} @else {
<div foo="7" bar="8" [binding]="9">{{expr}}</div>
}
`,
standalone: true,
imports: [Binding],
})
export class MyApp {
expr = true;
expr = 0;
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
consts: [["foo", "1", "bar", "2", 3, "binding"]]
consts: [["foo", "1", "bar", "2", 3, "binding"], ["foo", "4", "bar", "5", 3, "binding"], ["foo", "7", "bar", "8", 3, "binding"]],
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 2, 2, "div", 0);
$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);
Original file line number Diff line number Diff line change
@@ -1,12 +1,24 @@
import {Component} from '@angular/core';
import {Component, Directive, Input} from '@angular/core';

@Directive({standalone: true, selector: '[binding]'})
export class Binding {
@Input() binding = 0;
}


@Component({
template: `
@if (expr) {
<ng-template foo="1" bar="2">{{expr}}</ng-template>
@if (expr === 0) {
<ng-template foo="1" bar="2" [binding]="3">{{expr}}</ng-template>
} @else if (expr === 1) {
<ng-template foo="4" bar="5" [binding]="6">{{expr}}</ng-template>
} @else {
<ng-template foo="7" bar="8" [binding]="9">{{expr}}</ng-template>
}
`,
standalone: true,
imports: [Binding],
})
export class MyApp {
expr = true;
expr = 0;
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
consts: [["foo", "1", "bar", "2"]]
consts: [["foo", "1", "bar", "2", 3, "binding"], ["foo", "4", "bar", "5", 3, "binding"], ["foo", "7", "bar", "8", 3, "binding"]],
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 1, 0, null, 0);
$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);
Loading

0 comments on commit 7fc7f3f

Please sign in to comment.