Skip to content

A flag to disable reverse mapping in number enums #57134

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

Closed
6 tasks done
y-nk opened this issue Jan 23, 2024 · 10 comments
Closed
6 tasks done

A flag to disable reverse mapping in number enums #57134

y-nk opened this issue Jan 23, 2024 · 10 comments

Comments

@y-nk
Copy link

y-nk commented Jan 23, 2024

πŸ” Search Terms

"number" "enum" "Object.values"

βœ… Viability Checklist

⭐ Suggestion

A simple configuration flag to disable explicitly reverse mapping for enums (#30487)

enum Foobar {
  FOO,
  BAR,
}

console.log(Object.keys(Foobar)) // ["0", "1", "FOO", "BAR"] 
console.log(Object.values(Foobar)) // ["FOO", "BAR", 0, 1] 

While this is intended behavior, it can be found as unnatural behavior since string enums don't behave the same way. I don't mind the current behavior, but having a flag to align both number and string enums would make sense, at least to me.

πŸ“ƒ Motivating Example

enum Foobar {
  FOO,
  BAR,
}

function isFoobar(arg: any): arg is Foobar {
  return Object.values(Foobar).includes(arg)
}

console.log(isFoobar(Foobar.FOO)) // ok
console.log(isFoobar('FOO')) // should not pass, only 0 & 1 (as number) should be allowed

playground link

πŸ’» Use Cases

  1. What do you want to use this for?
    writing typeguards in the same way for string and number enums

  2. What shortcomings exist with current approaches?
    there's a need for a filtering pass on enum set

  3. What workarounds are you using in the meantime?

// inefficient but clean
function isFoobar(arg: any): arg is Foobar {
  return Object
    .values(Foobar)
    .filter((v: any) => !isNaN(v))
    .includes(arg)
}

// efficient but amazingly ugly
function cleanEnum(enumType: any) {
  Object.entries(enumType)
    .forEach(([k, v]: [any, any]) => {
      if (isNaN(v)) delete enumType[k]
    })
}

playground link

Other considerations

Maybe another approach would be to have a different way of writing enums so that they match instanceof but i can't think of anything which is not ugly and hacky.

Or since enums emit JS code, have their own .is() type guard function generated would be nice too.

@fatcerberus
Copy link

Such a flag might be problematic; suppose a .d.ts file for some library contains

declare enum FooBar {
    Foo,
    Bar,
}

Now further suppose the library writer enabled noEnumReverseMappings when compiling this library, but the user of the library didn't. The type checker would then allow reverse lookups on this enum, only for the app to crash because the entries don't actually exist at runtime...

@fatcerberus
Copy link

it can be found as unnatural behavior since string enums don't behave the same way

For the record, the only reason string enums don't get reverse mappings is because they are vulnerable to collisions whereas numeric enums aren't.

@y-nk
Copy link
Author

y-nk commented Jan 23, 2024

i believe that's not really related to my proposal.

i understand that enums are the only things emitting JS (in that sense you're right) but it's not my proposal which will solve the issue of mismatch of tsconfig between built lib and source.

for example, i wonder if - to some extend - a library with types compiled with noExplicitAny: false should be valid to be used in a project where noExplicitAny: true is set.

@fatcerberus
Copy link

i understand that enums are the only things emitting JS (in that sense you're right) but it's not my proposal which will solve the issue of mismatch of tsconfig between built lib and source.

Right, but I'm pointing it out because the potential for such mismatch to be a runtime footgun is very likely to be cited as a reason for the TS devs to reject the proposal.

@y-nk
Copy link
Author

y-nk commented Jan 23, 2024

For the record, the only reason string enums don't get reverse mappings is because they are vulnerable to collisions whereas numeric enums aren't.

this i know, but there are valid reasons to use number has enums. from storing in db, serialization concerns to magic calculations ; doesn't mean it should not be first class citizen

@MartinJohns
Copy link
Contributor

Essentially sounds like a duplicate of #44753 to me.

@fatcerberus
Copy link

And a maintainer already noticed the above issue too: #44753 (comment)

@y-nk
Copy link
Author

y-nk commented Jan 23, 2024

closed in favor of #37282 (still opened, #44753 is closed)

i still think it's a valid usecase, and while the reverse lookup may sound like a nice idea, it outputs invalid types and remain not on par with string enums which i think it's too subtle to be ok.

@y-nk y-nk closed this as completed Jan 23, 2024
@fatcerberus
Copy link

fatcerberus commented Jan 23, 2024

#37282 is very specifically about const enum, I don't think that's what you want. #44753 is the correct dupe, and it was closed because the devs have no intention of implementing it (due to the above-mentioned footgun). At any rate maintainers have stated that enum behavior is unlikely to be messed with at all due to there being an ES proposal to add enums to JS proper and they don't want to put in extra work when TS enums will eventually be deprecated in favor of native ones anyway.

@RyanCavanaugh
Copy link
Member

If you don't want the reverse lookup, there's not much reason left to write a numeric enum.

const MyEnum = {
  x: 1,
  y: 2
} as const;
type MyEnum = (typeof MyEnum)[keyof typeof MyEnum];

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants