Remove const enums from API

[weswigham]

Adds a compiler flag to emit const enums as normal enums in the declaration emit (requiring that they be preserved in the js emit), then enables it on the compiler.

Enabling the flag on the compiler itself required an LKG update - There's another LKG update (without this flag) pending at #5293.

Fixes #5219.

[DanielRosenwasser]

👍 provided we want this flag.

[sandersn]

Typo: declarations

[sandersn]

I can't come up with any better option names given the current command line parser design. If we ever have a chance to redo the command line parser design, I'd like to see something like

--constEnum=
  emit -- emit code for const enums but do not use the emitted code anywhere
  preserve -- emit and preserve enum references for consumers of this compilation's d.ts.
  inline -- inline const enums everywhere [default]

Then --constEnum=emit could be the equivalent of today's --preserveConstEnum.

[sandersn]

👍

[RyanCavanaugh]

Very hesitant on having a flag like this vs just a post-build script that uses the AST to strip out the const keyword. Let's discuss at the design meeting.

[mhegazy]

i agree with @RyanCavanaugh i do not think we need another flag. If you really want this behavior just do not use const enums.

[weswigham]

@mhegazy should we stop using const enums in the compiler?

[mhegazy]

@mhegazy should we stop using const enums in the compiler?

i did not say that :). we have two scenarios that are a bit contradicting, one in the compiler, we want to avoid the perf hit from the extra property lookup, and in the public API, we would like consumers to use the enum.

For our use cases, I think a post build step to clean up the .d.ts is better than adding yet another enum variant in the language.

[weswigham]

Alright, though by doing so we acknowledge it's a shortcoming in our compiler's declaration emit which is meant to be worked around with an external tool - I don't doubt that other libraries may have this issue and will have to have similar postbuild declaration processing.

[mhegazy]

I am not sure where the shortcoming is to be frank. but there are already too many enum concepts out there that necessary.

[weswigham]

I am not sure where the shortcoming is to be frank.

It's in how we do const inlining and then expose that const inlining was done via dts. Const enums work well in a big-bang ts build, where all of your ts deps get built at the same time against one another. When versions start to drift between compile time and build time, you lose the ability to safely inline const enums, since you can't guarantee that the library A you build your TS against is the library A you're running against at runtime (because the JS world is one of late binding).

For performance reasons, people enjoy const inlining (as we do), and so we have const enums, but for library purposes, you can't safely use const enums in your APIs (without monitoring them for even the smallest changes, so you know to increment a semver change indicative of the break). Part of me doubts we'll be the only TS library that will ever have this issue, and should others have it we'll not be the only ones who will want to solve it.

But I suppose it can always be revisited later, if/when other people bring it up, so it doesn't really matter for now.