AngularJs TypeScript typings and --strictFunctionTypes #16617

rkirov · 2018-06-28T00:25:50Z

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.

The text was updated successfully, but these errors were encountered:

This allows users to write callbacks using those aliases instead of the full type. Having convinient aliases is required for having AngularJS in TS work better with strictFunctionTypes flag on. see: angular/angular.js#16617 Also having them next to each other shows the symmetry between the require and controller param better.

…ype. This allows users to write callbacks using those aliases instead of the full type. Having convinient aliases is required for having AngularJS in TS work better with strictFunctionTypes flag on. see: angular/angular.js#16617 Also having them next to each other shows the symmetry between the require and controller param better.

gkalpak · 2018-06-28T09:55:11Z

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 · 2018-06-28T10:57:28Z

@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 · 2018-06-28T14:29:50Z

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 😛

rkirov mentioned this issue Jun 28, 2018

(angular.js) Introduce type aliases LinkFnControllerType and RequireT… DefinitelyTyped/DefinitelyTyped#26897

Closed

9 tasks

gkalpak added this to the 1.7.x - won't fix milestone Sep 26, 2018

AngularJs TypeScript typings and --strictFunctionTypes #16617

AngularJs TypeScript typings and --strictFunctionTypes #16617

rkirov commented Jun 28, 2018 •

edited

gkalpak commented Jun 28, 2018

thorn0 commented Jun 28, 2018

gkalpak commented Jun 28, 2018

AngularJs TypeScript typings and --strictFunctionTypes #16617

AngularJs TypeScript typings and --strictFunctionTypes #16617

Comments

rkirov commented Jun 28, 2018 • edited

gkalpak commented Jun 28, 2018

thorn0 commented Jun 28, 2018

gkalpak commented Jun 28, 2018

rkirov commented Jun 28, 2018 •

edited