Skip to content

fix(compiler): capture data bindings for content projection purposes in blocks #54876

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): capture data bindings for content projection purposes in blocks #54876

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
68 changes: 54 additions & 14 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 @@ -1696,38 +1696,58 @@ export declare class MyApp {
/****************************************************************************************************
* PARTIAL FILE: if_element_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;
}
}
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: `
@if (expr) {
<div foo="1" bar="2">{{expr}}</div>
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
}
`, 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) {
<div foo="1" bar="2">{{expr}}</div>
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
}
`,
standalone: true,
imports: [Binding],
}]
}] });

/****************************************************************************************************
* PARTIAL FILE: if_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: boolean;
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 @@ -1770,42 +1790,62 @@ export declare class MyApp {
/****************************************************************************************************
* PARTIAL FILE: for_element_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) {
<div foo="1" bar="2">{{item}}</div>
<div foo="1" bar="2" [binding]="3">{{item}}</div>
} @empty {
<span empty-foo="1" empty-bar="2">Empty!</span>
<span empty-foo="1" empty-bar="2" [binding]="3">Empty!</span>
}
`, 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) {
<div foo="1" bar="2">{{item}}</div>
<div foo="1" bar="2" [binding]="3">{{item}}</div>
} @empty {
<span empty-foo="1" empty-bar="2">Empty!</span>
<span empty-foo="1" empty-bar="2" [binding]="3">Empty!</span>
}
`,
standalone: true,
imports: [Binding],
}]
}] });

/****************************************************************************************************
* PARTIAL FILE: for_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 {
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
13 changes: 10 additions & 3 deletions ...ler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/for_element_root_node.ts
View file
Open in desktop
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) {
<div foo="1" bar="2">{{item}}</div>
<div foo="1" bar="2" [binding]="3">{{item}}</div>
} @empty {
<span empty-foo="1" empty-bar="2">Empty!</span>
<span empty-foo="1" empty-bar="2" [binding]="3">Empty!</span>
}
`,
standalone: true,
imports: [Binding],
})
export class MyApp {
items = [1, 2, 3];
Expand Down
4 changes: 2 additions & 2 deletions ...est/compliance/test_cases/r3_view_compiler_control_flow/for_element_root_node_template.js
View file
Open in desktop
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, 2, 1, "div", 0, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 2, 0, "span", 1);
$r3$.ɵɵrepeaterCreate(0, MyApp_For_1_Template, 2, 2, "div", 0, i0.ɵɵrepeaterTrackByIdentity, false, MyApp_ForEmpty_2_Template, 2, 1, "span", 1);
11 changes: 9 additions & 2 deletions ...iler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_element_root_node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
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) {
<div foo="1" bar="2">{{expr}}</div>
<div foo="1" bar="2" [binding]="3">{{expr}}</div>
}
`,
standalone: true,
imports: [Binding],
})
export class MyApp {
expr = true;
Expand Down
4 changes: 2 additions & 2 deletions ...test/compliance/test_cases/r3_view_compiler_control_flow/if_element_root_node_template.js
View file
Open in desktop
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"]]
…
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 2, 1, "div", 0);
$r3$.ɵɵtemplate(0, MyApp_Conditional_0_Template, 2, 2, "div", 0);
4 changes: 2 additions & 2 deletions .../test_cases/r3_view_compiler_listener/two_way_binding_to_signal_loop_variable_template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
function TestCmp_For_1_Template(rf, ctx) {
if (rf & 1) {
const $_r1$ = $r3$.ɵɵgetCurrentView();
$r3$.ɵɵelementStart(0, "input", 0);
$r3$.ɵɵelementStart(0, "input", 1);
$r3$.ɵɵtwoWayListener("ngModelChange", function TestCmp_For_1_Template_input_ngModelChange_0_listener($event) {
const $name_r2$ = $r3$.ɵɵrestoreView($_r1$).$implicit;
$r3$.ɵɵtwoWayBindingSet($name_r2$, $event);
Expand All @@ -19,7 +19,7 @@ function TestCmp_For_1_Template(rf, ctx) {

function TestCmp_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵrepeaterCreate(0, TestCmp_For_1_Template, 1, 1, "input", null, $r3$.ɵɵrepeaterTrackByIndex);
$r3$.ɵɵrepeaterCreate(0, TestCmp_For_1_Template, 1, 1, "input", 0, $r3$.ɵɵrepeaterTrackByIndex);
}
if (rf & 2) {
$r3$.ɵɵrepeater(ctx.names);
Expand Down
22 changes: 16 additions & 6 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 @@ -359,7 +359,7 @@ function ingestIfBlock(unit: ViewCompilationUnit, ifBlock: t.IfBlock): void {
cView.contextVariables.set(ifCase.expressionAlias.name, ir.CTX_REF);
}

let ifCaseI18nMeta = undefined;
let ifCaseI18nMeta: i18n.BlockPlaceholder|undefined = undefined;

@crisbeto crisbeto Mar 15, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change, I just saw that vscode was flagging this as an error.

if (ifCase.i18n !== undefined) {
if (!(ifCase.i18n instanceof i18n.BlockPlaceholder)) {
throw Error(`Unhandled i18n metadata type for if block: ${ifCase.i18n?.constructor.name}`);
Expand Down Expand Up @@ -402,7 +402,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);
let switchCaseI18nMeta = undefined;
let switchCaseI18nMeta: i18n.BlockPlaceholder|undefined = undefined;
if (switchCase.i18n !== undefined) {
if (!(switchCase.i18n instanceof i18n.BlockPlaceholder)) {
throw Error(
Expand Down Expand Up @@ -1269,18 +1269,28 @@ function ingestControlFlowInsertionPoint(
}
}

// If we've found a single root node, its tag name and *static* attributes can be copied
// to the surrounding template to be used for content projection. Note that it's important
// that we don't copy any bound attributes since they don't participate in content projection
// and they can be used in directive matching (in the case of `Template.templateAttrs`).
// If we've found a single root node, its tag name and attributes can be
// copied to the surrounding template to be used for content projection.
if (root !== null) {
// Collect the static attributes for content projection purposes.
for (const attr of root.attributes) {
const securityContext = domSchema.securityContext(NG_TEMPLATE_TAG_NAME, attr.name, true);
unit.update.push(ir.createBindingOp(
xref, ir.BindingKind.Attribute, attr.name, o.literal(attr.value), null, securityContext,
true, false, null, asMessage(attr.i18n), attr.sourceSpan));
}

// Also collect the inputs since they participate in content projection as well.

@crisbeto crisbeto Mar 15, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, this is where the TDB was constructing the attributes array and this is the code that was capturing both the static attributes and the inputs.

// Note that TDB used to collect the outputs as well, but it wasn't passing them into
// the template instruction. Here we just don't collect them.
for (const attr of root.inputs) {
if (attr.type !== e.BindingType.Animation && attr.type !== e.BindingType.Attribute) {
const securityContext = domSchema.securityContext(NG_TEMPLATE_TAG_NAME, attr.name, true);
unit.create.push(ir.createExtractedAttributeOp(
xref, ir.BindingKind.Property, null, attr.name, null, null, null, securityContext));
}
}

const tagName = root instanceof t.Element ? root.name : root.tagName;

// Don't pass along `ng-template` tag name since it enables directive matching.
Expand Down
42 changes: 41 additions & 1 deletion packages/core/test/acceptance/control_flow_for_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@


import {NgIf} from '@angular/common';
import {ChangeDetectorRef, Component, Directive, inject, OnInit, Pipe, PipeTransform, TemplateRef, ViewContainerRef} from '@angular/core';
import {ChangeDetectorRef, Component, Directive, inject, Input, OnInit, Pipe, PipeTransform, TemplateRef, ViewContainerRef} from '@angular/core';
import {TestBed} from '@angular/core/testing';

describe('control flow - for', () => {
Expand Down Expand Up @@ -482,6 +482,46 @@ describe('control flow - for', () => {
expect(fixture.nativeElement.textContent).toBe('Main: Before one1two1one2two2 After Slot: ');
});

it('should project an @for with a single root node with a data binding', () => {
let directiveCount = 0;

@Directive({standalone: true, selector: '[foo]'})
class Foo {
@Input('foo') value: any;

constructor() {
directiveCount++;
}
}

@Component({
standalone: true,
selector: 'test',
template: 'Main: <ng-content/> Slot: <ng-content select="[foo]"/>',
})
class TestComponent {
}

@Component({
standalone: true,
imports: [TestComponent, Foo],
template: `
<test>Before @for (item of items; track $index) {
<span [foo]="item">{{item}}</span>
} After</test>
`
})
class App {
items = [1, 2, 3];
}

const fixture = TestBed.createComponent(App);
fixture.detectChanges();

expect(fixture.nativeElement.textContent).toBe('Main: Before After Slot: 123');
expect(directiveCount).toBe(3);
});

it('should project an @for with an ng-container root node', () => {
@Component({
standalone: true,
Expand Down
42 changes: 41 additions & 1 deletion packages/core/test/acceptance/control_flow_if_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@


import {NgFor} from '@angular/common';
import {ChangeDetectorRef, Component, Directive, inject, OnInit, Pipe, PipeTransform, TemplateRef, ViewContainerRef} from '@angular/core';
import {ChangeDetectorRef, Component, Directive, inject, Input, OnInit, Pipe, PipeTransform, TemplateRef, ViewContainerRef} from '@angular/core';
import {TestBed} from '@angular/core/testing';

// Basic shared pipe used during testing.
Expand Down Expand Up @@ -289,6 +289,46 @@ describe('control flow - if', () => {
expect(fixture.nativeElement.textContent).toBe('Main: Before After Slot: foo');
});

it('should project an @if with a single root node with a data binding', () => {
let directiveCount = 0;

@Directive({standalone: true, selector: '[foo]'})
class Foo {
@Input('foo') value: any;

constructor() {
directiveCount++;
}
}

@Component({
standalone: true,
selector: 'test',
template: 'Main: <ng-content/> Slot: <ng-content select="[foo]"/>',
})
class TestComponent {
}

@Component({
standalone: true,
imports: [TestComponent, Foo],
template: `
<test>Before @if (true) {
<span [foo]="value">foo</span>
} After</test>
`
})
class App {
value = 1;
}

const fixture = TestBed.createComponent(App);
fixture.detectChanges();

expect(fixture.nativeElement.textContent).toBe('Main: Before After Slot: foo');
expect(directiveCount).toBe(1);
});

it('should project an @if with multiple root nodes into the catch-all slot', () => {
@Component({
standalone: true,
Expand Down