-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Infinity & NaN Type-level Support #51741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The TypeScript team hasn't accepted the linked issue #15135. If you can get it accepted, this PR will have a better chance of being reviewed. |
I don't think I can fix the linter though. |
b303aa1
to
47a33be
Compare
@weswigham do you remember the outcome of discussions when you experimented with this earlier? My memory is that we decided not to add these types but I don't remember the reason. I guess that the reason still holds, but it might not. |
To help with PR housekeeping, I'd like to close this PR unless @weswigham wants to advocate for the feature again. |
I would make it a draft for the time being. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally had NaN
and Infinity
types in early drafts of literal type PRs, so I'm biased, so I think this feature has a bit of value; however, as-is, this is a smidge incomplete. This adds the types and symbols, which is nice (and simplified our now-present syntactic NaN
checks, which is a nice bonus), but is missing the falsy behavior for NaN
which is the big sticking point in the whole thing. Once you correctly flag NaN
as falsy, every if (!someNumber)
in control flow needs to become 0 | NaN
, and that's truly painful to deal with because of how toxic NaN
is. And usually you don't even need to, since most people consider number
to mean only finite numbers, and NaN
and Infinity
aren't even considered as options. Now, does this mean they may have silent bugs? Yes. Maybe. But generally, people don't really want to add isFinite
checks everywhere.
IMO, I think the only way forward in that direction is with a new number
base type that explicitly includes the non-finite types. So number
today formally takes on the meaning of "finite numbers", and a hypothetical floating
(or whatever it's called) is number | Infinity | -Infinity | NaN
, so people can opt-in to the stricter checks. Now, that in turn has some DOM lib questions which'd need answering (are there any number
s that needs to become floating
, and will that extra strictness break people, does there need to be a flag?).... it's a bit of a rabbithole for very minimal benefit, since, in practice, NaN
and Infinity
don't usually come up in well-formed code, which is generally the goal.
Also notable: You can actually acquire an Infinity
type today already - type Infinity = 999999999999999999999999999999999999
does actually get you an Infinity
type (because numbers overflow). The global Infinity
is simply not typed that. This may void your warranty on declaration emit, however, since "literal types that print back as identifiers" is likely not something the emitter is pleased with, and both the builtin isFinite
guard doesn't guard it and control flow doesn't produce it from expression operations, which limits its use.
Anyways. Thoughts. I personally like the idea, in theory, but in practice, it's a bigger can of worms than is worth considering for pretty much no benefit. If someone came forward with all those solved, maybe we'd consider it, but even then I can't make any guarantees.
@@ -1456,6 +1457,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
const requireSymbol = createSymbol(SymbolFlags.Property, "require" as __String); | |||
const isolatedModulesLikeFlagName = compilerOptions.verbatimModuleSyntax ? "verbatimModuleSyntax" : "isolatedModules"; | |||
|
|||
const InfinitySymbol = createSymbol(SymbolFlags.Property, "Infinity" as __String); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer to not capitalize these, since they're not classes (even if it makes them a bit less consistent with their symbol names, it's fine).
@weswigham Would something like #28682 be considered? (I don’t think I am capable of handling it though) |
I think the toxicity of NaN should be separated from the need for Infinity as a literal type.
This may be true in most situations, but handling Infinity as a literal type is perfectly meaningful and critically important in some math-related codebases, especially after the introduction of bigint. Infinity is the only primitive that is guaranteed to be larger than any bigint (which is always "finite"), and many people are suffering from the lack of this support. See #32277 I personally consider it a bug that TypeScript still does not provide a way to handle a perfectly valid and well-defined member of |
Fix #15135
Fix #32277
Fix #42905
Related: #28682