feat(core): tighten the typings on Provider.provide #15340
Conversation
DzmitryShylovich
force-pushed
the
gh/15339
branch
2 times, most recently
from
dadcaef
to
ea13d7d
Compare
| */ | ||
| provide: any; | ||
| provide: string|Type<any>|InjectionToken<any>|Function; |
There was a problem hiding this comment.
After some digging I figured out that Type<any> doesn't match abstract classes (this is why I added Function as well. Didn't find anything better)
export interface Type<T> extends Function { new (...args: any[]): T; }
Abstract class can't be instantiated using new.
So get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T): T signature doesn't support abstract classes. is this intentional?
There was a problem hiding this comment.
Can you provide an example of how Type<any> does not match? Preferably on http://www.typescriptlang.org/play/
There was a problem hiding this comment.
There was a problem hiding this comment.
Can you provide an example of how Type does not match?
http://prntscr.com/en7kff
Plus CI failed on abstract classes like PlatformRef when i used
provide: string|Type<any>|InjectionToken<any>;
instead of
provide: string|Type<any>|InjectionToken<any>|Function;
There was a problem hiding this comment.
I get it now. MyAbstractClass is not matched by Type<any> because it does not have new ().
interface Type<T> {
new(...args:any[]):T // This is what is missing from abstract classes
}
I wonder if we can expand Type somehow to include abstract classes as well? Why is Type not just extends Function? (@tbosch do you know?)
There was a problem hiding this comment.
This is what is missing from abstract classes
yeap, we can't create an abstract class using new.
I wonder if we can expand Type somehow to include abstract classes as well?
I didn't find anything. Here's the same question on StackOverFlow http://stackoverflow.com/questions/36886082/abstract-constructor-type-in-typescript
Looks like we need to remove new(...args:any[]):T and just extend Function
DzmitryShylovich
force-pushed
the
gh/15339
branch
from
850435d
to
874d0b3
Compare
There was a problem hiding this comment.
this has to be split to the old API which still needs to be supported, and new api going forward.
The new API only allows Type and InjectableTokens. Old API supported any (not just string) So this has to be broken in a way so that if you using string than you gate a @deprecated warning where as when you use InjectableToken or Type you do not.
| */ | ||
| provide: any; | ||
| provide: string|Type<any>|InjectionToken<any>|Function; |
There was a problem hiding this comment.
Can you provide an example of how Type<any> does not match? Preferably on http://www.typescriptlang.org/play/
| */ | ||
| provide: any; | ||
| provide: string|Type<any>|InjectionToken<any>|Function; |
There was a problem hiding this comment.
chuckjaz
added
area: core
|
closing as outdated |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Closes #15339