TypeScript 2.4 weak types produces surprising Angular1 error

[evmar]

With TypeScript 2.4 a bunch of our Angular1 code produces a new error.

I reduced the code to something like the following. (The interfaces/types here are from the Angular1 d.ts on DefinitelyTyped, but I simplified them a bit to make the error more obvious.)

interface IController {
  $onInit?(): void;
}

type IControllerConstructor = (new (...args: any[]) => IController);

type Injectable<T> = T|Array<string|T>;

interface IDirective {
  controller?: string|Injectable<IControllerConstructor>;
}

class Demo {
  data: string;
};

function demo(): IDirective {
  return {
    controller: Demo,
  }
}

TypeScript 2.3 accepts this code, while 2.4 produces this error:

Type '{ controller: typeof Demo; }' is not assignable to type 'IDirective'.
  Types of property 'controller' are incompatible.
    Type 'typeof Demo' is not assignable to type 'string | (new (...args: any[]) => IController) | (string | (new (...args: any[]) => IController))...'.
      Type 'typeof Demo' is not assignable to type '(string | (new (...args: any[]) => IController))[]'.
        Property 'push' is missing in type 'typeof Demo'.

The error message is quite a surprise! I believe the underlying issue is that the "Demo" class is not seen to implement the IController interface anymore due to weak types, so TS picks the other union element in Injectable (the array).

I'm not sure how this can be improved, but perhaps we can think about the error?

[mhegazy]

Demo is not IController under the weak type check. it does not implement any of the IController optional members, and it has other members of itself. you would have seen the same error if you had class Demo implements IContoller.

export class Demo implements IController {
    data: string;
};

there is some relevant discussion about this in #16047 (comment)

Adding an index signature on IController should be sufficient allow the assignment.

the error message happens to be on the last constituent of the union type, here Array<T | string>, it is not possible for the compiler to know which branch was more useful in these cases..

[evmar]

Do you recommend we add an index signature to the d.ts files on DefinitelyTyped? It's a bit weird because it's a lie, but I'm not sure how else to work around this. Perhaps we could require users include the implements ... by implementing an (empty) $onInit method?

[mhegazy]

implements does not change the type. it just checks that the class is assignable to the interface.

[evmar]

Right, but that would a user-comprehensible place for us to recommend a fix. When they try adding implements they'd get an error about how they don't implement the interface, and fixing that (by satisfying the interface) it would fix this error.

Otherwise, I'm not sure what the workaround should be -- maybe some Demo as IController at the use site? Or would that hide other errors?

[mhegazy]

Right, but that would a user-comprehensible place for us to recommend a fix.

makes sense

and fixing that (by satisfying the interface) it would fix this error.

in this case, assuming i am understating the API correctly, the interface is meant to be weak. so not all users are expected to implement $onInit. so the fix would be adding an index signature on the type in DT.

Otherwise, I'm not sure what the workaround should be -- maybe some Demo as IController at the use site? Or would that hide other errors?

that would make the error go away too, but you do not want to do that every component.

[evmar]

I see. So is "add an index signature" the general pattern for forcing an interface back to being weak? We might need to sprinkle these around in various other places that use a similar pattern.

[ryanelian]

Just chiming in, the change was surprising for me too, but I managed to fix my project failing to build on TS 2.4.0 by sprinkling empty $onInit() for all my Controllers without $onInit().

[sandersn]

The Definitely Typed d.ts for angular should be fixed now. @evmar "add an index signature" is the easiest solution for library authors to use. If you are using a library and normally want the weak type check, you can cast to the weak type to get an exception.

[AllenConway]

@ryanelian - your suggestion worked. I was able to do the following to get it working:

export function MyDirective():ng.IDirective {
    return {
        templateUrl :  "my-directive.html",
        replace: true,
        restrict: 'E',
        controller: MyController
    }
}
export class MyController implements IController {
    //...additional controller code

    $onInit(){
    };
}

[abobwhite]

Why the heck would it be an OK solution to require to write an empty $onInit function when a controller is not used as a component or is a component and doesn't need that hook implemented!? So dumb...