Make getTypeOfSymbol, isArrayType, isTupleType public on TypeChecker#52467
Make getTypeOfSymbol, isArrayType, isTupleType public on TypeChecker#52467jakebailey merged 3 commits intomicrosoft:mainfrom
Conversation
|
Yes this is great, thank you for looking at exposing these! We feel just as bad about using these ...can you do |
src/compiler/types.ts
Outdated
| /** @internal */ isTupleType(type: Type): boolean; | ||
| isArrayType(type: Type): boolean; | ||
| isTupleType(type: Type): boolean; | ||
| /** @internal */ isArrayLikeType(type: Type): boolean; |
There was a problem hiding this comment.
(Maybe we should export
isArrayLikeTypeand invent aisTupleLikeTypeinstead?)
I'm not sure we have a need need for an isTupleLikeType on our end. We have very little logic that deals with tuples [Sourcegraph query for isTupleType in typescript-eslint]. Do you see a reason why we'd want that?
There was a problem hiding this comment.
The main "problem" with these functions (that we have to document in this PR) is that they don't return true for things that extend from Array/tuple, e.g. TS's own NodeArray probably won't return true even though it is an Array.
There was a problem hiding this comment.
Maybe that doesn't happen for tuples, though.
There was a problem hiding this comment.
How much code actively extends tuple though? Is it a pattern people even really use?
Seems like it would be a suuuper niche usecase to do type T = [1,2,3] & SomeType (I've never seen it!).
Tuples themselves are already pretty niche, so it's going to be so rare that people do the above, probably.
As josh said - we barely touch on tuples in our rules, and even in the broader OSS ecosystem there's only 70 usages in sourcegraph..
Probably not worth it TBH, considering you'd need to build it!
There was a problem hiding this comment.
I mean specifically like:
type SomeTuple = [..., ..., ...]
interface SomeInterface extends SomeTuple {
someExtraInfo: ...;
}
Which is analogous to our NodeArray type for a fixed size array.
But yes, it's not a blocker for this PR, I just was hoping to have it because it rounds out isArrayType+isArrayLikeType with isTupleType+isTupleLikeType.
That is of course #9879 which I personally think we should address (but is it of course larger than these pretty-safe ones). |
|
I have added descriptions for these extra functions (besides getTypeOfSymbol which I don't think needs one); this should be ready for review. I have opted to not also export |
bradzacher
left a comment
There was a problem hiding this comment.
I love this and I greatly appreciate you've clarified the caveats for the methods. In the past the explanations have often been just "they're not safe for all cases" - so the explicit documentation goes a long way to making the checker API more accessible to those that don't know the checker internals.
also doing some searching: fixes #37711
Fixes #37711
These are APIs which are used by typescript-eslint and appear to be generally useful: https://github.com/typescript-eslint/typescript-eslint/blob/09c04fb0d5a5dee0dc266abb2e21aa9a1c489712/packages/type-utils/typings/typescript.d.ts
By exposing
getTypeOfSymbol, we unlock a whole slew of uses. For example, the internal functiongetTypeOfPropertyOfTypecan be written externally using already public functions:Draft for now because
isArrayTypeandisTupleTypeneed comments describing their caveats, and/or we should export versions which handle subtypes of these. (Maybe we should exportisArrayLikeTypeand invent aisTupleLikeTypeinstead?)/cc @JoshuaKGoldberg @bradzacher