Skip to content

fix(compiler): control flow nodes with root at the end projected incorrectly #58607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
crisbeto wants to merge 1 commit into from
Closed

fix(compiler): control flow nodes with root at the end projected incorrectly #58607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 196 additions & 0 deletions ...s/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1964,6 +1964,70 @@ export declare class MyApp {
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
}

/****************************************************************************************************
* PARTIAL FILE: if_element_root_node_at_end.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: [{ 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: `
@if (expr === 0) {
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
} @else if (expr === 1) {
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
} @else {
Hello <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 === 0) {
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
} @else if (expr === 1) {
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
} @else {
Hello <div foo="7" bar="8" [binding]="9">{{expr}}</div>
}
`,
imports: [Binding],
}]
}] });

/****************************************************************************************************
* PARTIAL FILE: if_element_root_node_at_end.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: for_element_root_node.js
****************************************************************************************************/
Expand Down Expand Up @@ -2084,6 +2148,66 @@ export declare class MyApp {
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
}

/****************************************************************************************************
* PARTIAL FILE: for_element_root_node_at_end.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: [{ 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, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
@for (item of items; track item) {
Hello <div foo="1" bar="2" [binding]="3">{{item}}</div>
} @empty {
Hello <span empty-foo="1" empty-bar="2" [binding]="3">Empty!</span>
}
`, 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) {
Hello <div foo="1" bar="2" [binding]="3">{{item}}</div>
} @empty {
Hello <span empty-foo="1" empty-bar="2" [binding]="3">Empty!</span>
}
`,
imports: [Binding],
}]
}] });

/****************************************************************************************************
* PARTIAL FILE: for_element_root_node_at_end.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, true, never>;
}

/****************************************************************************************************
* PARTIAL FILE: switch_element_root_node.js
****************************************************************************************************/
Expand Down Expand Up @@ -2156,6 +2280,78 @@ export declare class MyApp {
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, true, never>;
}

/****************************************************************************************************
* PARTIAL FILE: switch_element_root_node_at_end.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: [{ 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) {
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
}
@case (1) {
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
}
@default {
Hello <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) {
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
}
@case (1) {
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
}
@default {
Hello <div foo="7" bar="8" [binding]="9">{{expr}}</div>
}
}
`,
imports: [Binding],
}]
}] });

/****************************************************************************************************
* PARTIAL FILE: switch_element_root_node_at_end.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
****************************************************************************************************/
Expand Down
45 changes: 45 additions & 0 deletions ...ges/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,21 @@
}
]
},
{
"description": "should generate an if block with an element root node preceded by non-element node",
"inputFiles": ["if_element_root_node_at_end.ts"],
"expectations": [
{
"files": [
{
"expected": "if_element_root_node_at_end_template.js",
"generated": "if_element_root_node_at_end.js"
}
],
"failureMessage": "Incorrect template"
}
]
},
{
"description": "should generate a for block with an element root node",
"inputFiles": ["for_element_root_node.ts"],
Expand Down Expand Up @@ -556,6 +571,21 @@
}
]
},
{
"description": "should generate a for block with an element root node preceded by non-element node",
"inputFiles": ["for_element_root_node_at_end.ts"],
"expectations": [
{
"files": [
{
"expected": "for_element_root_node_at_end_template.js",
"generated": "for_element_root_node_at_end.js"
}
],
"failureMessage": "Incorrect template"
}
]
},
{
"description": "should generate a switch block with cases that have element root nodes",
"inputFiles": ["switch_element_root_node.ts"],
Expand All @@ -571,6 +601,21 @@
}
]
},
{
"description": "should generate a switch block with cases that have element root nodes preceded by non-element node",
"inputFiles": ["switch_element_root_node_at_end.ts"],
"expectations": [
{
"files": [
{
"expected": "switch_element_root_node_at_end_template.js",
"generated": "switch_element_root_node_at_end.js"
}
],
"failureMessage": "Incorrect template"
}
]
},
{
"description": "should generate a switch block with cases that have ng-template root nodes",
"inputFiles": ["switch_template_root_node.ts"],
Expand Down
20 changes: 20 additions & 0 deletions .../test/compliance/test_cases/r3_view_compiler_control_flow/for_element_root_node_at_end.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import {Component, Directive, Input} from '@angular/core';

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

@Component({
template: `
@for (item of items; track item) {
Hello <div foo="1" bar="2" [binding]="3">{{item}}</div>
} @empty {
Hello <span empty-foo="1" empty-bar="2" [binding]="3">Empty!</span>
}
`,
imports: [Binding],
})
export class MyApp {
items = [1, 2, 3];
}
1 change: 1 addition & 0 deletions ...pliance/test_cases/r3_view_compiler_control_flow/for_element_root_node_at_end_template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 3, 2, null, null, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 3, 1);
22 changes: 22 additions & 0 deletions ...i/test/compliance/test_cases/r3_view_compiler_control_flow/if_element_root_node_at_end.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import {Component, Directive, Input} from '@angular/core';

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

@Component({
template: `
@if (expr === 0) {
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
} @else if (expr === 1) {
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
} @else {
Hello <div foo="7" bar="8" [binding]="9">{{expr}}</div>
}
`,
imports: [Binding],
})
export class MyApp {
expr = 0;
}
1 change: 1 addition & 0 deletions ...mpliance/test_cases/r3_view_compiler_control_flow/if_element_root_node_at_end_template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 3, 2)(1, MyApp_Conditional_1_Template, 3, 2)(2, MyApp_Conditional_2_Template, 3, 2);
26 changes: 26 additions & 0 deletions ...st/compliance/test_cases/r3_view_compiler_control_flow/switch_element_root_node_at_end.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import {Component, Directive, Input} from '@angular/core';

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

@Component({
template: `
@switch (expr) {
@case (0) {
Hello <div foo="1" bar="2" [binding]="3">{{expr}}</div>
}
@case (1) {
Hello <div foo="4" bar="5" [binding]="6">{{expr}}</div>
}
@default {
Hello <div foo="7" bar="8" [binding]="9">{{expr}}</div>
}
}
`,
imports: [Binding],
})
export class MyApp {
expr = 0;
}
1 change: 1 addition & 0 deletions ...ance/test_cases/r3_view_compiler_control_flow/switch_element_root_node_at_end_template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
$r3$.ɵɵtemplate(0, MyApp_Case_0_Template, 3, 2)(1, MyApp_Case_1_Template, 3, 2)(2, MyApp_Case_2_Template, 3, 2);
9 changes: 6 additions & 3 deletions packages/compiler/src/template/pipeline/src/ingest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1734,7 +1734,7 @@ function convertSourceSpan(
* 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.
* The diagnostic mentioned point in #1 will flag such cases to users.
*
* @returns Tag name to be used for the control flow template.
*/
Expand All @@ -1746,8 +1746,9 @@ function ingestControlFlowInsertionPoint(
let root: t.Element | t.Template | null = null;

for (const child of node.children) {
// Skip over comment nodes.
if (child instanceof t.Comment) {
// Skip over comment nodes and @let declarations since
// it doesn't matter where they end up in the DOM.
if (child instanceof t.Comment || child instanceof t.LetDeclaration) {
continue;
}

Expand All @@ -1759,6 +1760,8 @@ function ingestControlFlowInsertionPoint(
// Root nodes can only elements or templates with a tag name (e.g. `<div *foo></div>`).
if (child instanceof t.Element || (child instanceof t.Template && child.tagName !== null)) {
root = child;
} else {
return null;
}
}

Expand Down
Loading