Skip to content
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

Enum with special numbers overwrites own value #48956

Open
JacobLey opened this issue May 4, 2022 · 4 comments · Fixed by #56161
Open

Enum with special numbers overwrites own value #48956

JacobLey opened this issue May 4, 2022 · 4 comments · Fixed by #56161
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript

Comments

@JacobLey
Copy link

JacobLey commented May 4, 2022

Bug Report

🔎 Search Terms

Enum number computed overwrite Infinity NaN

🕗 Version & Regression Information

I noticed on 4.6.2, but using TS Playground am able to reproduce on earliest possible version (3.3.3). Also still occurs on nightly build.

⏯ Playground Link

Playground link with relevant code

💻 Code

enum Computed {
  Infinity = Infinity,
  NaN = NaN
}

🙁 Actual behavior

Typescript generates the following code for that enum using a "lookup object":

var Computed;
(function (Computed) {
    Computed[Computed["Infinity"] = Infinity] = "Infinity";
    Computed[Computed["NaN"] = NaN] = "NaN";
})(Computed || (Computed = {}));

As a result, the Computed enum actually has values of Strings, rather than the "computed" numbers as expected

console.log(Computed);
// { Infinity: 'Infinity', NaN: 'NaN' }

Note the "lookup object" behavior itself is not unexpected, but the side effect of overwriting keys is unexpected.

🙂 Expected behavior

Typescript should not overwrite an existing key with a new value. Most likely that would mean disabling the "lookup values" for computed.

console.log(Computed);
// { Infinity: Infinity, NaN: NaN }

Extra Discussion

This does not impact const enum because values like Infinity are considered "computed".

It is also possible to do something like

enum Computed {
  '[object Object]' = {},
  null = null
}

with similar results.
The latest versions of typescript warn at this behavior, but it is possible to compile and I don't think it warns on all versions. But I omitted from core issue for that reason.

Similarly

enum Overwrite {
  123 = 'FOO',
  BAR = 123,
}

has similar behavior (when compiled) but TS does warn about numeric names (I believe this warning occurs in all versions).

It does not occur when using string enums, as those never write "lookup objects"

enum NoOverwrite {
  FOO = 'SOME VAL',
  BAR = 'FOO',
  BAZ = 123
}
console.log(NoOverwrite);
// { 123: "BAZ", FOO: "SOME VAL", BAR: "FOO", BAZ: 123 }

Effectively this issue happens whenever the String(val) (generally .toString() internally) returns a key in the Enum.

Possible solutions:

  • Don't emit "lookup object"
    • I'm going to assume the lookup object behavior is intentional/beneficial for literal numbers, but perhaps not for computed... Although values like 1 + 2 are "computed" but are resolved at compile time. So maybe "computed" where the value is anything but a number literal.
  • Forbid special "numeric" names
    • Don't allow NaN, Infinity or -Infinity as an enum key name. There may be others I am not aware of right now... but I was not able to reproduce error with exponential notation like Number.EPSILON ("An enum member cannot have a numeric name.")
  • Don't allow computed values that are non numeric literals
    • Throw compile-time error when value is something like Infinity, but allow literals.
      enum Allowed {
          FOO = 1 + 2
      }
      enum NotAllowed {
          BAR = 1 / 0
      }
@fatcerberus
Copy link

fatcerberus commented May 4, 2022

Interestingly, the following is an error, likely because it would otherwise suffer from the same issue:

enum Fooey { "0", "1", "2" };
// TS2452 - An enum member cannot have a numeric name.

edit: oops, you already mentioned this

@jcalz
Copy link
Contributor

jcalz commented May 4, 2022

Wow, I love this edge case.

FYI, what you're calling "lookup" is known as a reverse mapping.

@JacobLey
Copy link
Author

JacobLey commented May 4, 2022

FYI, what you're calling "lookup" is known as a reverse mapping.

Woops yes wrong terminology on my part. I was getting the terminology from this SO post https://stackoverflow.com/questions/28818849/how-do-the-different-enum-variants-work-in-typescript

@jakebailey
Copy link
Member

#56161 has been reverted.

@jakebailey jakebailey reopened this Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants