From 26d0c8a75722643a6febfb98427fbd51f62784fc Mon Sep 17 00:00:00 2001
From: Anton Zolotkov <azolotkov@gmail.com>
Date: Wed, 20 Mar 2013 11:48:15 +0900
Subject: [PATCH] AngularJS: fixed typing of the ng.IPromise.then function

The type definition for ng.IPromise.then was, in my opinion, incorrect
semantically, and breaking on practical examples in my codebase.

An `IPromise`' `then` function takes a callback which is called with the
value of the promise once it's fulfilled. This could be a number, a
string, some object, an array of strings, anything really. Yet the
typing in angular.d.ts specified `then` as taking a `successCallback` of
type `(response: PromiseCallbackArg) => any`.

The definition of `PromiseCallbackArg` seems very permissive at first
glance, as it is defined to be an object with a bunch of fields, all
optional. Any object, a number, and a string all type check correctly
with such a type, but an array of strings, for example, does not
(`cPromise` in the provided list of test).

Furthermore, it seems to me that the `PromiseCallbackArg` definition was
added specifically to support the response type for promises returned by
the Angular `$http` service, the `ng.IHttpPromise`.

So instead of having an incorrect type on the `ng.IPromise.then` function,
I propose we return it to its generic form, and instead override the
type of the inherited `then` function in the `ng.IHttpPromise` interface.
This would also warrant renaming `PromiseCallbackArg` to
`IHttpPromiseCallbackArg`.
---
 angularjs/angular-tests.ts | 64 ++++++++++++++++++++++++++++++++++++--
 angularjs/angular.d.ts     | 19 +++++------
 2 files changed, 71 insertions(+), 12 deletions(-)

diff --git a/angularjs/angular-tests.ts b/angularjs/angular-tests.ts
index e1426a1a4d..d5414dc627 100644
--- a/angularjs/angular-tests.ts
+++ b/angularjs/angular-tests.ts
@@ -58,11 +58,11 @@ angular.module('http-auth-interceptor', [])
   .config(['$httpProvider', 'authServiceProvider', <any>function ($httpProvider: ng.IHttpProvider, authServiceProvider) {
 
       var interceptor = ['$rootScope', '$q', <any>function ($rootScope: ng.IScope, $q: ng.IQService) {
-          function success(response: ng.PromiseCallbackArg) {
+          function success(response: ng.IHttpPromiseCallbackArg) {
               return response;
           }
 
-          function error(response: ng.PromiseCallbackArg) {
+          function error(response: ng.IHttpPromiseCallbackArg) {
               if (response.status === 401) {
                   var deferred = $q.defer();
                   authServiceProvider.pushToBuffer(response.config, deferred);
@@ -79,4 +79,62 @@ angular.module('http-auth-interceptor', [])
 
       }];
       $httpProvider.responseInterceptors.push(interceptor);
-  }]);
\ No newline at end of file
+  }]);
+
+
+module HttpAndRegularPromiseTests {
+  interface Person {
+    firstName: string;
+    lastName:  string;
+  }
+
+  interface ExpectedResponse extends Person {}
+
+  interface SomeControllerScope extends ng.IScope {
+    person:    Person;
+    theAnswer: number;
+    letters:   string[];
+  }
+
+  interface OurApiPromiseCallbackArg extends ng.IHttpPromiseCallbackArg {
+    data?: ExpectedResponse;
+  }
+
+  var someController: Function = ($scope: SomeControllerScope, $http: ng.IHttpService, $q: ng.IQService) => {
+    $http.get("http://somewhere/some/resource")
+    .success((data: ExpectedResponse) => {
+      $scope.person = data;
+    });
+
+    $http.get("http://somewhere/some/resource")
+    .then((response: ng.IHttpPromiseCallbackArg) => {
+      // typing lost, so something like
+      // var i: number = response.data
+      // would type check
+      $scope.person = response.data;
+    });
+
+    $http.get("http://somewhere/some/resource")
+    .then((response: OurApiPromiseCallbackArg) => {
+      // typing lost, so something like
+      // var i: number = response.data
+      // would NOT type check
+      $scope.person = response.data;
+    });
+
+    var aPromise: ng.IPromise = $q.when({firstName: "Jack", lastName: "Sparrow"});
+    aPromise.then((person: Person) => {
+      $scope.person = person;
+    });
+
+    var bPromise: ng.IPromise = $q.when(42);
+    bPromise.then((answer: number) => {
+      $scope.theAnswer = answer;
+    });
+
+    var cPromise: ng.IPromise = $q.when(["a", "b", "c"]);
+    cPromise.then((letters: string[]) => {
+      $scope.letters = letters;
+    });
+  }
+}
diff --git a/angularjs/angular.d.ts b/angularjs/angular.d.ts
index 7f5cb187d5..b08fefe3e2 100644
--- a/angularjs/angular.d.ts
+++ b/angularjs/angular.d.ts
@@ -385,15 +385,8 @@ module ng {
         when(value: any): IPromise;
     }
 
-    interface PromiseCallbackArg {
-        data?: any;
-        status?: number;
-        headers?: (headerName: string) => string;
-        config?: IRequestConfig;
-    }
-
     interface IPromise {
-        then(successCallback: (response: PromiseCallbackArg) => any, errorCallback?: (response: PromiseCallbackArg) => any): IPromise;
+        then(successCallback: (promiseValue: any) => any, errorCallback?: (reason: any) => any): IPromise;
     }
 
     interface IDeferred {
@@ -524,9 +517,17 @@ module ng {
         transformResponse?: any;
     }
 
+    interface IHttpPromiseCallbackArg {
+        data?: any;
+        status?: number;
+        headers?: (headerName: string) => string;
+        config?: IRequestConfig;
+    }
+
     interface IHttpPromise extends IPromise {
         success(callback: (data: any, status: number, headers: (headerName: string) => string, config: IRequestConfig) => any): IHttpPromise;
         error(callback: (data: any, status: number, headers: (headerName: string) => string, config: IRequestConfig) => any): IHttpPromise;
+        then(successCallback: (response: IHttpPromiseCallbackArg) => any, errorCallback?: (response: IHttpPromiseCallbackArg) => any): IPromise;
     }
 
     interface IHttpProvider extends IServiceProvider {
@@ -658,4 +659,4 @@ module ng {
 
     }
 
-}
\ No newline at end of file
+}