fix(compiler): fix where pipes live

Impure pipes need to live on the view
that used them and need a new instance for
each call site.

Impure pipes need to live on the component view, cached across all child views,
and need a new pure proxy for each for
each call site that lives on the view
of the call site.

Fixes 

This bug was introduced not long ago by 152a117d5c27e56d1b32d69df2f69d34b94c0760
This commit is contained in:
Tobias Bosch 2016-05-03 08:59:25 -07:00
parent 52a6ba7ed9
commit dd6e0cf1b5
4 changed files with 71 additions and 36 deletions

@ -7,17 +7,35 @@ import {Identifiers, identifierToken} from '../identifiers';
import {injectFromViewParentInjector, createPureProxy, getPropertyInView} from './util';
class _PurePipeProxy {
constructor(public instance: o.ReadPropExpr, public argCount: number) {}
constructor(public view: CompileView, public instance: o.ReadPropExpr, public argCount: number) {}
}
export class CompilePipe {
meta: CompilePipeMetadata;
static call(view: CompileView, name: string, args: o.Expression[]): o.Expression {
var compView = view.componentView;
var meta = _findPipeMeta(compView, name);
var pipe: CompilePipe;
if (meta.pure) {
// pure pipes live on the component view
pipe = compView.purePipes.get(name);
if (isBlank(pipe)) {
pipe = new CompilePipe(compView, meta);
compView.purePipes.set(name, pipe);
compView.pipes.push(pipe);
}
} else {
// Non pure pipes live on the view that called it
pipe = new CompilePipe(view, meta);
view.pipes.push(pipe);
}
return pipe._call(view, args);
}
instance: o.ReadPropExpr;
private _purePipeProxies: _PurePipeProxy[] = [];
constructor(public view: CompileView, name: string) {
this.meta = _findPipeMeta(view, name);
this.instance = o.THIS_EXPR.prop(`_pipe_${name}_${view.pipeCount++}`);
constructor(public view: CompileView, public meta: CompilePipeMetadata) {
this.instance = o.THIS_EXPR.prop(`_pipe_${meta.name}_${view.pipeCount++}`);
}
get pure(): boolean { return this.meta.pure; }
@ -35,21 +53,26 @@ export class CompilePipe {
.set(o.importExpr(this.meta.type).instantiate(deps))
.toStmt());
this._purePipeProxies.forEach((purePipeProxy) => {
createPureProxy(
this.instance.prop('transform').callMethod(o.BuiltinMethod.bind, [this.instance]),
purePipeProxy.argCount, purePipeProxy.instance, this.view);
var pipeInstanceSeenFromPureProxy =
getPropertyInView(this.instance, purePipeProxy.view, this.view);
createPureProxy(pipeInstanceSeenFromPureProxy.prop('transform')
.callMethod(o.BuiltinMethod.bind, [pipeInstanceSeenFromPureProxy]),
purePipeProxy.argCount, purePipeProxy.instance, purePipeProxy.view);
});
}
call(callingView: CompileView, args: o.Expression[]): o.Expression {
private _call(callingView: CompileView, args: o.Expression[]): o.Expression {
if (this.meta.pure) {
// PurePipeProxies live on the view that called them.
var purePipeProxy = new _PurePipeProxy(
o.THIS_EXPR.prop(`${this.instance.name}_${this._purePipeProxies.length}`), args.length);
callingView, o.THIS_EXPR.prop(`${this.instance.name}_${this._purePipeProxies.length}`),
args.length);
this._purePipeProxies.push(purePipeProxy);
return getPropertyInView(
o.importExpr(Identifiers.castByValue)
.callFn([purePipeProxy.instance, this.instance.prop('transform')]),
callingView, this.view)
return o.importExpr(Identifiers.castByValue)
.callFn([
purePipeProxy.instance,
getPropertyInView(this.instance.prop('transform'), callingView, this.view)
])
.callFn(args);
} else {
return getPropertyInView(this.instance, callingView, this.view).callMethod('transform', args);
@ -57,7 +80,6 @@ export class CompilePipe {
}
}
function _findPipeMeta(view: CompileView, name: string): CompilePipeMetadata {
var pipeMeta: CompilePipeMetadata = null;
for (var i = view.pipeMetas.length - 1; i >= 0; i--) {

@ -127,16 +127,7 @@ export class CompileView implements NameResolver {
}
callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression {
var compView = this.componentView;
var pipe = compView.purePipes.get(name);
if (isBlank(pipe)) {
pipe = new CompilePipe(compView, name);
if (pipe.pure) {
compView.purePipes.set(name, pipe);
}
compView.pipes.push(pipe);
}
return pipe.call(this, [input].concat(args));
return CompilePipe.call(this, name, [input].concat(args));
}
getLocal(name: string): o.Expression {

@ -68,6 +68,7 @@ import {
AfterViewInit,
AfterViewChecked
} from '@angular/core';
import {NgFor} from '@angular/common';
import {By} from '@angular/platform-browser/src/dom/debug/by';
import {AsyncPipe} from '@angular/common';
@ -544,16 +545,23 @@ export function main() {
it('should call pure pipes that are used multiple times only when the arguments change',
fakeAsync(() => {
var ctx = createCompFixture(`<div [someProp]="name | countingPipe"></div><div [someProp]="age | countingPipe"></div>`, Person);
var ctx = createCompFixture(
`<div [someProp]="name | countingPipe"></div><div [someProp]="age | countingPipe"></div>` +
'<div *ngFor="let x of [1,2]" [someProp]="address.city | countingPipe"></div>',
Person);
ctx.componentInstance.name = 'a';
ctx.componentInstance.age = 10;
ctx.componentInstance.address = new Address('mtv');
ctx.detectChanges(false);
expect(renderLog.loggedValues).toEqual(['a state:0', '10 state:1']);
expect(renderLog.loggedValues)
.toEqual(['mtv state:0', 'mtv state:1', 'a state:2', '10 state:3']);
ctx.detectChanges(false);
expect(renderLog.loggedValues).toEqual(['a state:0', '10 state:1']);
expect(renderLog.loggedValues)
.toEqual(['mtv state:0', 'mtv state:1', 'a state:2', '10 state:3']);
ctx.componentInstance.age = 11;
ctx.detectChanges(false);
expect(renderLog.loggedValues).toEqual(['a state:0', '10 state:1', '11 state:2']);
expect(renderLog.loggedValues)
.toEqual(['mtv state:0', 'mtv state:1', 'a state:2', '10 state:3', '11 state:4']);
}));
it('should call impure pipes on each change detection run', fakeAsync(() => {
@ -1098,6 +1106,7 @@ const ALL_DIRECTIVES = /*@ts2dart_const*/[
forwardRef(() => OrderCheckDirective2),
forwardRef(() => OrderCheckDirective0),
forwardRef(() => OrderCheckDirective1),
NgFor
];
const ALL_PIPES = /*@ts2dart_const*/[

@ -37,7 +37,7 @@ import {
Host,
SkipSelfMetadata
} from '@angular/core';
import {NgIf} from '@angular/common';
import {NgIf, NgFor} from '@angular/common';
import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
const ALL_DIRECTIVES = /*@ts2dart_const*/[
@ -71,7 +71,8 @@ const ALL_DIRECTIVES = /*@ts2dart_const*/[
forwardRef(() => DirectiveNeedsChangeDetectorRef),
forwardRef(() => PushComponentNeedsChangeDetectorRef),
forwardRef(() => NeedsHostAppService),
NgIf
NgIf,
NgFor
];
const ALL_PIPES = /*@ts2dart_const*/[
@ -670,23 +671,35 @@ export function main() {
it('should cache pure pipes', fakeAsync(() => {
var el = createComp(
'<div [simpleDirective]="true | purePipe"></div><div *ngIf="true" [simpleDirective]="true | purePipe"></div>',
'<div [simpleDirective]="true | purePipe"></div><div [simpleDirective]="true | purePipe"></div>' +
'<div *ngFor="let x of [1,2]" [simpleDirective]="true | purePipe"></div>',
tcb);
var purePipe1 = el.children[0].inject(SimpleDirective).value;
var purePipe2 = el.children[1].inject(SimpleDirective).value;
var purePipe3 = el.children[2].inject(SimpleDirective).value;
var purePipe4 = el.children[3].inject(SimpleDirective).value;
expect(purePipe1).toBeAnInstanceOf(PurePipe);
expect(purePipe1).toBe(purePipe2);
expect(purePipe2).toBe(purePipe1);
expect(purePipe3).toBe(purePipe1);
expect(purePipe4).toBe(purePipe1);
}));
it('should not cache pure pipes', fakeAsync(() => {
it('should not cache impure pipes', fakeAsync(() => {
var el = createComp(
'<div [simpleDirective]="true | impurePipe"></div><div [simpleDirective]="true | impurePipe"></div>',
'<div [simpleDirective]="true | impurePipe"></div><div [simpleDirective]="true | impurePipe"></div>' +
'<div *ngFor="let x of [1,2]" [simpleDirective]="true | impurePipe"></div>',
tcb);
var purePipe1 = el.children[0].inject(SimpleDirective).value;
var purePipe2 = el.children[1].inject(SimpleDirective).value;
var purePipe3 = el.children[2].inject(SimpleDirective).value;
var purePipe4 = el.children[3].inject(SimpleDirective).value;
expect(purePipe1).toBeAnInstanceOf(ImpurePipe);
expect(purePipe2).toBeAnInstanceOf(ImpurePipe);
expect(purePipe1).not.toBe(purePipe2);
expect(purePipe2).not.toBe(purePipe1);
expect(purePipe3).toBeAnInstanceOf(ImpurePipe);
expect(purePipe3).not.toBe(purePipe1);
expect(purePipe4).toBeAnInstanceOf(ImpurePipe);
expect(purePipe4).not.toBe(purePipe1);
}));
});
});