Skip to content

Commit d662630

Browse files
tboschtbosch
committed
Revert "fix(compiler): only call pure pipes if their input changed."
This reverts commit 8db6215.
1 parent c3daccd commit d662630

4 files changed

Lines changed: 9 additions & 78 deletions

File tree

‎modules/angular2/src/compiler/view_compiler/compile_view.ts‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ export class CompileView implements NameResolver {
124124
}
125125
}
126126

127-
callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression {
127+
createPipe(name: string): o.Expression {
128128
var pipeMeta: CompilePipeMetadata = null;
129129
for (var i = this.pipeMetas.length - 1; i >= 0; i--) {
130130
var localPipeMeta = this.pipeMetas[i];
@@ -139,7 +139,6 @@ export class CompileView implements NameResolver {
139139
}
140140
var pipeFieldName = pipeMeta.pure ? `_pipe_${name}` : `_pipe_${name}_${this.pipes.size}`;
141141
var pipeExpr = this.pipes.get(pipeFieldName);
142-
var pipeFieldCacheProp = o.THIS_EXPR.prop(`${pipeFieldName}_cache`);
143142
if (isBlank(pipeExpr)) {
144143
var deps = pipeMeta.type.diDeps.map((diDep) => {
145144
if (diDep.token.equalsTo(identifierToken(Identifiers.ChangeDetectorRef))) {
@@ -149,12 +148,6 @@ export class CompileView implements NameResolver {
149148
});
150149
this.fields.push(
151150
new o.ClassField(pipeFieldName, o.importType(pipeMeta.type), [o.StmtModifier.Private]));
152-
if (pipeMeta.pure) {
153-
this.fields.push(new o.ClassField(pipeFieldCacheProp.name, null, [o.StmtModifier.Private]));
154-
this.createMethod.addStmt(o.THIS_EXPR.prop(pipeFieldCacheProp.name)
155-
.set(o.importExpr(Identifiers.uninitialized))
156-
.toStmt());
157-
}
158151
this.createMethod.resetDebugInfo(null, null);
159152
this.createMethod.addStmt(o.THIS_EXPR.prop(pipeFieldName)
160153
.set(o.importExpr(pipeMeta.type).instantiate(deps))
@@ -163,15 +156,7 @@ export class CompileView implements NameResolver {
163156
this.pipes.set(pipeFieldName, pipeExpr);
164157
bindPipeDestroyLifecycleCallbacks(pipeMeta, pipeExpr, this);
165158
}
166-
var callPipeExpr: o.Expression = pipeExpr.callMethod('transform', [input, o.literalArr(args)]);
167-
if (pipeMeta.pure) {
168-
callPipeExpr =
169-
o.THIS_EXPR.callMethod(
170-
'checkPurePipe',
171-
[o.literal(this.literalArrayCount++), o.literalArr([input].concat(args))])
172-
.conditional(pipeFieldCacheProp.set(callPipeExpr), pipeFieldCacheProp);
173-
}
174-
return callPipeExpr;
159+
return pipeExpr;
175160
}
176161

177162
getVariable(name: string): o.Expression {

‎modules/angular2/src/compiler/view_compiler/expression_converter.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {isBlank, isPresent, isArray, CONST_EXPR} from 'angular2/src/facade/lang'
88
var IMPLICIT_RECEIVER = o.variable('#implicit');
99

1010
export interface NameResolver {
11-
callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression;
11+
createPipe(name: string): o.Expression;
1212
getVariable(name: string): o.Expression;
1313
createLiteralArray(values: o.Expression[]): o.Expression;
1414
createLiteralMap(values: Array<Array<string | o.Expression>>): o.Expression;
@@ -132,12 +132,13 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
132132
ast.falseExp.visit(this, _Mode.Expression)));
133133
}
134134
visitPipe(ast: cdAst.BindingPipe, mode: _Mode): any {
135+
var pipeInstance = this._nameResolver.createPipe(ast.name);
135136
var input = ast.exp.visit(this, _Mode.Expression);
136137
var args = this.visitAll(ast.args, _Mode.Expression);
137-
var pipeResult = this._nameResolver.callPipe(ast.name, input, args);
138138
this.needsValueUnwrapper = true;
139-
return convertToStatementIfNeeded(mode,
140-
this._valueUnwrapper.callMethod('unwrap', [pipeResult]));
139+
return convertToStatementIfNeeded(
140+
mode, this._valueUnwrapper.callMethod(
141+
'unwrap', [pipeInstance.callMethod('transform', [input, o.literalArr(args)])]));
141142
}
142143
visitFunctionCall(ast: cdAst.FunctionCall, mode: _Mode): any {
143144
return convertToStatementIfNeeded(mode, ast.target.visit(this, _Mode.Expression)

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -360,34 +360,22 @@ export abstract class AppView<T> {
360360
this.viewContainerElement = null;
361361
}
362362

363-
checkPurePipe(id: number, newArgs: any[]): boolean {
364-
var prevArgs = this._literalArrayCache[id];
365-
var newPresent = isPresent(newArgs);
366-
var prevPresent = isPresent(prevArgs);
367-
if (newPresent !== prevPresent || (newPresent && !arrayLooseIdentical(prevArgs, newArgs))) {
368-
this._literalArrayCache[id] = newArgs;
369-
return true;
370-
} else {
371-
return false;
372-
}
373-
}
374-
375363
literalArray(id: number, value: any[]): any[] {
364+
var prevValue = this._literalArrayCache[id];
376365
if (isBlank(value)) {
377366
return value;
378367
}
379-
var prevValue = this._literalArrayCache[id];
380368
if (isBlank(prevValue) || !arrayLooseIdentical(prevValue, value)) {
381369
prevValue = this._literalArrayCache[id] = value;
382370
}
383371
return prevValue;
384372
}
385373

386374
literalMap(id: number, value: {[key: string]: any}): {[key: string]: any} {
375+
var prevValue = this._literalMapCache[id];
387376
if (isBlank(value)) {
388377
return value;
389378
}
390-
var prevValue = this._literalMapCache[id];
391379
if (isBlank(prevValue) || !mapLooseIdentical(prevValue, value)) {
392380
prevValue = this._literalMapCache[id] = value;
393381
}

‎modules/angular2/test/core/linker/change_detection_integration_spec.ts‎

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -491,42 +491,6 @@ export function main() {
491491

492492
expect(renderLog.log).toEqual(['someProp=Megatron']);
493493
}));
494-
495-
it('should call pure pipes only if the arguments change', fakeAsync(() => {
496-
var ctx = _bindSimpleValue('name | countingPipe', Person);
497-
// change from undefined -> null
498-
ctx.componentInstance.name = null;
499-
ctx.detectChanges(false);
500-
expect(renderLog.loggedValues).toEqual(['null state:0']);
501-
ctx.detectChanges(false);
502-
expect(renderLog.loggedValues).toEqual(['null state:0']);
503-
504-
// change from null -> some value
505-
ctx.componentInstance.name = 'bob';
506-
ctx.detectChanges(false);
507-
expect(renderLog.loggedValues).toEqual(['null state:0', 'bob state:1']);
508-
ctx.detectChanges(false);
509-
expect(renderLog.loggedValues).toEqual(['null state:0', 'bob state:1']);
510-
511-
// change from some value -> some other value
512-
ctx.componentInstance.name = 'bart';
513-
ctx.detectChanges(false);
514-
expect(renderLog.loggedValues)
515-
.toEqual(['null state:0', 'bob state:1', 'bart state:2']);
516-
ctx.detectChanges(false);
517-
expect(renderLog.loggedValues)
518-
.toEqual(['null state:0', 'bob state:1', 'bart state:2']);
519-
520-
}));
521-
522-
it('should call impure pipes on each change detection run', fakeAsync(() => {
523-
var ctx = _bindSimpleValue('name | countingImpurePipe', Person);
524-
ctx.componentInstance.name = 'bob';
525-
ctx.detectChanges(false);
526-
expect(renderLog.loggedValues).toEqual(['bob state:0']);
527-
ctx.detectChanges(false);
528-
expect(renderLog.loggedValues).toEqual(['bob state:0', 'bob state:1']);
529-
}));
530494
});
531495

532496
describe('event expressions', () => {
@@ -1050,7 +1014,6 @@ const ALL_DIRECTIVES = CONST_EXPR([
10501014

10511015
const ALL_PIPES = CONST_EXPR([
10521016
forwardRef(() => CountingPipe),
1053-
forwardRef(() => CountingImpurePipe),
10541017
forwardRef(() => MultiArgPipe),
10551018
forwardRef(() => PipeWithOnDestroy),
10561019
forwardRef(() => IdentityPipe),
@@ -1126,12 +1089,6 @@ class CountingPipe implements PipeTransform {
11261089
transform(value, args = null) { return `${value} state:${this.state ++}`; }
11271090
}
11281091

1129-
@Pipe({name: 'countingImpurePipe', pure: false})
1130-
class CountingImpurePipe implements PipeTransform {
1131-
state: number = 0;
1132-
transform(value, args = null) { return `${value} state:${this.state ++}`; }
1133-
}
1134-
11351092
@Pipe({name: 'pipeWithOnDestroy'})
11361093
class PipeWithOnDestroy implements PipeTransform, OnDestroy {
11371094
constructor(private directiveLog: DirectiveLog) {}

0 commit comments

Comments
 (0)