Skip to content

Commit 839edaa

Browse files
committed
feat(pipes): changed PipeTransform to make onDestroy optional
BREAKING CHANGE: Before: Angular called onDestroy on all pipes. After: Angular calls onDestroy only on pipes that have the onDestroy method.
1 parent ac31191 commit 839edaa

File tree

8 files changed

+60
-18
lines changed

8 files changed

+60
-18
lines changed

modules/angular2/src/change_detection/change_detection.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export {DynamicChangeDetector} from './dynamic_change_detector';
4444
export {ChangeDetectorRef} from './change_detector_ref';
4545
export {IterableDiffers, IterableDiffer, IterableDifferFactory} from './differs/iterable_differs';
4646
export {KeyValueDiffers, KeyValueDiffer, KeyValueDifferFactory} from './differs/keyvalue_differs';
47-
export {PipeTransform, BasePipeTransform} from './pipe_transform';
47+
export {PipeTransform, PipeOnDestroy, BasePipeTransform} from './pipe_transform';
4848
export {WrappedValue} from './change_detection_util';
4949

5050
/**

modules/angular2/src/change_detection/change_detection_util.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {CONST_EXPR, isPresent, isBlank, BaseException, Type} from 'angular2/src/
22
import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
33
import {ProtoRecord} from './proto_record';
44
import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants';
5+
import {implementsOnDestroy} from './pipe_lifecycle_reflector';
56

67

78
/**
@@ -180,4 +181,10 @@ export class ChangeDetectionUtil {
180181
null :
181182
protos[selfIndex - 1]; // self index is shifted by one because of context
182183
}
184+
185+
static callPipeOnDestroy(pipe: any): void {
186+
if (implementsOnDestroy(pipe)) {
187+
pipe.onDestroy();
188+
}
189+
}
183190
}

modules/angular2/src/change_detection/codegen_name_util.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,13 @@ export class CodegenNameUtil {
146146
* Generates statements destroying all pipe variables.
147147
*/
148148
genPipeOnDestroy(): string {
149-
return ListWrapper.join(ListWrapper.map(ListWrapper.filter(this.records, (r) => {
150-
return r.isPipeRecord();
151-
}), (r) => { return `${this.getPipeName(r.selfIndex)}.onDestroy();`; }), '\n');
149+
return ListWrapper.join(
150+
ListWrapper.map(
151+
ListWrapper.filter(this.records, (r) => { return r.isPipeRecord(); }),
152+
(r) => {
153+
return `${this.utilName}.callPipeOnDestroy(${this.getPipeName(r.selfIndex)});`;
154+
}),
155+
'\n');
152156
}
153157

