Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

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

Browse source 
Fixes an edge case where a control flow node that has non-projectable nodes followed by an element node at the end would cause the entire control flow node to be project. For example if we have a projection target of `Main: <ng-content/> Slot: <ng-content select="[foo]"/>`, inserting a node of `@if (true) {Hello <span foo>world</span>}` would project the entire `Hello world` into the `[foo]` slot.

In the process of working on the issue, I also found that `@let` declarations at the root of the control flow node would prevent content projection as well.

PR Close #58607
This commit is contained in:
Kristiyan Kostadinov 2024-11-12 10:10:22 +01:00 committed by Jessica Janiuk
parent f2bda8e3fc
commit c421ffdbfb
12 changed files with 460 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

196
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/GOLDEN_PARTIAL.js
View file

@ -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
****************************************************************************************************/
@ -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
****************************************************************************************************/
@ -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
****************************************************************************************************/

45
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json
View file

@ -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"],
@ -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"],
@ -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"],

20
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_element_root_node_at_end.ts Normal file
View file

@ -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
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_element_root_node_at_end_template.js Normal file
View file

@ -0,0 +1 @@
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 3, 2, null, null, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 3, 1);

22
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_element_root_node_at_end.ts Normal file
View file

@ -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
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_element_root_node_at_end_template.js Normal file
View file

@ -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
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_element_root_node_at_end.ts Normal file
View file

@ -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
packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_element_root_node_at_end_template.js Normal file
View file

@ -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
packages/compiler/src/template/pipeline/src/ingest.ts
View file

@ -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.
*/
@ -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;
}
@ -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;
}
}

53
packages/core/test/acceptance/control_flow_for_spec.ts
View file

@ -1081,5 +1081,58 @@ describe('control flow - for', () => {
expect(directiveCount).toBe(1);
expect(fixture.nativeElement.textContent).toBe('Main: Before After Slot: 1');
});
it('should not project an @for that has text followed by one element node at the root', () => {
@Component({
selector: 'test',
template: 'Main: <ng-content/> Slot: <ng-content select="[foo]"/>',
})
class TestComponent {}
@Component({
imports: [TestComponent],
template: `
<test>
@for (item of items; track $index) {Hello <span foo>{{item}}</span>}
</test>
`,
})
class App {
items = [1];
}
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('Main: Hello 1 Slot: ');
});
it('should project an @for with a single root node and @let declarations into the root node slot', () => {
@Component({
standalone: true,
selector: 'test',
template: 'Main: <ng-content/> Slot: <ng-content select="[foo]"/>',
})
class TestComponent {}
@Component({
standalone: true,
imports: [TestComponent],
template: `
<test>Before @for (item of items; track $index) {
@let a = item + 1;
@let b = a + 1;
<span foo>{{b}}</span>
} After</test>
`,
})
class App {
items = [1];
}
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('Main: Before After Slot: 3');
});
});
});

49
packages/core/test/acceptance/control_flow_if_spec.ts
View file

@ -836,5 +836,54 @@ describe('control flow - if', () => {
expect(directiveCount).toBe(1);
expect(fixture.nativeElement.textContent).toBe('Main: Before After Slot: foo');
});
it('should not project an @if that has text followed by one element node at the root', () => {
@Component({
selector: 'test',
template: 'Main: <ng-content/> Slot: <ng-content select="[foo]"/>',
})
class TestComponent {}
@Component({
imports: [TestComponent],
template: `
<test>
@if (true) {Hello <span foo>world</span>}
</test>
`,
})
class App {}
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('Main: Hello world Slot: ');
});
it('should project an @if with a single root node and @let declarations into the root node slot', () => {
@Component({
standalone: true,
selector: 'test',
template: 'Main: <ng-content/> Slot: <ng-content select="[foo]"/>',
})
class TestComponent {}
@Component({
standalone: true,
imports: [TestComponent],
template: `
<test>Before @if (true) {
@let a = 1;
@let b = a + 1;
<span foo>{{b}}</span>
} After</test>
`,
})
class App {}
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('Main: Before After Slot: 2');
});
});
});

40
packages/platform-server/test/full_app_hydration_spec.ts
View file

@ -7828,6 +7828,46 @@ describe('platform-server full application hydration integration', () => {
verifyClientAndSSRContentsMatch(ssrContents, clientRootNode);
expect(clientRootNode.textContent).toContain('foo');
});
it('should handle let declaration inside a projected control flow node', async () => {
@Component({
selector: 'test',
template: 'Main: <ng-content/> Slot: <ng-content slot="foo"/>',
})
class TestComponent {}
@Component({
selector: 'app',
imports: [TestComponent],
template: `
<test>
@let a = 1;
@let b = a + 1;
<span foo>{{b}}</span>
</test>
`,
})
class SimpleComponent {}
const html = await ssr(SimpleComponent);
const ssrContents = getAppContents(html);
expect(ssrContents).toContain('<app ngh');
expect(ssrContents).toContain(
'Main: <!--ngtns--> Slot: <span foo="">2</span></test></app>',
);
resetTViewsFor(SimpleComponent);
const appRef = await prepareEnvironmentAndHydrate(doc, html, SimpleComponent);
const compRef = getComponentRef<SimpleComponent>(appRef);
appRef.tick();
const clientRootNode = compRef.location.nativeElement;
verifyAllNodesClaimedForHydration(clientRootNode);
verifyClientAndSSRContentsMatch(ssrContents, clientRootNode);
expect(clientRootNode.textContent).toContain('Main: Slot: 2');
});
});
describe('zoneless', () => {