refactor(pipes): removed pipes from properties

BREAKING CHANGE:

This PR remove an ability to use pipes in the properties config. Instead, inject the pipe registry.

modules/angular2/test/change_detection/change_detector_spec.ts

@@ -1,3 +1,4 @@
+///<reference path="../../src/change_detection/pipes/pipe.ts"/>
 import {
   ddescribe,
   describe,
@@ -853,28 +854,19 @@ export function main() {
   });
 }
 
-class CountingPipe extends Pipe {
-  state: number;
+class CountingPipe implements Pipe {
+  state: number = 0;
 
-  constructor() {
-    super();
-    this.state = 0;
-  }
+  onDestroy() {}
 
   supports(newValue) { return true; }
 
   transform(value) { return `${value} state:${this.state ++}`; }
 }
 
-class OncePipe extends Pipe {
-  called: boolean;
-  destroyCalled: boolean;
-
-  constructor() {
-    super();
-    this.called = false;
-    this.destroyCalled = false;
-  }
+class OncePipe implements Pipe {
+  called: boolean = false;
+  destroyCalled: boolean = false;
 
   supports(newValue) { return !this.called; }
 
@@ -886,11 +878,19 @@ class OncePipe extends Pipe {
   }
 }
 
-class IdentityPipe extends Pipe {
+class IdentityPipe implements Pipe {
+  supports(obj): boolean { return true; }
+
+  onDestroy() {}
+
   transform(value) { return value; }
 }
 
-class WrappedPipe extends Pipe {
+class WrappedPipe implements Pipe {
+  supports(obj): boolean { return true; }
+
+  onDestroy() {}
+
   transform(value) { return WrappedValue.wrap(value); }
 }
 
@@ -907,7 +907,7 @@ class FakePipeRegistry extends PipeRegistry {
     this.numberOfLookups = 0;
   }
 
-  get(type: string, obj, cdRef) {
+  get(type: string, obj, cdRef?, existingPipe?) {
     if (type != this.pipeType) return null;
     this.numberOfLookups++;
     this.cdRef = cdRef;

modules/angular2/test/change_detection/parser/parser_spec.ts

@@ -6,7 +6,7 @@ import {Parser} from 'angular2/src/change_detection/parser/parser';
 import {Unparser} from './unparser';
 import {Lexer} from 'angular2/src/change_detection/parser/lexer';
 import {Locals} from 'angular2/src/change_detection/parser/locals';
-import {Pipe, LiteralPrimitive} from 'angular2/src/change_detection/parser/ast';
+import {BindingPipe, LiteralPrimitive} from 'angular2/src/change_detection/parser/ast';
 
 class TestData {
   constructor(public a?: any, public b?: any, public fnReturnValue?: any) {}
@@ -39,8 +39,6 @@ export function main() {
     return createParser().parseInterpolation(text, location);
   }
 
-  function addPipes(ast, pipes): any { return createParser().addPipes(ast, pipes); }
-
   function emptyLocals() { return new Locals(null, new Map()); }
 
   function evalAction(text, passedInContext = null, passedInLocals = null) {
@@ -412,7 +410,7 @@ export function main() {
         it("should parse pipes", () => {
           var originalExp = '"Foo" | uppercase';
           var ast = parseBinding(originalExp).ast;
-          expect(ast).toBeAnInstanceOf(Pipe);
+          expect(ast).toBeAnInstanceOf(BindingPipe);
           expect(new Unparser().unparse(ast)).toEqual(`(${originalExp})`);
         });
 
@@ -600,7 +598,7 @@ export function main() {
       it('should parse pipes', () => {
         var bindings = parseTemplateBindings('key value|pipe');
         var ast = bindings[0].expression.ast;
-        expect(ast).toBeAnInstanceOf(Pipe);
+        expect(ast).toBeAnInstanceOf(BindingPipe);
       });
     });
 
@@ -622,29 +620,6 @@ export function main() {
       });
     });
 
-    describe('addPipes', () => {
-      it('should return the given ast whe the list of pipes is empty', () => {
-        var ast = parseBinding("1 + 1", "Location");
-        var transformedAst = addPipes(ast, []);
-        expect(transformedAst).toBe(ast);
-      });
-
-      it('should append pipe ast nodes', () => {
-        var ast = parseBinding("1 + 1", "Location");
-        var transformedAst = addPipes(ast, ['one', 'two']);
-        expect(transformedAst.ast.name).toEqual("two");
-        expect(transformedAst.ast.exp.name).toEqual("one");
-        expect(transformedAst.ast.exp.exp.operation).toEqual("+");
-      });
-
-      it('should preserve location and source', () => {
-        var ast = parseBinding("1 + 1", "Location");
-        var transformedAst = addPipes(ast, ['one', 'two']);
-        expect(transformedAst.source).toEqual("1 + 1");
-        expect(transformedAst.location).toEqual("Location");
-      });
-    });
-
     describe('wrapLiteralPrimitive', () => {
       it('should wrap a literal primitive', () => {
         expect(createParser().wrapLiteralPrimitive("foo", null).eval(null, emptyLocals()))

modules/angular2/test/change_detection/parser/unparser.ts

@@ -8,7 +8,7 @@ import {
   Conditional,
   EmptyExpr,
   If,
-  Pipe,
+  BindingPipe,
   FunctionCall,
   ImplicitReceiver,
   Interpolation,
@@ -81,7 +81,7 @@ export class Unparser implements AstVisitor {
     }
   }
 
-  visitPipe(ast: Pipe) {
+  visitPipe(ast: BindingPipe) {
     this._expression += '(';
     this._visit(ast.exp);
     this._expression += ` | ${ast.name}`;

modules/angular2/test/change_detection/parser/unparser_spec.ts

@@ -10,7 +10,7 @@ import {
   Conditional,
   EmptyExpr,
   If,
-  Pipe,
+  BindingPipe,
   ImplicitReceiver,
   Interpolation,
   KeyedAccess,
@@ -68,7 +68,7 @@ export function main() {
     it('should support Pipe', () => {
       var originalExp = '(a | b)';
       var ast = parseBinding(originalExp).ast;
-      expect(ast).toBeAnInstanceOf(Pipe);
+      expect(ast).toBeAnInstanceOf(BindingPipe);
       expect(unparser.unparse(ast)).toEqual(originalExp);
     });
 

modules/angular2/test/change_detection/pipes/pipe_registry_spec.ts

@@ -1,45 +1,77 @@
-import {ddescribe, describe, it, iit, xit, expect, beforeEach, afterEach} from 'angular2/test_lib';
+import {
+  ddescribe,
+  describe,
+  it,
+  iit,
+  xit,
+  expect,
+  beforeEach,
+  afterEach,
+  SpyPipe,
+  SpyPipeFactory
+} from 'angular2/test_lib';
 
 import {PipeRegistry} from 'angular2/src/change_detection/pipes/pipe_registry';
-import {Pipe} from 'angular2/src/change_detection/pipes/pipe';
 
 export function main() {
   describe("pipe registry", () => {
-    var firstPipe = new Pipe();
-    var secondPipe = new Pipe();
+    var firstPipe;
+    var secondPipe;
+
+    var firstPipeFactory;
+    var secondPipeFactory;
+
+    beforeEach(() => {
+      firstPipe = <any>new SpyPipe();
+      secondPipe = <any>new SpyPipe();
+
+      firstPipeFactory = <any>new SpyPipeFactory();
+      secondPipeFactory = <any>new SpyPipeFactory();
+    });
+
+    it("should return an existing pipe if it can support the passed in object", () => {
+      var r = new PipeRegistry({"type": []});
+
+      firstPipe.spy("supports").andReturn(true);
+
+      expect(r.get("type", "some object", null, firstPipe)).toEqual(firstPipe);
+    });
+
+    it("should call onDestroy on the provided pipe if it cannot support the provided object",
+       () => {
+         firstPipe.spy("supports").andReturn(false);
+         firstPipeFactory.spy("supports").andReturn(true);
+         firstPipeFactory.spy("create").andReturn(secondPipe);
+
+         var r = new PipeRegistry({"type": [firstPipeFactory]});
+
+         expect(r.get("type", "some object", null, firstPipe)).toEqual(secondPipe);
+         expect(firstPipe.spy("onDestroy")).toHaveBeenCalled();
+       });
 
     it("should return the first pipe supporting the data type", () => {
-      var r = new PipeRegistry(
-          {"type": [new PipeFactory(false, firstPipe), new PipeFactory(true, secondPipe)]});
+      firstPipeFactory.spy("supports").andReturn(false);
+      firstPipeFactory.spy("create").andReturn(firstPipe);
 
-      expect(r.get("type", "some object", null)).toBe(secondPipe);
+      secondPipeFactory.spy("supports").andReturn(true);
+      secondPipeFactory.spy("create").andReturn(secondPipe);
+
+      var r = new PipeRegistry({"type": [firstPipeFactory, secondPipeFactory]});
+
+      expect(r.get("type", "some object")).toBe(secondPipe);
     });
 
     it("should throw when no matching type", () => {
       var r = new PipeRegistry({});
-      expect(() => r.get("unknown", "some object", null))
+      expect(() => r.get("unknown", "some object"))
           .toThrowError(`Cannot find 'unknown' pipe supporting object 'some object'`);
     });
 
     it("should throw when no matching pipe", () => {
       var r = new PipeRegistry({"type": []});
 
-      expect(() => r.get("type", "some object", null))
+      expect(() => r.get("type", "some object"))
           .toThrowError(`Cannot find 'type' pipe supporting object 'some object'`);
     });
   });
-}
-
-class PipeFactory {
-  shouldSupport: boolean;
-  pipe: any;
-
-  constructor(shouldSupport: boolean, pipe: any) {
-    this.shouldSupport = shouldSupport;
-    this.pipe = pipe;
-  }
-
-  supports(obj): boolean { return this.shouldSupport; }
-
-  create(cdRef): Pipe { return this.pipe; }
-}
+}

modules/angular2/test/core/compiler/integration_spec.ts

@@ -36,6 +36,7 @@ import {PromiseWrapper, EventEmitter, ObservableWrapper} from 'angular2/src/faca
 
 import {Injector, bind, Injectable, Binding, forwardRef, OpaqueToken, Inject} from 'angular2/di';
 import {
+  PipeFactory,
   PipeRegistry,
   defaultPipeRegistry,
   ChangeDetection,
@@ -243,12 +244,11 @@ export function main() {
           ];
         });
 
-        it("should support pipes in bindings and bind config",
+        it("should support pipes in bindings",
            inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
              tb.overrideView(MyComp, new viewAnn.View({
-               template:
-                   '<component-with-pipes #comp [prop]="ctxProp | double"></component-with-pipes>',
-               directives: [ComponentWithPipes]
+               template: '<div [my-dir] #dir="mydir" [elprop]="ctxProp | double"></div>',
+               directives: [MyDir]
              }));
 
              tb.createView(MyComp, {context: ctx})
@@ -256,10 +256,8 @@ export function main() {
                    ctx.ctxProp = 'a';
                    view.detectChanges();
 
-                   var comp = view.rawView.locals.get("comp");
-
-                   // it is doubled twice: once in the binding, second time in the bind config
-                   expect(comp.prop).toEqual('aaaa');
+                   var dir = view.rawView.locals.get("dir");
+                   expect(dir.dirProp).toEqual('aa');
                    async.done();
                  });
            }));
@@ -1292,7 +1290,7 @@ class DynamicViewport {
   }
 }
 
-@Directive({selector: '[my-dir]', properties: ['dirProp: elprop']})
+@Directive({selector: '[my-dir]', properties: ['dirProp: elprop'], exportAs: 'mydir'})
 @Injectable()
 class MyDir {
   dirProp: string;
@@ -1349,7 +1347,7 @@ class MyComp {
   }
 }
 
-@Component({selector: 'component-with-pipes', properties: ["prop: prop | double"]})
+@Component({selector: 'component-with-pipes', properties: ["prop"]})
 @View({template: ''})
 @Injectable()
 class ComponentWithPipes {
@@ -1430,14 +1428,16 @@ class SomeViewport {
 }
 
 @Injectable()
-class DoublePipe extends Pipe {
+class DoublePipe implements Pipe {
+  onDestroy() {}
+
   supports(obj) { return true; }
 
   transform(value) { return `${value}${value}`; }
 }
 
 @Injectable()
-class DoublePipeFactory {
+class DoublePipeFactory implements PipeFactory {
   supports(obj) { return true; }
 
   create(cdRef) { return new DoublePipe(); }

modules/angular2/test/render/dom/compiler/directive_parser_spec.ts

@@ -86,17 +86,6 @@ export function main() {
       expect(directiveBinding.propertyBindings.get('dirProp').source).toEqual('someExpr');
     });
 
-    it('should bind directive properties with pipes', () => {
-      var results = process(el('<div some-decor-props></div>'),
-                            {'elProp': parser.parseBinding('someExpr', '')});
-      var directiveBinding = results[0].directives[0];
-      var pipedProp = <any>directiveBinding.propertyBindings.get('doubleProp');
-      var simpleProp = <any>directiveBinding.propertyBindings.get('dirProp');
-      expect(pipedProp.ast.name).toEqual('double');
-      expect(pipedProp.ast.exp).toEqual(simpleProp.ast);
-      expect(simpleProp.source).toEqual('someExpr');
-    });
-
     it('should bind directive properties from attribute values', () => {
       var results = process(el('<div some-decor-props el-prop="someValue"></div>'));
       var directiveBinding = results[0].directives[0];
@@ -237,7 +226,7 @@ var decoratorWithMultipleAttrs = DirectiveMetadata.create({
 
 var someDirectiveWithProps = DirectiveMetadata.create({
   selector: '[some-decor-props]',
-  properties: ['dirProp: elProp', 'doubleProp: elProp | double'],
+  properties: ['dirProp: elProp'],
   readAttributes: ['some-attr']
 });