Expose getStringLiteralType and getNumberLiteralType on the TypeChecker, plus remove /** @internal */ from several useful methods.

[sstchur]

Expose the following on the TypeChecker:

• getStringType
• getStringLiteralType
• getNumberType
• getNumberLiteralType
• getBooleanType
• getFalseType
• getTrueType
• getUndefinedType
• getNullType
• isTypeAssignableTo

Fixes #50694

I opened a suggestion issue #50694 to suggest exposing getStringLiteralType and getNumberLiteralType. The rationale was that they would help avoid questionable and hacky code in an ES-Lint rule I was working on and that they would be useful to other developers using the TypeChecker. All of the other methods were technically available on the TypeChecker, but not "visibile" due to having been marked /** @internal */. This PR exposes the two recommended in #50694 and removes the Internal designation on several others that are useful for developers leveraging the TypeChecker.

@andrewbranch suggested that I create the PR myself, so I did :-)

I read the Contributing.md, and I'm aware of the following:

Features (things that add new or improved functionality to TypeScript) may be accepted, but will need to first be approved (labelled "help wanted" or in the "Backlog" milestone) by a TypeScript project maintainer in the suggestion issue. Features with language design impact, or that are adequately satisfied with external tools, will not be accepted."

And I'm also aware that I don't have such a label from a TS project maintainer, but at Andrew's suggestion, I have created the PR anyway. Please let me know if there is anything else I can do to make the PR better or the review process easier.

[typescript-bot]

The TypeScript team hasn't accepted the linked issue #50694. If you can get it accepted, this PR will have a better chance of being reviewed.

[jakebailey]

See also #9879 (since this PR seems to contain a partial export of what that proposal suggests should be exported). @weswigham

[sstchur]

See also #9879 (since this PR seems to contain a partial export of what that proposal suggests should be exported). @weswigham

If there is broad agreement on the TS team that everything in that proposal should be exposed, I could take a crack at handling all of it. LMK.

[jakebailey]

If there is broad agreement on the TS team that everything in that proposal should be exposed, I could take a crack at handling all of it. LMK.

I'm not aware of any quite yet, but it did come up today in the context of #52467 and #52470.

[sstchur]

So is what I'm hearing a request for me to:

• remove the change that exposes isTypeAssignableTo
• include change that exposes any, void, never, and symbol

Anything else?

And I see "Changes requested" and along with that "All checks have passed." Does that mean someone already suggested the changes, or do I still need to make them? Sorry, not too familiar with this Github UI

[jakebailey]

So is what I'm hearing a request for me to:

• remove the change that exposes isTypeAssignableTo
• include change that exposes any, void, never, and symbol

Anything else?

Nope, that sounds good.

And I see "Changes requested" and along with that "All checks have passed." Does that mean someone already suggested the changes, or do I still need to make them? Sorry, not too familiar with this Github UI

"Changes requested" is a review status, it just means that someone has an objection. Specific comments may include suggested code but that wouldn't really work for this kind of PR. "All checks have passed" is just that this change passed CI (which could be true of any PR even if we didn't want it).

You have to make the changes yourself, i.e. remove @internal from any/void/never/symbol and then restore the comment for isAssignableTo.

[sstchur]

Perfect, thanks for clarifying! I'll get it done soon.

[jakebailey]

Rereading this again, do we want to expose the fresh parameter? Or should we maybe overload it and only expose one without parameters publicly?

[sstchur]

What does the fresh param do? In my personal needs using this, I'm just invoking getFalseType(). How likely is it that people using this would need/want to pass in the fresh arg?

[andrewbranch]

Literal types have fresh and regular variants. When assigning from a fresh literal to a mutable location, the resulting type is widened to the base type:

const freshFalse = false;
const regularFalse: false = false;

let widenedToBoolean = freshFalse;
let stillFalse = regularFalse;

from an API perspective, this means it’s important to note that

checker.getFalseType(true) !== checker.getFalseType(false)

but that a false type will always be one of these. Within the checker, we would probably never do an equality comparison to determine if a type is false; we’d call getTypeFacts and look at those, but that’s not exported. Soooo, whether to expose it, 🤷‍♂️?

[jakebailey]

Lots of places appear to do things like foo === falseType || foo === freshFalseType too. I guess it depends, but it might be important if you are trying to do something with these functions...

Alternatively what people want externally is "is this assignable to false" but obviously that API is a different issue so...

[sstchur]

Interesting. So in my case, I have checker.getFalseType(false) (since I don't pass anything in). And I'm later using that, but not with ===. Rather, I'm using it with isTypeAssignableTo, because what I desire to know is if some type I have could accept the value false.

For example:

const foo: false | number = getRandomNumberOrMaybeFalse();

Later, I want to know if foo would accept false. I don't want or need to know if foo would accept boolean, only if it will accept false.

But I have to admit that I don't know which of the two: checker.getFalseType(true) or checker.getFalseType(false) I want in this case, or if it even matters?

And the whole reason I mention my use-case at all is to try to add insight that might help decide if we need to expose the freshType arg or not? I can't purport to know what others might be doing with getFalseType or if their use-cases are at all like mine.

[andrewbranch]

Fresh/regular shouldn’t matter for assignability, so it sounds like in your case it doesn’t matter.

Noticing that a couple of our codefixes grab a type, check its flags for BooleanLiteral, and then compare to checker.getFalseType(true) and checker.getFalseType(false), I’m inclined to say it’s fine to leave it exposed. Codemods are a good example of a reasonable usage of our API.

[sstchur]

Obviously the value of my input is limited, since I don't have the same depth on knowledge about TS that you guys have, but my intuition would be to leave it exposed. As a developer, I prefer having flexibility available to me, even if I don't use it, so long as that flexibility doesn't make my life difficult. And in this case, it doesn't seem to, so I say leave it.

[jakebailey]

Note that there are now conflicts, probably because I merged another PR that exposed more things in the checker. You should be able to merge from main, ignore the conflicts, then rerun tests and accept baselines to resolve the conflicts.

[andrewbranch]

I’ll do it real quick

[andrewbranch]

My default setting of --no-typecheck preventing me from actually checking in updated API baselines strikes again

[sstchur]

I’ll do it real quick

Thanks!

[andrewbranch]

I made the changes we decided on in #52790, and added JSDoc on all the types that include multiple versions of the intrinsic warning against using equality checks. It sounded like we were fine to accept this once those changes were made. Can I get a review and sign-off from anyone?