fix(compiler): only call pure pipes if their input changed.

Author: tbosch

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 {

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<Array<string | o.Expression>>): 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)

modules/angular2/src/core/linker/view.ts

@@ -360,22 +360,34 @@ export abstract class AppView<T> {
     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;
     }
     return prevValue;
   }
 
   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;
     }

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) {}