Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

angular.d.ts updates with constructor and factory interfaces. #2369

Merged
merged 5 commits into from
Jul 24, 2014

Conversation

ngbrown
Copy link
Contributor

@ngbrown ngbrown commented Jun 19, 2014

Adds better intellisense for ng.IModule.provider() and ng.IModule.directive() when used with a constructor/class or object factory function.

Also replaces single use of ng.auto.IProvider with identical ng.IServiceProvider.

Instead of IModule.provider simply accepting an Function for a class
constructor, use an interface to enforce the type of class.
Adds intellisense when constructing directive objects
Missed provider functions that return objects.  Ensure that they also
implement ng.IServiceProvider.
Converted the test module, 'http-auth-interceptor', to a class to
eliminate error on using anonymous type constructor.
@ngbrown
Copy link
Contributor Author

ngbrown commented Jun 19, 2014

To get the CI tests to pass, I had to converted the test module, 'http-auth-interceptor', to a class to eliminate error on using anonymous type constructor.

I'm not sure what the philosophy on combining JS constructs like anonymous types:

new (function() { this.$get = function() {}; })

and letting them work with DefinitelyTyped instead of converting them to idiomatic classes:

class MyService {
    $get() {
    }
}

new MyService();

Since an example using these constructs pasted into TypeScript will still compile through even with an error/warnings like this, I think this approach is enhances the intent of the AngularJS library and lets people improve their code.

@vvakame
Copy link
Member

vvakame commented Jul 10, 2014

sorry, i was late..
Please wait a more little..

@vvakame vvakame merged commit 72b9fbe into DefinitelyTyped:master Jul 24, 2014
@vvakame
Copy link
Member

vvakame commented Jul 24, 2014

@ngbrown merged! thanks mate.

@symi
Copy link

symi commented Jul 29, 2014

This latest change ng.IModule.directive() has broken the valid use case where the directive is returned from a constructor function. i.e the following needs adding (consistent with how IServiceProviderClass works):

directive(name: string, directiveConstructor: IDirectiveClass): IModule;

and

interface IDirectiveClass {
    new(...args: any[]): IDirective;
}

@@ -119,7 +128,7 @@ declare module ng {
*/
controller(name: string, inlineAnnotatedConstructor: any[]): IModule;
controller(object : Object): IModule;
directive(name: string, directiveFactory: Function): IModule;
directive(name: string, directiveFactory: IDirectiveFactory): IModule;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

causes #2618

@basarat
Copy link
Member

basarat commented Aug 7, 2014

has broken the valid use case where the directive is returned from a constructor function

Tracking #2618

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants