AngularJs TypeScript typings and --strictFunctionTypes

[rkirov]

I'm submitting a ...

• bug report
• feature request
• design discussion

TypeScript introduced a new strictness flag with TS 2.7 - https://github.com/Microsoft/TypeScript/wiki/What%27s-new-in-TypeScript#strict-function-types

It makes typechecking for callback based APIs much stricter. For example:

function f(mixedArray: (string|number)[]) {
  mixedArray.map((x: string) => x.charAt(0));
}

only errors when --strictFunctionTypes is turned on.

AngularJs has many callback based APIs, which will be checked at more strictly with the flag. Some real bugs will be exposed, but also it will start erring on many correct patterns. For example:

{
  require: 'ngModel'
  link: (...., controller: NgModelController) {...}

This is now an error because TS doesn't know that require and link are related. As written the typings require one handle the full spectrum of options in the link function IController | IController[] | {[key: string]: IController}.

TS types are quite expressive and @gkalpak and I tried to write up some prototype of typings that are aware of the link between require and link:

interface IDirective<T extends string|string[]> {
  link?: (x: LinkT<T>) => void;
  require?: T;
}

type LinkT<T> =
    T extends string ? IController : T extends string[] ? IController[] : never;

But even with full conditional types, it is impossible to extend this for user-defined directives.

So as plan B, I am going to contribute some nice type aliases to DefinitelyTyped type LinkFnControllerType = IController | IController[] | {[key: string]: IController}
and propose the canonical solution to be writing the link function as:

link: (..., controller: angular.LinkFnControllerType) => {
  let myController = controller as MyControllerType;
}

One added benefit is that there is an explicit 'as' cast to remind the reader to check with the 'require' and controller statements.

[gkalpak]

If one is fine with having to pass one extra type argument to IDirective (i.e. the share of require), then we could avoid the "casting" by specifying a list of "known controllers" (for built-in directives) and allow the user to expand the list with their own directives.

The main benefits are that they don't have to cast at all when requiring built-in directives (and still get the correct types) and only having to define the mapping (directive name --> controller type) once for their custom directive.

By being more specific about the shape of your require property, you will also get better type-checking for your controllers argument (e.g. not only {[key: string]: IController} but {ngModel: INgModelController} which also checks the keys).

This is what I am talking about: poc

I could find a way to auto-infer the controller types for arrays, but we might be able to cover the most common cases (e.g. arrays of length 2, 3, 4). (For larger arrays, you would get the broader IController type.)

The main downside (that I can think of) is having to provide the extra type argument to IDirective.

[thorn0]

@gkalpak Keep in mind that the keys of the require object aren't (don't have to be) the same as the names of the required directives:

require: { myProp: 'ngModel', yourProp: '^^someOtherDirective' }

So I don't think anything can be done here.

link: (..., controller) => {
  const myController = controller as MyControllerType;
}

This (without the redundant type annotation) is indeed the right thing to do.

[gkalpak]

Nooo 😢 You ruined everything 😭
And apparently TypeScript doesn't allow (which wouldn't be 100% accurate anyway):

{
  [key in keyof T]: T[key] extends keyof IKnownControllers ?
    IKnownControllers[T[key]] :    <-- Error: "T[key] can't be used to index IKnownControllers" :(
    IController
}

We might still be able to use the approach for string and string[] (or pre-defined common lengths), but again there is not way to correctly type the directive API 😁
So, probably not worth trying too hard 😛