From 2afc40608d8545e431bd4705c139225f782cee2e Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 9 Feb 2019 16:04:15 +0100 Subject: [PATCH] fix(ivy): support injecting ChangeDetectorRef on templates (#27565) Previously, using a pipe in an input binding on an ng-template would evaluate the pipe in the context of node that was processed before the template. This caused the retrieval of e.g. ChangeDetectorRef to be incorrect, resulting in one of the following bugs depending on the template's structure: 1. If the template was at the root of a view, the previously processed node would be the component's host node outside of the current view. Accessing that node in the context of the current view results in a crash. 2. For templates not at the root, the ChangeDetectorRef injected into the pipe would correspond with the previously processed node. If that node hosts a component, the ChangeDetectorRef would not correspond with the view that the ng-template is part of. The solution to the above problem is two-fold: 1. Template compilation is adjusted such that the template instruction is emitted before any instructions produced by input bindings, such as pipes. This ensures that pipes are evaluated in the context of the template's container node. 2. A ChangeDetectorRef can be requested for container nodes. Fixes #28587 PR Close #27565 --- .../r3_view_compiler_template_spec.ts | 44 +++++++++++++++++++ .../compiler/src/render3/view/template.ts | 26 +++++------ .../src/render3/view_engine_compatibility.ts | 2 +- packages/core/test/acceptance/di_spec.ts | 42 ++++++++++++++++++ 4 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 packages/core/test/acceptance/di_spec.ts diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts index 75e2a2c000..5af1113bca 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts @@ -558,4 +558,48 @@ describe('compiler compliance: template', () => { expect(allListenerFunctionsNames.length).toBe(3); expect(allListenerFunctionsNames).toEqual(uniqueListenerFunctionNames); }); + + it('should support pipes in template bindings', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + selector: 'my-component', + template: \` +
\` + }) + export class MyComponent {} + + @NgModule({declarations: [MyComponent]}) + export class MyModule {} + ` + } + }; + + const template = ` + const $c0$ = [${AttributeMarker.SelectOnly}, "ngIf"]; + + function MyComponent_div_0_Template(rf, ctx) { + if (rf & 1) { + $i0$.ɵelement(0, "div"); + } + } + + // ... + + template: function MyComponent_Template(rf, ctx) { + if (rf & 1) { + $i0$.ɵtemplate(0, MyComponent_div_0_Template, 1, 0, "div", $c0$); + $i0$.ɵpipe(1, "pipe"); + } if (rf & 2) { + $i0$.ɵelementProperty(0, "ngIf", $i0$.ɵbind($i0$.ɵpipeBind1(1, 1, ctx.val))); + } + }`; + + const result = compile(files, angularFiles); + + expectEmit(result.source, template, 'Incorrect template'); + }); }); diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 8bb9c6aa99..cd0f9474ce 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -803,19 +803,6 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver parameters.push(o.importExpr(R3.templateRefExtractor)); } - // handle property bindings e.g. p(1, 'ngForOf', ɵbind(ctx.items)); - const context = o.variable(CONTEXT_NAME); - template.inputs.forEach(input => { - const value = input.value.visit(this._valueConverter); - this.allocateBindingSlots(value); - this.updateInstruction(template.sourceSpan, R3.elementProperty, () => { - return [ - o.literal(templateIndex), o.literal(input.name), - this.convertPropertyBinding(context, value) - ]; - }); - }); - // Create the template function const templateVisitor = new TemplateDefinitionBuilder( this.constantPool, this._bindingScope, this.level + 1, contextName, this.i18n, @@ -845,6 +832,19 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver return trimTrailingNulls(parameters); }); + // handle property bindings e.g. ɵelementProperty(1, 'ngForOf', ɵbind(ctx.items)); + const context = o.variable(CONTEXT_NAME); + template.inputs.forEach(input => { + const value = input.value.visit(this._valueConverter); + this.allocateBindingSlots(value); + this.updateInstruction(template.sourceSpan, R3.elementProperty, () => { + return [ + o.literal(templateIndex), o.literal(input.name), + this.convertPropertyBinding(context, value) + ]; + }); + }); + // Generate listeners for directive output template.outputs.forEach((outputAst: t.BoundEvent) => { this.creationInstruction( diff --git a/packages/core/src/render3/view_engine_compatibility.ts b/packages/core/src/render3/view_engine_compatibility.ts index 3f6fb55a9d..a2a5f604dc 100644 --- a/packages/core/src/render3/view_engine_compatibility.ts +++ b/packages/core/src/render3/view_engine_compatibility.ts @@ -345,7 +345,7 @@ export function createViewRef( const componentIndex = hostTNode.directiveStart; const componentView = getComponentViewByIndex(hostTNode.index, hostView); return new ViewRef(componentView, context, componentIndex); - } else if (hostTNode.type === TNodeType.Element) { + } else if (hostTNode.type === TNodeType.Element || hostTNode.type === TNodeType.Container) { const hostComponentView = findComponentView(hostView); return new ViewRef(hostComponentView, hostComponentView[CONTEXT], -1); } diff --git a/packages/core/test/acceptance/di_spec.ts b/packages/core/test/acceptance/di_spec.ts new file mode 100644 index 0000000000..7be5795430 --- /dev/null +++ b/packages/core/test/acceptance/di_spec.ts @@ -0,0 +1,42 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {CommonModule} from '@angular/common'; +import {ChangeDetectorRef, Component, Pipe, PipeTransform} from '@angular/core'; +import {ViewRef} from '@angular/core/src/render3/view_ref'; +import {TestBed} from '@angular/core/testing'; + +describe('di', () => { + describe('ChangeDetectorRef', () => { + it('should inject host component ChangeDetectorRef into directives on templates', () => { + let pipeInstance: MyPipe; + + @Pipe({name: 'pipe'}) + class MyPipe implements PipeTransform { + constructor(public cdr: ChangeDetectorRef) { pipeInstance = this; } + + transform(value: any): any { return value; } + } + + @Component({ + selector: 'my-app', + template: `
Visible
`, + }) + class MyApp { + showing = true; + + constructor(public cdr: ChangeDetectorRef) {} + } + + TestBed.configureTestingModule({declarations: [MyApp, MyPipe], imports: [CommonModule]}); + const fixture = TestBed.createComponent(MyApp); + fixture.detectChanges(); + expect((pipeInstance !.cdr as ViewRef).context).toBe(fixture.componentInstance); + }); + }); +});