diff --git a/modules/angular2/src/compiler/view_compiler/compile_view.ts b/modules/angular2/src/compiler/view_compiler/compile_view.ts index 41fad43044..1c1bf99ed2 100644 --- a/modules/angular2/src/compiler/view_compiler/compile_view.ts +++ b/modules/angular2/src/compiler/view_compiler/compile_view.ts @@ -124,7 +124,7 @@ export class CompileView implements NameResolver { } } - createPipe(name: string): o.Expression { + callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression { var pipeMeta: CompilePipeMetadata = null; for (var i = this.pipeMetas.length - 1; i >= 0; i--) { var localPipeMeta = this.pipeMetas[i]; @@ -139,6 +139,7 @@ export class CompileView implements NameResolver { } var pipeFieldName = pipeMeta.pure ? `_pipe_${name}` : `_pipe_${name}_${this.pipes.size}`; var pipeExpr = this.pipes.get(pipeFieldName); + var pipeFieldCacheProp = o.THIS_EXPR.prop(`${pipeFieldName}_cache`); if (isBlank(pipeExpr)) { var deps = pipeMeta.type.diDeps.map((diDep) => { if (diDep.token.equalsTo(identifierToken(Identifiers.ChangeDetectorRef))) { @@ -148,6 +149,12 @@ export class CompileView implements NameResolver { }); this.fields.push( new o.ClassField(pipeFieldName, o.importType(pipeMeta.type), [o.StmtModifier.Private])); + if (pipeMeta.pure) { + this.fields.push(new o.ClassField(pipeFieldCacheProp.name, null, [o.StmtModifier.Private])); + this.createMethod.addStmt(o.THIS_EXPR.prop(pipeFieldCacheProp.name) + .set(o.importExpr(Identifiers.uninitialized)) + .toStmt()); + } this.createMethod.resetDebugInfo(null, null); this.createMethod.addStmt(o.THIS_EXPR.prop(pipeFieldName) .set(o.importExpr(pipeMeta.type).instantiate(deps)) @@ -156,7 +163,15 @@ export class CompileView implements NameResolver { this.pipes.set(pipeFieldName, pipeExpr); bindPipeDestroyLifecycleCallbacks(pipeMeta, pipeExpr, this); } - return pipeExpr; + var callPipeExpr: o.Expression = pipeExpr.callMethod('transform', [input, o.literalArr(args)]); + if (pipeMeta.pure) { + callPipeExpr = + o.THIS_EXPR.callMethod( + 'checkPurePipe', + [o.literal(this.literalArrayCount++), o.literalArr([input].concat(args))]) + .conditional(pipeFieldCacheProp.set(callPipeExpr), pipeFieldCacheProp); + } + return callPipeExpr; } getVariable(name: string): o.Expression { diff --git a/modules/angular2/src/compiler/view_compiler/expression_converter.ts b/modules/angular2/src/compiler/view_compiler/expression_converter.ts index c50f19595b..b3d512cc5f 100644 --- a/modules/angular2/src/compiler/view_compiler/expression_converter.ts +++ b/modules/angular2/src/compiler/view_compiler/expression_converter.ts @@ -8,7 +8,7 @@ import {isBlank, isPresent, isArray, CONST_EXPR} from 'angular2/src/facade/lang' var IMPLICIT_RECEIVER = o.variable('#implicit'); export interface NameResolver { - createPipe(name: string): o.Expression; + callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression; getVariable(name: string): o.Expression; createLiteralArray(values: o.Expression[]): o.Expression; createLiteralMap(values: Array>): o.Expression; @@ -132,13 +132,12 @@ class _AstToIrVisitor implements cdAst.AstVisitor { ast.falseExp.visit(this, _Mode.Expression))); } visitPipe(ast: cdAst.BindingPipe, mode: _Mode): any { - var pipeInstance = this._nameResolver.createPipe(ast.name); var input = ast.exp.visit(this, _Mode.Expression); var args = this.visitAll(ast.args, _Mode.Expression); + var pipeResult = this._nameResolver.callPipe(ast.name, input, args); this.needsValueUnwrapper = true; - return convertToStatementIfNeeded( - mode, this._valueUnwrapper.callMethod( - 'unwrap', [pipeInstance.callMethod('transform', [input, o.literalArr(args)])])); + return convertToStatementIfNeeded(mode, + this._valueUnwrapper.callMethod('unwrap', [pipeResult])); } visitFunctionCall(ast: cdAst.FunctionCall, mode: _Mode): any { return convertToStatementIfNeeded(mode, ast.target.visit(this, _Mode.Expression) diff --git a/modules/angular2/src/core/linker/view.ts b/modules/angular2/src/core/linker/view.ts index dd1001f531..1f97473c2e 100644 --- a/modules/angular2/src/core/linker/view.ts +++ b/modules/angular2/src/core/linker/view.ts @@ -360,11 +360,23 @@ export abstract class AppView { this.viewContainerElement = null; } + checkPurePipe(id: number, newArgs: any[]): boolean { + var prevArgs = this._literalArrayCache[id]; + var newPresent = isPresent(newArgs); + var prevPresent = isPresent(prevArgs); + if (newPresent !== prevPresent || (newPresent && !arrayLooseIdentical(prevArgs, newArgs))) { + this._literalArrayCache[id] = newArgs; + return true; + } else { + return false; + } + } + literalArray(id: number, value: any[]): any[] { - var prevValue = this._literalArrayCache[id]; if (isBlank(value)) { return value; } + var prevValue = this._literalArrayCache[id]; if (isBlank(prevValue) || !arrayLooseIdentical(prevValue, value)) { prevValue = this._literalArrayCache[id] = value; } @@ -372,10 +384,10 @@ export abstract class AppView { } literalMap(id: number, value: {[key: string]: any}): {[key: string]: any} { - var prevValue = this._literalMapCache[id]; if (isBlank(value)) { return value; } + var prevValue = this._literalMapCache[id]; if (isBlank(prevValue) || !mapLooseIdentical(prevValue, value)) { prevValue = this._literalMapCache[id] = value; } diff --git a/modules/angular2/test/core/linker/change_detection_integration_spec.ts b/modules/angular2/test/core/linker/change_detection_integration_spec.ts index becf958cf8..c15b10437b 100644 --- a/modules/angular2/test/core/linker/change_detection_integration_spec.ts +++ b/modules/angular2/test/core/linker/change_detection_integration_spec.ts @@ -491,6 +491,42 @@ export function main() { expect(renderLog.log).toEqual(['someProp=Megatron']); })); + + it('should call pure pipes only if the arguments change', fakeAsync(() => { + var ctx = _bindSimpleValue('name | countingPipe', Person); + // change from undefined -> null + ctx.componentInstance.name = null; + ctx.detectChanges(false); + expect(renderLog.loggedValues).toEqual(['null state:0']); + ctx.detectChanges(false); + expect(renderLog.loggedValues).toEqual(['null state:0']); + + // change from null -> some value + ctx.componentInstance.name = 'bob'; + ctx.detectChanges(false); + expect(renderLog.loggedValues).toEqual(['null state:0', 'bob state:1']); + ctx.detectChanges(false); + expect(renderLog.loggedValues).toEqual(['null state:0', 'bob state:1']); + + // change from some value -> some other value + ctx.componentInstance.name = 'bart'; + ctx.detectChanges(false); + expect(renderLog.loggedValues) + .toEqual(['null state:0', 'bob state:1', 'bart state:2']); + ctx.detectChanges(false); + expect(renderLog.loggedValues) + .toEqual(['null state:0', 'bob state:1', 'bart state:2']); + + })); + + it('should call impure pipes on each change detection run', fakeAsync(() => { + var ctx = _bindSimpleValue('name | countingImpurePipe', Person); + ctx.componentInstance.name = 'bob'; + ctx.detectChanges(false); + expect(renderLog.loggedValues).toEqual(['bob state:0']); + ctx.detectChanges(false); + expect(renderLog.loggedValues).toEqual(['bob state:0', 'bob state:1']); + })); }); describe('event expressions', () => { @@ -1014,6 +1050,7 @@ const ALL_DIRECTIVES = CONST_EXPR([ const ALL_PIPES = CONST_EXPR([ forwardRef(() => CountingPipe), + forwardRef(() => CountingImpurePipe), forwardRef(() => MultiArgPipe), forwardRef(() => PipeWithOnDestroy), forwardRef(() => IdentityPipe), @@ -1089,6 +1126,12 @@ class CountingPipe implements PipeTransform { transform(value, args = null) { return `${value} state:${this.state ++}`; } } +@Pipe({name: 'countingImpurePipe', pure: false}) +class CountingImpurePipe implements PipeTransform { + state: number = 0; + transform(value, args = null) { return `${value} state:${this.state ++}`; } +} + @Pipe({name: 'pipeWithOnDestroy'}) class PipeWithOnDestroy implements PipeTransform, OnDestroy { constructor(private directiveLog: DirectiveLog) {}