From d6626309fd8a25ce22ef76adc7ca2c3ee931ab37 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Mon, 25 Apr 2016 07:53:49 -0700 Subject: [PATCH] Revert "fix(compiler): only call pure pipes if their input changed." This reverts commit 8db62151d27f6f698c30f0944610ef92c66fcb5a. --- .../compiler/view_compiler/compile_view.ts | 19 +------- .../view_compiler/expression_converter.ts | 9 ++-- modules/angular2/src/core/linker/view.ts | 16 +------ .../change_detection_integration_spec.ts | 43 ------------------- 4 files changed, 9 insertions(+), 78 deletions(-) diff --git a/modules/angular2/src/compiler/view_compiler/compile_view.ts b/modules/angular2/src/compiler/view_compiler/compile_view.ts index 1c1bf99ed2..41fad43044 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 { } } - callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression { + createPipe(name: string): o.Expression { var pipeMeta: CompilePipeMetadata = null; for (var i = this.pipeMetas.length - 1; i >= 0; i--) { var localPipeMeta = this.pipeMetas[i]; @@ -139,7 +139,6 @@ 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))) { @@ -149,12 +148,6 @@ 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)) @@ -163,15 +156,7 @@ export class CompileView implements NameResolver { this.pipes.set(pipeFieldName, pipeExpr); bindPipeDestroyLifecycleCallbacks(pipeMeta, pipeExpr, this); } - 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; + return pipeExpr; } 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 b3d512cc5f..c50f19595b 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 { - callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression; + createPipe(name: string): o.Expression; getVariable(name: string): o.Expression; createLiteralArray(values: o.Expression[]): o.Expression; createLiteralMap(values: Array>): o.Expression; @@ -132,12 +132,13 @@ 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', [pipeResult])); + return convertToStatementIfNeeded( + mode, this._valueUnwrapper.callMethod( + 'unwrap', [pipeInstance.callMethod('transform', [input, o.literalArr(args)])])); } 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 1f97473c2e..dd1001f531 100644 --- a/modules/angular2/src/core/linker/view.ts +++ b/modules/angular2/src/core/linker/view.ts @@ -360,23 +360,11 @@ 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; } @@ -384,10 +372,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 c15b10437b..becf958cf8 100644 --- a/modules/angular2/test/core/linker/change_detection_integration_spec.ts +++ b/modules/angular2/test/core/linker/change_detection_integration_spec.ts @@ -491,42 +491,6 @@ 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', () => { @@ -1050,7 +1014,6 @@ const ALL_DIRECTIVES = CONST_EXPR([ const ALL_PIPES = CONST_EXPR([ forwardRef(() => CountingPipe), - forwardRef(() => CountingImpurePipe), forwardRef(() => MultiArgPipe), forwardRef(() => PipeWithOnDestroy), forwardRef(() => IdentityPipe), @@ -1126,12 +1089,6 @@ 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) {}