diff --git a/modules/angular2/angular2.ts b/modules/angular2/angular2.ts index 2f2a1ebc4c..6cf3a5f6de 100644 --- a/modules/angular2/angular2.ts +++ b/modules/angular2/angular2.ts @@ -1,4 +1,4 @@ export * from './core'; export * from './profile'; export * from './lifecycle_hooks'; -export * from './bootstrap'; +export * from './bootstrap'; \ No newline at end of file diff --git a/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts b/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts index 9b2dff23f5..5243570fb4 100644 --- a/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts +++ b/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts @@ -236,13 +236,18 @@ export class ChangeDetectorJITGenerator { var newValue = this._names.getLocalName(r.selfIndex); var pipe = this._names.getPipeName(r.selfIndex); - var pipeType = r.name; - var read = ` + var pipeName = r.name; + + var init = ` if (${pipe} === ${UTIL}.uninitialized) { - ${pipe} = ${this._names.getPipesAccessorName()}.get('${pipeType}'); + ${pipe} = ${this._names.getPipesAccessorName()}.get('${pipeName}'); } - ${newValue} = ${pipe}.transform(${context}, [${argString}]); `; + var read = `${newValue} = ${pipe}.pipe.transform(${context}, [${argString}]);`; + + var contexOrArgCheck = r.args.map((a) => this._names.getChangeName(a)); + contexOrArgCheck.push(this._names.getChangeName(r.contextIndex)); + var condition = `!${pipe}.pure || (${contexOrArgCheck.join(" || ")})`; var check = ` if (${oldValue} !== ${newValue}) { @@ -254,7 +259,13 @@ export class ChangeDetectorJITGenerator { } `; - return r.shouldBeChecked() ? `${read}${check}` : read; + var genCode = r.shouldBeChecked() ? `${read}${check}` : read; + + if (r.isUsedByOtherRecord()) { + return `${init} if (${condition}) { ${genCode} } else { ${newValue} = ${oldValue}; }`; + } else { + return `${init} if (${condition}) { ${genCode} }`; + } } _genReferenceCheck(r: ProtoRecord): string { diff --git a/modules/angular2/src/core/change_detection/change_detection_util.ts b/modules/angular2/src/core/change_detection/change_detection_util.ts index 3293916314..8b329af0a5 100644 --- a/modules/angular2/src/core/change_detection/change_detection_util.ts +++ b/modules/angular2/src/core/change_detection/change_detection_util.ts @@ -12,6 +12,7 @@ import {ChangeDetectionStrategy, isDefaultChangeDetectionStrategy} from './const import {implementsOnDestroy} from './pipe_lifecycle_reflector'; import {BindingTarget} from './binding_record'; import {DirectiveIndex} from './directive_record'; +import {SelectedPipe} from './pipes'; /** @@ -193,9 +194,9 @@ export class ChangeDetectionUtil { protos[selfIndex - 1]; // self index is shifted by one because of context } - static callPipeOnDestroy(pipe: any): void { - if (implementsOnDestroy(pipe)) { - pipe.onDestroy(); + static callPipeOnDestroy(selectedPipe: SelectedPipe): void { + if (implementsOnDestroy(selectedPipe.pipe)) { + (selectedPipe.pipe).onDestroy(); } } diff --git a/modules/angular2/src/core/change_detection/dynamic_change_detector.ts b/modules/angular2/src/core/change_detection/dynamic_change_detector.ts index b9f8f26924..58602f55f7 100644 --- a/modules/angular2/src/core/change_detection/dynamic_change_detector.ts +++ b/modules/angular2/src/core/change_detection/dynamic_change_detector.ts @@ -333,38 +333,39 @@ export class DynamicChangeDetector extends AbstractChangeDetector { _pipeCheck(proto: ProtoRecord, throwOnChange: boolean, values: any[]) { var context = this._readContext(proto, values); - var args = this._readArgs(proto, values); + var selectedPipe = this._pipeFor(proto, context); + if (!selectedPipe.pure || this._argsOrContextChanged(proto)) { + var args = this._readArgs(proto, values); + var currValue = selectedPipe.pipe.transform(context, args); - var pipe = this._pipeFor(proto, context); - var currValue = pipe.transform(context, args); + if (proto.shouldBeChecked()) { + var prevValue = this._readSelf(proto, values); + if (!isSame(prevValue, currValue)) { + currValue = ChangeDetectionUtil.unwrapValue(currValue); - if (proto.shouldBeChecked()) { - var prevValue = this._readSelf(proto, values); - if (!isSame(prevValue, currValue)) { - currValue = ChangeDetectionUtil.unwrapValue(currValue); + if (proto.lastInBinding) { + var change = ChangeDetectionUtil.simpleChange(prevValue, currValue); + if (throwOnChange) this.throwOnChangeError(prevValue, currValue); - if (proto.lastInBinding) { - var change = ChangeDetectionUtil.simpleChange(prevValue, currValue); - if (throwOnChange) this.throwOnChangeError(prevValue, currValue); + this._writeSelf(proto, currValue, values); + this._setChanged(proto, true); - this._writeSelf(proto, currValue, values); - this._setChanged(proto, true); - - return change; + return change; + } else { + this._writeSelf(proto, currValue, values); + this._setChanged(proto, true); + return null; + } } else { - this._writeSelf(proto, currValue, values); - this._setChanged(proto, true); + this._setChanged(proto, false); return null; } } else { - this._setChanged(proto, false); + this._writeSelf(proto, currValue, values); + this._setChanged(proto, true); return null; } - } else { - this._writeSelf(proto, currValue, values); - this._setChanged(proto, true); - return null; } } @@ -413,6 +414,10 @@ export class DynamicChangeDetector extends AbstractChangeDetector { return false; } + _argsOrContextChanged(proto: ProtoRecord): boolean { + return this._argsChanged(proto) || this.changes[proto.contextIndex]; + } + _readArgs(proto: ProtoRecord, values: any[]) { var res = ListWrapper.createFixedSize(proto.args.length); var args = proto.args; diff --git a/modules/angular2/src/core/change_detection/pipes.ts b/modules/angular2/src/core/change_detection/pipes.ts index b9eb15ee0c..b23f25d34b 100644 --- a/modules/angular2/src/core/change_detection/pipes.ts +++ b/modules/angular2/src/core/change_detection/pipes.ts @@ -1,3 +1,7 @@ import {PipeTransform} from './pipe_transform'; -export interface Pipes { get(name: string): PipeTransform; } \ No newline at end of file +export interface Pipes { get(name: string): SelectedPipe; } + +export class SelectedPipe { + constructor(public pipe: PipeTransform, public pure: boolean) {} +} \ No newline at end of file diff --git a/modules/angular2/src/core/change_detection/proto_change_detector.ts b/modules/angular2/src/core/change_detection/proto_change_detector.ts index cf002f438f..80d3c71fc8 100644 --- a/modules/angular2/src/core/change_detection/proto_change_detector.ts +++ b/modules/angular2/src/core/change_detection/proto_change_detector.ts @@ -102,6 +102,11 @@ export class ProtoRecordBuilder { rec.args.forEach(recordIndex => this.records[recordIndex - 1].argumentToPureFunction = true); } + if (rec.mode === RecordType.Pipe) { + rec.args.forEach(recordIndex => this.records[recordIndex - 1].argumentToPureFunction = + true); + this.records[rec.contextIndex - 1].argumentToPureFunction = true; + } } } diff --git a/modules/angular2/src/core/change_detection/proto_record.ts b/modules/angular2/src/core/change_detection/proto_record.ts index fa5ebc3506..e4c45a6940 100644 --- a/modules/angular2/src/core/change_detection/proto_record.ts +++ b/modules/angular2/src/core/change_detection/proto_record.ts @@ -36,7 +36,8 @@ export class ProtoRecord { isUsedByOtherRecord(): boolean { return !this.lastInBinding || this.referencedBySelf; } shouldBeChecked(): boolean { - return this.argumentToPureFunction || this.lastInBinding || this.isPureFunction(); + return this.argumentToPureFunction || this.lastInBinding || this.isPureFunction() || + this.isPipeRecord(); } isPipeRecord(): boolean { return this.mode === RecordType.Pipe; } diff --git a/modules/angular2/src/core/metadata.dart b/modules/angular2/src/core/metadata.dart index 131483958b..20c366f5bc 100644 --- a/modules/angular2/src/core/metadata.dart +++ b/modules/angular2/src/core/metadata.dart @@ -69,7 +69,7 @@ class View extends ViewMetadata { * See: [PipeMetadata] for docs. */ class Pipe extends PipeMetadata { - const Pipe({name}) : super(name: name); + const Pipe({name, pure}) : super(name: name, pure: pure); } /** diff --git a/modules/angular2/src/core/metadata.ts b/modules/angular2/src/core/metadata.ts index 5807481df0..5bcffd7eef 100644 --- a/modules/angular2/src/core/metadata.ts +++ b/modules/angular2/src/core/metadata.ts @@ -398,10 +398,8 @@ export interface QueryFactory { * ``` */ export interface PipeFactory { - (obj: {name: string}): any; - new (obj: { - name: string, - }): any; + (obj: {name: string, pure?: boolean}): any; + new (obj: {name: string, pure?: boolean}): any; } /** diff --git a/modules/angular2/src/core/metadata/directives.ts b/modules/angular2/src/core/metadata/directives.ts index 613c433257..04b0798c8a 100644 --- a/modules/angular2/src/core/metadata/directives.ts +++ b/modules/angular2/src/core/metadata/directives.ts @@ -1,6 +1,6 @@ -import {CONST, CONST_EXPR} from 'angular2/src/core/facade/lang'; -import {ChangeDetectionStrategy} from 'angular2/src/core/change_detection'; +import {isPresent, CONST, CONST_EXPR} from 'angular2/src/core/facade/lang'; import {InjectableMetadata} from 'angular2/src/core/di/metadata'; +import {ChangeDetectionStrategy} from 'angular2/src/core/change_detection'; /** * Directives allow you to attach behavior to elements in the DOM. @@ -861,11 +861,15 @@ export class ComponentMetadata extends DirectiveMetadata { @CONST() export class PipeMetadata extends InjectableMetadata { name: string; + _pure: boolean; - constructor({name}: {name: string}) { + constructor({name, pure}: {name: string, pure: boolean}) { super(); this.name = name; + this._pure = pure; } + + get pure(): boolean { return isPresent(this._pure) ? this._pure : true; } } /** diff --git a/modules/angular2/src/core/pipes/async_pipe.ts b/modules/angular2/src/core/pipes/async_pipe.ts index 7835aee1cc..43de5c26aa 100644 --- a/modules/angular2/src/core/pipes/async_pipe.ts +++ b/modules/angular2/src/core/pipes/async_pipe.ts @@ -59,7 +59,7 @@ var _observableStrategy = new ObservableStrategy(); * * ``` */ -@Pipe({name: 'async'}) +@Pipe({name: 'async', pure: false}) @Injectable() export class AsyncPipe implements PipeTransform, PipeOnDestroy { _latestValue: Object = null; diff --git a/modules/angular2/src/core/pipes/pipe_binding.ts b/modules/angular2/src/core/pipes/pipe_binding.ts index 2708d07866..2af5805954 100644 --- a/modules/angular2/src/core/pipes/pipe_binding.ts +++ b/modules/angular2/src/core/pipes/pipe_binding.ts @@ -4,14 +4,15 @@ import {Key, ResolvedBinding, Binding} from 'angular2/src/core/di'; import {PipeMetadata} from '../metadata/directives'; export class PipeBinding extends ResolvedBinding { - constructor(public name: string, key: Key, resolvedFactories: ResolvedFactory[], - multiBinding: boolean) { + constructor(public name: string, public pure: boolean, key: Key, + resolvedFactories: ResolvedFactory[], multiBinding: boolean) { super(key, resolvedFactories, multiBinding); } static createFromType(type: Type, metadata: PipeMetadata): PipeBinding { var binding = new Binding(type, {toClass: type}); var rb = resolveBinding(binding); - return new PipeBinding(metadata.name, rb.key, rb.resolvedFactories, rb.multiBinding); + return new PipeBinding(metadata.name, metadata.pure, rb.key, rb.resolvedFactories, + rb.multiBinding); } } diff --git a/modules/angular2/src/core/pipes/pipes.ts b/modules/angular2/src/core/pipes/pipes.ts index b2151569fe..7d0f6b7b24 100644 --- a/modules/angular2/src/core/pipes/pipes.ts +++ b/modules/angular2/src/core/pipes/pipes.ts @@ -1,4 +1,5 @@ import {isBlank, isPresent, BaseException, CONST, Type} from 'angular2/src/core/facade/lang'; +import {StringMapWrapper} from 'angular2/src/core/facade/collection'; import { Injectable, OptionalMetadata, @@ -25,11 +26,25 @@ export class ProtoPipes { } } + + export class Pipes implements cd.Pipes { + _config: StringMap = {}; + constructor(public proto: ProtoPipes, public injector: Injector) {} - get(name: string): any { - var b = this.proto.get(name); - return this.injector.instantiateResolved(b); + get(name: string): cd.SelectedPipe { + var cached = StringMapWrapper.get(this._config, name); + if (isPresent(cached)) return cached; + + var p = this.proto.get(name); + var transform = this.injector.instantiateResolved(p); + var res = new cd.SelectedPipe(transform, p.pure); + + if (p.pure) { + StringMapWrapper.set(this._config, name, res); + } + + return res; } } diff --git a/modules/angular2/test/core/change_detection/change_detector_spec.ts b/modules/angular2/test/core/change_detection/change_detector_spec.ts index 26069194d9..6366cdeb8f 100644 --- a/modules/angular2/test/core/change_detection/change_detector_spec.ts +++ b/modules/angular2/test/core/change_detection/change_detector_spec.ts @@ -17,7 +17,8 @@ import { isBlank, isJsObject, BaseException, - FunctionWrapper + FunctionWrapper, + normalizeBool } from 'angular2/src/core/facade/lang'; import {ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/core/facade/collection'; @@ -41,7 +42,7 @@ import { ProtoChangeDetector } from 'angular2/src/core/change_detection/change_detection'; -import {Pipes} from 'angular2/src/core/change_detection/pipes'; +import {SelectedPipe, Pipes} from 'angular2/src/core/change_detection/pipes'; import {JitProtoChangeDetector} from 'angular2/src/core/change_detection/jit_proto_change_detector'; import {getDefinition} from './change_detector_config'; @@ -351,6 +352,36 @@ export function main() { expect(val.dispatcher.loggedValues).toEqual(['value one two default']); }); + it('should not reevaluate pure pipes unless its context or arg changes', () => { + var pipe = new CountingPipe(); + var registry = new FakePipes('pipe', () => pipe, {pure: true}); + var person = new Person('bob'); + var val = _createChangeDetector('name | pipe', person, registry); + + val.changeDetector.detectChanges(); + expect(pipe.state).toEqual(1); + + val.changeDetector.detectChanges(); + expect(pipe.state).toEqual(1); + + person.name = 'jim'; + val.changeDetector.detectChanges(); + expect(pipe.state).toEqual(2); + }); + + it('should reevaluate impure pipes neither context nor arg changes', () => { + var pipe = new CountingPipe(); + var registry = new FakePipes('pipe', () => pipe, {pure: false}); + var person = new Person('bob'); + var val = _createChangeDetector('name | pipe', person, registry); + + val.changeDetector.detectChanges(); + expect(pipe.state).toEqual(1); + + val.changeDetector.detectChanges(); + expect(pipe.state).toEqual(2); + }); + it('should support pipes as arguments to pure functions', () => { var registry = new FakePipes('pipe', () => new IdentityPipe()); var person = new Person('bob'); @@ -1116,24 +1147,6 @@ export function main() { }); }); - describe('pipes', () => { - it('should support pipes', () => { - var registry = new FakePipes('pipe', () => new CountingPipe()); - var ctx = new Person('Megatron'); - - var val = _createChangeDetector('name | pipe', ctx, registry); - - val.changeDetector.detectChanges(); - - expect(val.dispatcher.log).toEqual(['propName=Megatron state:0']); - - val.dispatcher.clear(); - val.changeDetector.detectChanges(); - - expect(val.dispatcher.log).toEqual(['propName=Megatron state:1']); - }); - }); - it('should do nothing when no change', () => { var registry = new FakePipes('pipe', () => new IdentityPipe()); var ctx = new Person('Megatron'); @@ -1256,13 +1269,16 @@ class MultiArgPipe implements PipeTransform { class FakePipes implements Pipes { numberOfLookups = 0; + pure: boolean; - constructor(public pipeType: string, public factory: Function) {} + constructor(public pipeType: string, public factory: Function, {pure}: {pure?: boolean} = {}) { + this.pure = normalizeBool(pure); + } get(type: string) { if (type != this.pipeType) return null; this.numberOfLookups++; - return this.factory(); + return new SelectedPipe(this.factory(), this.pure); } } diff --git a/modules/angular2/test/core/pipes/pipes_spec.ts b/modules/angular2/test/core/pipes/pipes_spec.ts index 32187fc286..c5947bef25 100644 --- a/modules/angular2/test/core/pipes/pipes_spec.ts +++ b/modules/angular2/test/core/pipes/pipes_spec.ts @@ -36,7 +36,8 @@ export function main() { it('should instantiate a pipe', () => { var proto = new ProtoPipes([PipeBinding.createFromType(PipeA, new Pipe({name: 'a'}))]); var pipes = new Pipes(proto, injector); - expect(pipes.get("a")).toBeAnInstanceOf(PipeA); + + expect(pipes.get("a").pipe).toBeAnInstanceOf(PipeA); }); it('should throw when no pipe found', () => { @@ -48,7 +49,25 @@ export function main() { it('should inject dependencies from the provided injector', () => { var proto = new ProtoPipes([PipeBinding.createFromType(PipeB, new Pipe({name: 'b'}))]); var pipes = new Pipes(proto, injector); - expect(pipes.get("b").dep).toEqual("dependency"); + expect((pipes.get("b").pipe).dep).toEqual("dependency"); + }); + + it('should cache pure pipes', () => { + var proto = + new ProtoPipes([PipeBinding.createFromType(PipeA, new Pipe({name: 'a', pure: true}))]); + var pipes = new Pipes(proto, injector); + + expect(pipes.get("a").pure).toEqual(true); + expect(pipes.get("a")).toBe(pipes.get("a")); + }); + + it('should NOT cache impure pipes', () => { + var proto = + new ProtoPipes([PipeBinding.createFromType(PipeA, new Pipe({name: 'a', pure: false}))]); + var pipes = new Pipes(proto, injector); + + expect(pipes.get("a").pure).toEqual(false); + expect(pipes.get("a")).not.toBe(pipes.get("a")); }); }); } diff --git a/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart b/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart index 597be85eff..a7442c39a3 100644 --- a/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart +++ b/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart @@ -335,13 +335,20 @@ class _CodegenState { var pipe = _names.getPipeName(r.selfIndex); var pipeType = r.name; - var read = ''' + var init = ''' if ($_IDENTICAL_CHECK_FN($pipe, $_UTIL.uninitialized)) { $pipe = ${_names.getPipesAccessorName()}.get('$pipeType'); } - $newValue = $pipe.transform($context, [$argString]); '''; + var read = ''' + $newValue = $pipe.pipe.transform($context, [$argString]); + '''; + + var contexOrArgCheck = r.args.map((a) => _names.getChangeName(a)).toList(); + contexOrArgCheck.add(_names.getChangeName(r.contextIndex)); + var condition = '''!${pipe}.pure || (${contexOrArgCheck.join(" || ")})'''; + var check = ''' if ($_NOT_IDENTICAL_CHECK_FN($oldValue, $newValue)) { $newValue = $_UTIL.unwrapValue($newValue); @@ -352,7 +359,13 @@ class _CodegenState { } '''; - return r.shouldBeChecked() ? "${read}${check}" : read; + var genCode = r.shouldBeChecked() ? '''${read}${check}''' : read; + + if (r.isUsedByOtherRecord()) { + return '''${init} if (${condition}) { ${genCode} } else { ${newValue} = ${oldValue}; }'''; + } else { + return '''${init} if (${condition}) { ${genCode} }'''; + } } String _genReferenceCheck(ProtoRecord r) {