154158
getPipeName(idx: int): string {

modules/angular2/src/change_detection/dynamic_change_detector.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
4848
_destroyPipes() {
4949
for (var i = 0; i < this.localPipes.length; ++i) {
5050
if (isPresent(this.localPipes[i])) {
51-
this.localPipes[i].onDestroy();
51+
ChangeDetectionUtil.callPipeOnDestroy(this.localPipes[i]);
5252
}
5353
}
5454
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
library angular2.core.compiler.pipe_lifecycle_reflector;
2+
3+
import 'package:angular2/src/change_detection/pipe_transform.dart';
4+
5+
bool implementsOnDestroy(Object pipe) => pipe is PipeOnDestroy;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function implementsOnDestroy(pipe: any): boolean {
2+
return pipe.constructor.prototype.onDestroy;
3+
}

modules/angular2/src/change_detection/pipe_transform.ts

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,36 @@ import {ABSTRACT, BaseException, CONST, Type} from 'angular2/src/facade/lang';
77
*
88
* ```
99
* class DoublePipe implements PipeTransform {
10-
* onDestroy() {}
11-
*
1210
* transform(value, args = []) {
1311
* return `${value}${value}`;
1412
* }
1513
* }
1614
* ```
1715
*/
18-
export interface PipeTransform {
19-
onDestroy(): void;
16+
export interface PipeTransform { transform(value: any, args: List<any>): any; }
2017

21-
transform(value: any, args: List<any>): any;
22-
}
18+
/**
19+
* An interface that stateful pipes should implement.
20+
*
21+
* #Example
22+
*
23+
* ```
24+
* class StatefulPipe implements PipeTransform, PipeOnDestroy {
25+
* connection;
26+
*
27+
* onDestroy() {
28+
* this.connection.release();
29+
* }
30+
*
31+
* transform(value, args = []) {
32+
* this.connection = createConnection();
33+
* // ...
34+
* return someValue;
35+
* }
36+
* }
37+
* ```
38+
*/
39+
export interface PipeOnDestroy { onDestroy(): void; }
2340

2441
/**
2542
* Provides default implementation of the `onDestroy` method.
@@ -35,7 +52,7 @@ export interface PipeTransform {
3552
* ```
3653
*/
3754
@CONST()
38-
export class BasePipeTransform implements PipeTransform {
55+
export class BasePipeTransform implements PipeTransform, PipeOnDestroy {
3956
onDestroy(): void {}
4057
transform(value: any, args: List<any>): any { return _abstract(); }
4158
}

modules/angular2/test/change_detection/change_detector_spec.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
DirectiveRecord,
3131
DirectiveIndex,
3232
PipeTransform,
33+
PipeOnDestroy,
3334
CHECK_ALWAYS,
3435
CHECK_ONCE,
3536
CHECKED,
@@ -768,7 +769,7 @@ export function main() {
768769
expect(cd.hydrated()).toBe(true);
769770
});
770771

771-
it('should destroy all active pipes during dehyration', () => {
772+
it('should destroy all active pipes implementing onDestroy during dehyration', () => {
772773
var pipe = new PipeWithOnDestroy();
773774
var registry = new FakePipes('pipe', () => pipe);
774775
var cd = _createChangeDetector('name | pipe', new Person('bob'), registry).changeDetector;
@@ -779,6 +780,15 @@ export function main() {
779780
expect(pipe.destroyCalled).toBe(true);
780781
});
781782

783+
it('should not call onDestroy all pipes that do not implement onDestroy', () => {
784+
var pipe = new CountingPipe();
785+
var registry = new FakePipes('pipe', () => pipe);
786+
var cd = _createChangeDetector('name | pipe', new Person('bob'), registry).changeDetector;
787+
788+
cd.detectChanges();
789+
expect(() => cd.dehydrate()).not.toThrow();
790+
});
791+
782792
it('should throw when detectChanges is called on a dehydrated detector', () => {
783793
var context = new Person('Bob');
784794
var val = _createChangeDetector('name', context);
@@ -844,24 +854,21 @@ export function main() {
844854

845855
class CountingPipe implements PipeTransform {
846856
state: number = 0;
847-
onDestroy() {}
848857
transform(value, args = null) { return `${value} state:${this.state ++}`; }
849858
}
850859

851-
class PipeWithOnDestroy implements PipeTransform {
860+
class PipeWithOnDestroy implements PipeTransform, PipeOnDestroy {
852861
destroyCalled: boolean = false;
853862
onDestroy() { this.destroyCalled = true; }
854863

855864
transform(value, args = null) { return null; }
856865
}
857866

858867
class IdentityPipe implements PipeTransform {
859-
onDestroy() {}
860868
transform(value, args = null) { return value; }
861869
}
862870

863871
class WrappedPipe implements PipeTransform {
864-
onDestroy() {}
865872
transform(value, args = null) { return WrappedValue.wrap(value); }
866873
}
867874

@@ -872,7 +879,6 @@ class MultiArgPipe implements PipeTransform {
872879
var arg3 = args.length > 2 ? args[2] : 'default';
873880
return `${value} ${arg1} ${arg2} ${arg3}`;
874881
}
875-
onDestroy(): void {}
876882
}
877883

878884
class FakePipes implements Pipes {

0 commit comments

Comments
 (0)