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
This commit is contained in:
JoostK 2019-02-09 16:04:15 +01:00 committed by Miško Hevery
parent ac58d01a8e
commit 2afc40608d
4 changed files with 100 additions and 14 deletions

View File

@ -558,4 +558,48 @@ describe('compiler compliance: template', () => {
expect(allListenerFunctionsNames.length).toBe(3); expect(allListenerFunctionsNames.length).toBe(3);
expect(allListenerFunctionsNames).toEqual(uniqueListenerFunctionNames); 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: \`
<div *ngIf="val | pipe"></div>\`
})
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');
});
}); });

View File

@ -803,19 +803,6 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
parameters.push(o.importExpr(R3.templateRefExtractor)); 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 // Create the template function
const templateVisitor = new TemplateDefinitionBuilder( const templateVisitor = new TemplateDefinitionBuilder(
this.constantPool, this._bindingScope, this.level + 1, contextName, this.i18n, this.constantPool, this._bindingScope, this.level + 1, contextName, this.i18n,
@ -845,6 +832,19 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
return trimTrailingNulls(parameters); 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 // Generate listeners for directive output
template.outputs.forEach((outputAst: t.BoundEvent) => { template.outputs.forEach((outputAst: t.BoundEvent) => {
this.creationInstruction( this.creationInstruction(

View File

@ -345,7 +345,7 @@ export function createViewRef(
const componentIndex = hostTNode.directiveStart; const componentIndex = hostTNode.directiveStart;
const componentView = getComponentViewByIndex(hostTNode.index, hostView); const componentView = getComponentViewByIndex(hostTNode.index, hostView);
return new ViewRef(componentView, context, componentIndex); 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); const hostComponentView = findComponentView(hostView);
return new ViewRef(hostComponentView, hostComponentView[CONTEXT], -1); return new ViewRef(hostComponentView, hostComponentView[CONTEXT], -1);
} }

View File

@ -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: `<div *ngIf="showing | pipe">Visible</div>`,
})
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<MyApp>).context).toBe(fixture.componentInstance);
});
});
});