diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts index 531ad8eb0e8..d5d6ceb26a3 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts @@ -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 = [ diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index cb83a2e99c9..901b8935849 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -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 = []; + const result: Array = []; 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); + } + } } } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js index c6bf561e331..1f536ddae64 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js @@ -2014,6 +2014,152 @@ export declare class MyApp { static ɵcmp: i0.ɵɵComponentDeclaration; } +/**************************************************************************************************** + * 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) { +
{{expr}}
+ } + @case (1) { +
{{expr}}
+ } + @default { +
{{expr}}
+ } + } + `, 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) { +
{{expr}}
+ } + @case (1) { +
{{expr}}
+ } + @default { +
{{expr}}
+ } + } + `, + 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; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +export declare class MyApp { + expr: number; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + +/**************************************************************************************************** + * 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) { + {{expr}} + } + @case (1) { + {{expr}} + } + @default { + {{expr}} + } + } + `, 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) { + {{expr}} + } + @case (1) { + {{expr}} + } + @default { + {{expr}} + } + } + `, + 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; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +export declare class MyApp { + expr: number; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + /**************************************************************************************************** * PARTIAL FILE: nested_for_computed_template_variables.js ****************************************************************************************************/ diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json index 480b2b8fa54..c15f1764ffd 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json @@ -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"], diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_element_root_node.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_element_root_node.ts new file mode 100644 index 00000000000..f00cab77c67 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_element_root_node.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) { +
{{expr}}
+ } + @case (1) { +
{{expr}}
+ } + @default { +
{{expr}}
+ } + } + `, + standalone: true, + imports: [Binding], +}) +export class MyApp { + expr = 0; +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_element_root_node_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_element_root_node_template.js new file mode 100644 index 00000000000..51cdeb48def --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_element_root_node_template.js @@ -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); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_template_root_node.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_template_root_node.ts new file mode 100644 index 00000000000..ba6660ae343 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_template_root_node.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) { + {{expr}} + } + @case (1) { + {{expr}} + } + @default { + {{expr}} + } + } + `, + standalone: true, + imports: [Binding], +}) +export class MyApp { + expr = 0; +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_template_root_node_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_template_root_node_template.js new file mode 100644 index 00000000000..843ffc9a198 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_template_root_node_template.js @@ -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); diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 92ac0ea4967..6e38f24b991 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -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: ' ', + standalone: true, + }) + class Comp {} + + @Component({ + standalone: true, + imports: [Comp], + template: \` + + @switch (expr) { + @case (1) { +
+ breaks projection + } + } +
+ \`, + }) + 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: ' ', + standalone: true, + }) + class Comp {} + + @Component({ + standalone: true, + imports: [Comp], + template: \` + + @switch (expr) { + @case (1) { +
+ } + @default { +
+ breaks projection + } + } +
+ \`, + }) + 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.`); + }); }); }); }); diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index 5530c520cb9..65f8caaace6 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -397,6 +397,7 @@ function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock let conditions: Array = []; 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) { diff --git a/packages/core/test/acceptance/control_flow_switch_spec.ts b/packages/core/test/acceptance/control_flow_switch_spec.ts index 64c09e87d9f..d37dab18d36 100644 --- a/packages/core/test/acceptance/control_flow_switch_spec.ts +++ b/packages/core/test/acceptance/control_flow_switch_spec.ts @@ -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: (), case 2: (), 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: (), case 2: (), case 3: ()', + }) + class TestComponent { + } - @Component({ - standalone: true, - imports: [TestComponent], - template: ` - - @switch (value) { - @case (1) { - value 1 - } - @case (2) { - value 2 - } - @case (3) { - value 3 - } - } - - ` - }) - class App { - value = 1; - } + @Component({ + standalone: true, + imports: [TestComponent], + template: ` + + @switch (value) { + @case (1) { + value 1 + } + @case (2) { + value 2 + } + @case (3) { + value 3 + } + } + + ` + }) + 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: (), case 2: (), 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: (), case 2: (), case 3: ()', + }) + class TestComponent { + } - @Component({ - standalone: true, - imports: [TestComponent], - template: ` - - @switch (value) { - @case (1) { - value 1 - } - @case (2) { - value 2 - } - @case (3) { - value 3 - } - } - - ` - }) - class App { - value = 1; - } + @Component({ + standalone: true, + imports: [TestComponent], + template: ` + + @switch (value) { + @case (1) { + value 1 + } + @case (2) { + value 2 + } + @case (3) { + value 3 + } + } + + ` + }) + 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)'); + }); });