fix(compiler): capture switch block cases for content projection (#54921)

Captures the individual cases in `switch` blocks for content projection purposes.

PR Close #54921
This commit is contained in:
Kristiyan Kostadinov
2024-03-18 11:46:53 +01:00
committed by Dylan Hunn
parent 7fc7f3f05f
commit a369f43fbd
11 changed files with 416 additions and 91 deletions
@@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {AbsoluteSourceSpan, BindingPipe, PropertyRead, PropertyWrite, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstForLoopBlock, TmplAstForLoopBlockEmpty, TmplAstHoverDeferredTrigger, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstReference, TmplAstTemplate, TmplAstVariable, TmplAstViewportDeferredTrigger} from '@angular/compiler';
import {AbsoluteSourceSpan, BindingPipe, PropertyRead, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstForLoopBlock, TmplAstForLoopBlockEmpty, TmplAstHoverDeferredTrigger, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstReference, TmplAstSwitchBlockCase, TmplAstTemplate, TmplAstVariable, TmplAstViewportDeferredTrigger} from '@angular/compiler';
import ts from 'typescript';
import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '../../diagnostics';
@@ -124,7 +124,8 @@ export interface OutOfBandDiagnosticRecorder {
controlFlowPreventingContentProjection(
templateId: TemplateId, category: ts.DiagnosticCategory,
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty,
controlFlowNode: TmplAstIfBlockBranch|TmplAstSwitchBlockCase|TmplAstForLoopBlock|
TmplAstForLoopBlockEmpty,
preservesWhitespaces: boolean): void;
}
@@ -393,7 +394,8 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
controlFlowPreventingContentProjection(
templateId: TemplateId, category: ts.DiagnosticCategory,
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty,
controlFlowNode: TmplAstIfBlockBranch|TmplAstSwitchBlockCase|TmplAstForLoopBlock|
TmplAstForLoopBlockEmpty,
preservesWhitespaces: boolean): void {
const blockName = controlFlowNode.nameSpan.toString().trim();
const lines = [
@@ -944,7 +944,7 @@ class TcbDomSchemaCheckerOp extends TcbOp {
* A `TcbOp` that finds and flags control flow nodes that interfere with content projection.
*
* Context:
* `@if` and `@for` try to emulate the content projection behavior of `*ngIf` and `*ngFor`
* Control flow blocks try to emulate the content projection behavior of `*ngIf` and `*ngFor`
* in order to reduce breakages when moving from one syntax to the other (see #52414), however the
* approach only works if there's only one element at the root of the control flow expression.
* This means that a stray sibling node (e.g. text) can prevent an element from being projected
@@ -999,7 +999,8 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
}
private findPotentialControlFlowNodes() {
const result: Array<TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstForLoopBlockEmpty> = [];
const result: Array<TmplAstIfBlockBranch|TmplAstSwitchBlockCase|TmplAstForLoopBlock|
TmplAstForLoopBlockEmpty> = [];
for (const child of this.element.children) {
if (child instanceof TmplAstForLoopBlock) {
@@ -1015,6 +1016,12 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
result.push(branch);
}
}
} else if (child instanceof TmplAstSwitchBlock) {
for (const current of child.cases) {
if (this.shouldCheck(current)) {
result.push(current);
}
}
}
}
@@ -2014,6 +2014,152 @@ export declare class MyApp {
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
}
/****************************************************************************************************
* PARTIAL FILE: switch_element_root_node.js
****************************************************************************************************/
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 = 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: `
@switch (expr) {
@case (0) {
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
}
@case (1) {
<div foo="4" bar="5" [binding]="6">{{expr}}</div>
}
@default {
<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: `
@switch (expr) {
@case (0) {
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
}
@case (1) {
<div foo="4" bar="5" [binding]="6">{{expr}}</div>
}
@default {
<div foo="7" bar="8" [binding]="9">{{expr}}</div>
}
}
`,
standalone: true,
imports: [Binding],
}]
}] });
/****************************************************************************************************
* PARTIAL FILE: switch_element_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: number;
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
}
/****************************************************************************************************
* PARTIAL FILE: switch_template_root_node.js
****************************************************************************************************/
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 = 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: `
@switch (expr) {
@case (0) {
<ng-template foo="1" bar="2" [binding]="3">{{expr}}</ng-template>
}
@case (1) {
<ng-template foo="4" bar="5" [binding]="6">{{expr}}</ng-template>
}
@default {
<ng-template foo="7" bar="8" [binding]="9">{{expr}}</ng-template>
}
}
`, 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: `
@switch (expr) {
@case (0) {
<ng-template foo="1" bar="2" [binding]="3">{{expr}}</ng-template>
}
@case (1) {
<ng-template foo="4" bar="5" [binding]="6">{{expr}}</ng-template>
}
@default {
<ng-template foo="7" bar="8" [binding]="9">{{expr}}</ng-template>
}
}
`,
standalone: true,
imports: [Binding],
}]
}] });
/****************************************************************************************************
* PARTIAL FILE: switch_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: number;
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
}
/****************************************************************************************************
* PARTIAL FILE: nested_for_computed_template_variables.js
****************************************************************************************************/
@@ -541,6 +541,36 @@
}
]
},
{
"description": "should generate a switch block with cases that have element root nodes",
"inputFiles": ["switch_element_root_node.ts"],
"expectations": [
{
"files": [
{
"expected": "switch_element_root_node_template.js",
"generated": "switch_element_root_node.js"
}
],
"failureMessage": "Incorrect template"
}
]
},
{
"description": "should generate a switch block with cases that have ng-template root nodes",
"inputFiles": ["switch_template_root_node.ts"],
"expectations": [
{
"files": [
{
"expected": "switch_template_root_node_template.js",
"generated": "switch_template_root_node.js"
}
],
"failureMessage": "Incorrect template"
}
]
},
{
"description": "should generate computed for loop variables that depend on shadowed $index and $count",
"inputFiles": ["nested_for_computed_template_variables.ts"],
@@ -0,0 +1,27 @@
import {Component, Directive, Input} from '@angular/core';
@Directive({standalone: true, selector: '[binding]'})
export class Binding {
@Input() binding = 0;
}
@Component({
template: `
@switch (expr) {
@case (0) {
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
}
@case (1) {
<div foo="4" bar="5" [binding]="6">{{expr}}</div>
}
@default {
<div foo="7" bar="8" [binding]="9">{{expr}}</div>
}
}
`,
standalone: true,
imports: [Binding],
})
export class MyApp {
expr = 0;
}
@@ -0,0 +1,3 @@
consts: [["foo", "1", "bar", "2", 3, "binding"], ["foo", "4", "bar", "5", 3, "binding"], ["foo", "7", "bar", "8", 3, "binding"]],
…
$r3$.ɵɵtemplate(0, MyApp_Case_0_Template, 2, 2, "div", 0)(1, MyApp_Case_1_Template, 2, 2, "div", 1)(2, MyApp_Case_2_Template, 2, 2, "div", 2);
@@ -0,0 +1,27 @@
import {Component, Directive, Input} from '@angular/core';
@Directive({standalone: true, selector: '[binding]'})
export class Binding {
@Input() binding = 0;
}
@Component({
template: `
@switch (expr) {
@case (0) {
<ng-template foo="1" bar="2" [binding]="3">{{expr}}</ng-template>
}
@case (1) {
<ng-template foo="4" bar="5" [binding]="6">{{expr}}</ng-template>
}
@default {
<ng-template foo="7" bar="8" [binding]="9">{{expr}}</ng-template>
}
}
`,
standalone: true,
imports: [Binding],
})
export class MyApp {
expr = 0;
}
@@ -0,0 +1,3 @@
consts: [["foo", "1", "bar", "2", 3, "binding"], ["foo", "4", "bar", "5", 3, "binding"], ["foo", "7", "bar", "8", 3, "binding"]],
…
$r3$.ɵɵtemplate(0, MyApp_Case_0_Template, 1, 1, null, 0)(1, MyApp_Case_1_Template, 1, 1, null, 1)(2, MyApp_Case_2_Template, 1, 1, null, 2);
@@ -5790,6 +5790,85 @@ suppress
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
it('should report when an @case block prevents an element from being projected', () => {
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'comp',
template: '<ng-content/> <ng-content select="bar, [foo]"/>',
standalone: true,
})
class Comp {}
@Component({
standalone: true,
imports: [Comp],
template: \`
<comp>
@switch (expr) {
@case (1) {
<div foo></div>
breaks projection
}
}
</comp>
\`,
})
class TestCmp {
expr = 1;
}
`);
const diags =
env.driveDiagnostics().map(d => ts.flattenDiagnosticMessageText(d.messageText, ''));
expect(diags.length).toBe(1);
expect(diags[0]).toContain(
`Node matches the "bar, [foo]" slot of the "Comp" component, but will ` +
`not be projected into the specific slot because the surrounding @case has more than one node at its root.`);
});
it('should report when an @default block prevents an element from being projected', () => {
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'comp',
template: '<ng-content select="[foo]"/> <ng-content select="[bar]"/>',
standalone: true,
})
class Comp {}
@Component({
standalone: true,
imports: [Comp],
template: \`
<comp>
@switch (expr) {
@case (1) {
<div foo></div>
}
@default {
<div bar></div>
breaks projection
}
}
</comp>
\`,
})
class TestCmp {
expr = 2;
}
`);
const diags =
env.driveDiagnostics().map(d => ts.flattenDiagnosticMessageText(d.messageText, ''));
expect(diags.length).toBe(1);
expect(diags[0]).toContain(
`Node matches the "[bar]" slot of the "Comp" component, but will ` +
`not be projected into the specific slot because the surrounding @default has more than one node at its root.`);
});
});
});
});
@@ -397,6 +397,7 @@ function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock
let conditions: Array<ir.ConditionalCaseExpr> = [];
for (const switchCase of switchBlock.cases) {
const cView = unit.job.allocateView(unit.xref);
const tagName = ingestControlFlowInsertionPoint(unit, cView.xref, switchCase);
let switchCaseI18nMeta: i18n.BlockPlaceholder|undefined = undefined;
if (switchCase.i18n !== undefined) {
if (!(switchCase.i18n instanceof i18n.BlockPlaceholder)) {
@@ -406,7 +407,7 @@ function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock
switchCaseI18nMeta = switchCase.i18n;
}
const templateOp = ir.createTemplateOp(
cView.xref, ir.TemplateKind.Block, null, 'Case', ir.Namespace.HTML, switchCaseI18nMeta,
cView.xref, ir.TemplateKind.Block, tagName, 'Case', ir.Namespace.HTML, switchCaseI18nMeta,
switchCase.startSourceSpan, switchCase.sourceSpan);
unit.create.push(templateOp);
if (firstXref === null) {
@@ -1259,7 +1260,7 @@ function convertSourceSpan(
*/
function ingestControlFlowInsertionPoint(
unit: ViewCompilationUnit, xref: ir.XrefId,
node: t.IfBlockBranch|t.ForLoopBlock|t.ForLoopBlockEmpty): string|null {
node: t.IfBlockBranch|t.SwitchBlockCase|t.ForLoopBlock|t.ForLoopBlockEmpty): string|null {
let root: t.Element|t.Template|null = null;
for (const child of node.children) {
@@ -136,97 +136,97 @@ describe('control flow - switch', () => {
expect(fixture.nativeElement.textContent).toBe('One');
});
xit('should project @switch cases into appropriate slots when selectors are used for all cases',
() => {
@Component({
standalone: true,
selector: 'test',
template:
'case 1: (<ng-content select="[case_1]"/>), case 2: (<ng-content select="[case_2]"/>), case 3: (<ng-content select="[case_3]"/>)',
})
class TestComponent {
}
it('should project @switch cases into appropriate slots when selectors are used for all cases',
() => {
@Component({
standalone: true,
selector: 'test',
template:
'case 1: (<ng-content select="[case_1]"/>), case 2: (<ng-content select="[case_2]"/>), case 3: (<ng-content select="[case_3]"/>)',
})
class TestComponent {
}
@Component({
standalone: true,
imports: [TestComponent],
template: `
<test>
@switch (value) {
@case (1) {
<span case_1>value 1</span>
}
@case (2) {
<span case_2>value 2</span>
}
@case (3) {
<span case_3>value 3</span>
}
}
</test>
`
})
class App {
value = 1;
}
@Component({
standalone: true,
imports: [TestComponent],
template: `
<test>
@switch (value) {
@case (1) {
<span case_1>value 1</span>
}
@case (2) {
<span case_2>value 2</span>
}
@case (3) {
<span case_3>value 3</span>
}
}
</test>
`
})
class App {
value = 1;
}
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (value 1), case 2: (), case 3: ()');
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (value 1), case 2: (), case 3: ()');
fixture.componentInstance.value = 2;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (), case 2: (value 2), case 3: ()');
fixture.componentInstance.value = 2;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (), case 2: (value 2), case 3: ()');
fixture.componentInstance.value = 3;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (), case 2: (), case 3: (value 3)');
});
fixture.componentInstance.value = 3;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (), case 2: (), case 3: (value 3)');
});
xit('should project @switch cases into appropriate slots when selectors are used for some cases',
() => {
@Component({
standalone: true,
selector: 'test',
template:
'case 1: (<ng-content select="[case_1]"/>), case 2: (<ng-content />), case 3: (<ng-content select="[case_3]"/>)',
})
class TestComponent {
}
it('should project @switch cases into appropriate slots when selectors are used for some cases',
() => {
@Component({
standalone: true,
selector: 'test',
template:
'case 1: (<ng-content select="[case_1]"/>), case 2: (<ng-content />), case 3: (<ng-content select="[case_3]"/>)',
})
class TestComponent {
}
@Component({
standalone: true,
imports: [TestComponent],
template: `
<test>
@switch (value) {
@case (1) {
<span case_1>value 1</span>
}
@case (2) {
<span>value 2</span>
}
@case (3) {
<span case_3>value 3</span>
}
}
</test>
`
})
class App {
value = 1;
}
@Component({
standalone: true,
imports: [TestComponent],
template: `
<test>
@switch (value) {
@case (1) {
<span case_1>value 1</span>
}
@case (2) {
<span>value 2</span>
}
@case (3) {
<span case_3>value 3</span>
}
}
</test>
`
})
class App {
value = 1;
}
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (value 1), case 2: (), case 3: ()');
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (value 1), case 2: (), case 3: ()');
fixture.componentInstance.value = 2;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (), case 2: (value 2), case 3: ()');
fixture.componentInstance.value = 2;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (), case 2: (value 2), case 3: ()');
fixture.componentInstance.value = 3;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (), case 2: (), case 3: (value 3)');
});
fixture.componentInstance.value = 3;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('case 1: (), case 2: (), case 3: (value 3)');
});
});