Skip to content

Commit

Permalink
feat(pipes): changed PipeTransform to make onDestroy optional
Browse files Browse the repository at this point in the history
BREAKING CHANGE:

Before:
  Angular called onDestroy on all pipes.

After:
  Angular calls onDestroy only on pipes that have the onDestroy method.
  • Loading branch information
vsavkin committed Aug 13, 2015
1 parent ac31191 commit 839edaa
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 18 deletions.
2 changes: 1 addition & 1 deletion modules/angular2/src/change_detection/change_detection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export {DynamicChangeDetector} from './dynamic_change_detector';
export {ChangeDetectorRef} from './change_detector_ref';
export {IterableDiffers, IterableDiffer, IterableDifferFactory} from './differs/iterable_differs';
export {KeyValueDiffers, KeyValueDiffer, KeyValueDifferFactory} from './differs/keyvalue_differs';
export {PipeTransform, BasePipeTransform} from './pipe_transform';
export {PipeTransform, PipeOnDestroy, BasePipeTransform} from './pipe_transform';
export {WrappedValue} from './change_detection_util';

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {CONST_EXPR, isPresent, isBlank, BaseException, Type} from 'angular2/src/
import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
import {ProtoRecord} from './proto_record';
import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants';
import {implementsOnDestroy} from './pipe_lifecycle_reflector';


/**
Expand Down Expand Up @@ -180,4 +181,10 @@ export class ChangeDetectionUtil {
null :
protos[selfIndex - 1]; // self index is shifted by one because of context
}

static callPipeOnDestroy(pipe: any): void {
if (implementsOnDestroy(pipe)) {
pipe.onDestroy();
}
}
}
10 changes: 7 additions & 3 deletions modules/angular2/src/change_detection/codegen_name_util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,13 @@ export class CodegenNameUtil {
* Generates statements destroying all pipe variables.
*/
genPipeOnDestroy(): string {
return ListWrapper.join(ListWrapper.map(ListWrapper.filter(this.records, (r) => {
return r.isPipeRecord();
}), (r) => { return `${this.getPipeName(r.selfIndex)}.onDestroy();`; }), '\n');
return ListWrapper.join(
ListWrapper.map(
ListWrapper.filter(this.records, (r) => { return r.isPipeRecord(); }),
(r) => {
return `${this.utilName}.callPipeOnDestroy(${this.getPipeName(r.selfIndex)});`;
}),
'\n');
}

getPipeName(idx: int): string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
_destroyPipes() {
for (var i = 0; i < this.localPipes.length; ++i) {
if (isPresent(this.localPipes[i])) {
this.localPipes[i].onDestroy();
ChangeDetectionUtil.callPipeOnDestroy(this.localPipes[i]);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
library angular2.core.compiler.pipe_lifecycle_reflector;

import 'package:angular2/src/change_detection/pipe_transform.dart';

bool implementsOnDestroy(Object pipe) => pipe is PipeOnDestroy;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function implementsOnDestroy(pipe: any): boolean {
return pipe.constructor.prototype.onDestroy;
}
31 changes: 24 additions & 7 deletions modules/angular2/src/change_detection/pipe_transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,36 @@ import {ABSTRACT, BaseException, CONST, Type} from 'angular2/src/facade/lang';
*
* ```
* class DoublePipe implements PipeTransform {
* onDestroy() {}
*
* transform(value, args = []) {
* return `${value}${value}`;
* }
* }
* ```
*/
export interface PipeTransform {
onDestroy(): void;
export interface PipeTransform { transform(value: any, args: List<any>): any; }

transform(value: any, args: List<any>): any;
}
/**
* An interface that stateful pipes should implement.
*
* #Example
*
* ```
* class StatefulPipe implements PipeTransform, PipeOnDestroy {
* connection;
*
* onDestroy() {
* this.connection.release();
* }
*
* transform(value, args = []) {
* this.connection = createConnection();
* // ...
* return someValue;
* }
* }
* ```
*/
export interface PipeOnDestroy { onDestroy(): void; }

/**
* Provides default implementation of the `onDestroy` method.
Expand All @@ -35,7 +52,7 @@ export interface PipeTransform {
* ```
*/
@CONST()
export class BasePipeTransform implements PipeTransform {
export class BasePipeTransform implements PipeTransform, PipeOnDestroy {
onDestroy(): void {}
transform(value: any, args: List<any>): any { return _abstract(); }
}
Expand Down
18 changes: 12 additions & 6 deletions modules/angular2/test/change_detection/change_detector_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
DirectiveRecord,
DirectiveIndex,
PipeTransform,
PipeOnDestroy,
CHECK_ALWAYS,
CHECK_ONCE,
CHECKED,
Expand Down Expand Up @@ -768,7 +769,7 @@ export function main() {
expect(cd.hydrated()).toBe(true);
});

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

it('should not call onDestroy all pipes that do not implement onDestroy', () => {
var pipe = new CountingPipe();
var registry = new FakePipes('pipe', () => pipe);
var cd = _createChangeDetector('name | pipe', new Person('bob'), registry).changeDetector;

cd.detectChanges();
expect(() => cd.dehydrate()).not.toThrow();
});

it('should throw when detectChanges is called on a dehydrated detector', () => {
var context = new Person('Bob');
var val = _createChangeDetector('name', context);
Expand Down Expand Up @@ -844,24 +854,21 @@ export function main() {

class CountingPipe implements PipeTransform {
state: number = 0;
onDestroy() {}
transform(value, args = null) { return `${value} state:${this.state ++}`; }
}

class PipeWithOnDestroy implements PipeTransform {
class PipeWithOnDestroy implements PipeTransform, PipeOnDestroy {
destroyCalled: boolean = false;
onDestroy() { this.destroyCalled = true; }

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

class IdentityPipe implements PipeTransform {
onDestroy() {}
transform(value, args = null) { return value; }
}

class WrappedPipe implements PipeTransform {
onDestroy() {}
transform(value, args = null) { return WrappedValue.wrap(value); }
}

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

class FakePipes implements Pipes {
Expand Down

0 comments on commit 839edaa

Please sign in to comment.