Disallow Infinity, -Infinity and NaN as an enum key name

[magic-akari]

• Fixes Enum with special numbers overwrites own value #48956

[jakebailey]

@typescript-bot test top200
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot pack this

[typescript-bot]

Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 71e0db6. You can monitor the build here.

Update: The results are in!

[typescript-bot]

Heya @jakebailey, I've started to run the tarball bundle task on this PR at 71e0db6. You can monitor the build here.

[typescript-bot]

Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 71e0db6. You can monitor the build here.

Update: The results are in!

[typescript-bot]

Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 71e0db6. You can monitor the build here.

Update: The results are in!

[typescript-bot]

Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/158287/artifacts?artifactName=tgz&fileId=1C5C846517B607217AB45065CB8E2CCDCF7096C077DAD97509D5CAC7C7684CCE02&fileName=/typescript-5.3.0-insiders.20231020.tgz"
    }
}

and then running npm install.

---

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.3.0-pr-56161-5".;

[typescript-bot]

@jakebailey Here are the results of running the user test suite comparing main and refs/pull/56161/merge:

There were infrastructure failures potentially unrelated to your change:

• 3 instances of "Package install failed"
• 1 instance of "Unknown failure"

Otherwise...

Everything looks good!

[typescript-bot]

Hey @jakebailey, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

[typescript-bot]

@jakebailey Here are the results of running the top-repos suite comparing main and refs/pull/56161/merge:

Everything looks good!

[jakebailey]

I can't recall; was banning this actually something we wanted? Can't remember what we said in the design meeting, or maybe we didn't talk about it at all?

[sandersn]

My memory is that we briefly discussed this and said "yes of course you can't do that because of the lookup object", but I could be wrong.

We did discuss Infinity types a couple of weeks after your comment, but that's the only mention of Infinity and NaN in the design notes. And then a couple of days after that, we discussed enum assignability. So it sounds like my memory smudged the two discussions together and in fact we never did.

I still think this is a good thing to forbid since the emit is bad and it's unlikely to be something people want to write.

[jakebailey]

My impression was that this PR was good, as they're not good enum names, but that the recent infinity chat was about the whole 1e999 thing allowing a half-functional backdoor into gaining an infinity type, which we banned but then backed out and decided that we were instead going to look into introducing those literal types into the type system instead.

[sandersn]

Right, and since enum members introduce values as well as types, I think that we'll still want to ban these names because of their value problems. So...I think it's good to merge? Pretty sure.

[jakebailey]

Yep.

[JoostK]

@DanielRosenwasser This change is missing from the breaking changes section in the the blog post, perhaps it can be included.

[DanielRosenwasser]

Even if this ends up being the desired behavior - I'm surprised we merged this because the logic was explicitly there, and was added 8ish years ago

• const enum should allow "Infinity" as a valid name #6153
• Suggestion Backlog Slog, 1/4/2016 #6347
• Allow Infinity and NaN to be used as an Enum property identifier #10749

Given that there's no linked design meeting notes, in the future let's have a more intentional discussion before merging a change like this.