Skip to content

Commit

Permalink
fix(compiler): capture data bindings for content projection purposes …
Browse files Browse the repository at this point in the history
…in blocks (#54876)

Fixes a regression in the template pipeline where data bindings weren't being captured for content projection purposes.

Fixes #54872.

PR Close #54876
  • Loading branch information
crisbeto authored and alxhub committed Mar 15, 2024
1 parent 42318e7 commit c078820
Show file tree
Hide file tree
Showing 9 changed files with 177 additions and 33 deletions.
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
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
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);
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
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);
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
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,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;
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 @@ -394,7 +394,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 @@ -1256,18 +1256,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.
// 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
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
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

0 comments on commit c078820

Please sign in to comment.