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

const/non-const enum cleanup #2183

Closed
RyanCavanaugh opened this issue Mar 2, 2015 · 3 comments
Closed

const/non-const enum cleanup #2183

RyanCavanaugh opened this issue Mar 2, 2015 · 3 comments
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@RyanCavanaugh
Copy link
Member

Key points from discussion from this morning:

  1. Assume all non-ambient members of non-const enums are computed (even those initialized with literals), both for inlining purposes and for .d.ts generation purposes
  2. Continue to aggressively compute initializer expressions in both const and non-const enums
  3. Ambient non-const enums preserve existing behavior (computed iff there is no initializer)
  4. Continue to have --preserveConstEnums flag, which causes emit of the lookup object for const enums but does not change inlining

Tag @ahejlsberg

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Mar 2, 2015
@danquirk
Copy link
Member

danquirk commented Mar 2, 2015

Note this would likely be a breaking change

@RyanCavanaugh
Copy link
Member Author

A retelling of how we got here (note: using "1.0" here as a stand-in for all behavior prior to Vlad's proposed PR which does more aggressive initializer folding)

For regular enums, in 1.0, we would only inline constant members, and a member was only constant if its initializer was a literal. This led to a "workaround" (or "design feature", depending) that you could use an equivalent syntactic form to get a computed enum member, e.g. enum X { Y = (3) }. This is an important scenario for when you don't want to ship a .d.ts file that hardcodes in enum values because you intend to change those enum values in the future.

Vlad's change more aggressively resolves computational expressions in enum member initializers, so the member Y = (3) or Y = 3+0 now looks like a constant initializer rather than a computed one. This is an important breaking change for the aforementioned people who want to have a "constancy not guaranteed" .d.ts file, or who are doing wacky separate compilation (e.g. Monaco) and don't want referring implementation files to have their enum references inlined.

One proposed workaround was that you would initialize your "should not be inlined" enum members via a function call (e.g. enum X = Math.min(3)), but this is obviously weird.

The cleanest place to land is that we treat all non-const enum members as computed. This is a breaking change, but only in one scenario -- where a non-ambient enum is declared with constant members, consumed only at inlined sites, and the emitted JS (of the lookup object) is excluded from the script context. This scenario is easily fixed, though, because any enum fitting this profile can have the const keyword added to its declaration to force inlining at all consumption sites (which obviously must work, if all the non-const enum consumption sites were originally inlined).

@vladima vladima mentioned this issue Mar 4, 2015
@RyanCavanaugh RyanCavanaugh added Fixed A PR has been merged for this issue Committed The team has roadmapped this issue and removed In Discussion Not yet reached consensus labels Mar 19, 2015
@mhegazy mhegazy added this to the TypeScript 1.5 milestone Mar 19, 2015
@RyanCavanaugh
Copy link
Member Author

Ref #2594

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants