Repository navigation
TypeScript 2.4 weak types produces surprising Angular1 error #16536
Description
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?
Activity
mhegazy commentedon Jun 14, 2017
Demois notIControllerunder the weak type check. it does not implement any of theIControlleroptional members, and it has other members of itself. you would have seen the same error if you had classDemoimplementsIContoller.there is some relevant discussion about this in #16047 (comment)
Adding an index signature on
IControllershould 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 commentedon Jun 15, 2017
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)$onInitmethod?mhegazy commentedon Jun 15, 2017
implementsdoes not change the type. it just checks that the class is assignable to the interface.evmar commentedon Jun 15, 2017
Right, but that would a user-comprehensible place for us to recommend a fix. When they try adding
implementsthey'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 IControllerat the use site? Or would that hide other errors?mhegazy commentedon Jun 15, 2017
makes sense
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.that would make the error go away too, but you do not want to do that every component.
evmar commentedon Jun 15, 2017
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 commentedon Jun 19, 2017
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 commentedon Jun 22, 2017
The Definitely Typed d.ts for angular should be fixed now. Evan Martin (@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 commentedon Nov 9, 2017
Ryan Elian (@ryanelian) - your suggestion worked. I was able to do the following to get it working:
abobwhite commentedon Nov 14, 2017
Why the heck would it be an OK solution to require to write an empty
$onInitfunction when a controller is not used as a component or is a component and doesn't need that hook implemented!? So dumb...