Declare non-const enums if preserveConstEnums is true

[jablko]

Currently if you tsc --declaration --isolatedModules, the compiler turns non-ambient const enums (.ts files) into ambient const enums (.d.ts files) that can't themselves be consumed with --isolatedModules: Ambient const enums (values -- types are fine) are incompatible with isolatedModules.

In every other way tsc --isolatedModules ignores the const modifier -- treats const enums as non-const:

• Emits .js values (like preserveConstEnums)
• Doesn't inline values and doesn't elide imports (unlike preserveConstEnums)

This PR doesn't emit the const modifier if isolatedModules is true, effectively suppressing the const modifier entirely in that case. tsc --declaration --isolatedModules then turns non-ambient const enums into ambient non-const enums which can in turn be consumed with --isolatedModules: #37774 (comment)

The affected scenario is when you:

1. Produce .d.ts with isolatedModules: true (currently: const modifiers preserved, this PR: const modifiers dropped)
2. Consume the produced .d.ts:
   • With isolatedModules: true: Currently: error. This PR: no error, values and imports preserved, same as all enums, (non-ambient) const enums included.
   • With isolatedModules: false: Currently: values inlined, imports elided. This PR: same as with isolatedModules: true (values and imports preserved).

When a .d.ts is produced with isolatedModules: true and consumed with isolatedModules: false, what's the point of inlining values and eliding imports? If you consume any part of the produced library that itself uses the const enum values, you'll end up importing the enum's .js values.

So this PR will only introduce imports that don't currently exist if you publish a library that declares const enum values it doesn't itself use, and in that case if you really want to publish isolatedModules-incompatible ambient const enums, produce its .d.ts with isolatedModules: false.

Edit: Inlining enums from other projects comes with pitfalls:

2. Consumers can inline enum values from upstream version A at compile time and import version B at runtime, with different values, if you're not very careful:
   • Pain points with const enum #37179 (comment)
   • Remove const enums from API #5297 (comment)
   • Const enums in the TS Compiler API can make depending on typescript difficult #5219 (comment)

Roughly this PR propagates today's isolatedModules const enum behavior transitively to .d.ts consumers.

Fixes #37774

[typescript-bot]

The TypeScript team hasn't accepted the linked issue #37774. If you can get it accepted, this PR will have a better chance of being reviewed.

[jablko]

Alternatively this could be done when parsing vs. emitting const enums?

[jablko]

This doesn't fix #37774 if isolatedModules: false, preserveConstEnums: true. The advantage of publishing with isolatedModules: false, preserveConstEnums: true is that you can consume parts of the produced library that themselves use const enum values, without importing the enum's .js values ...

[sandersn]

@jablko is this a proof of concept? When I skimmed the issue I didn't see that it had arrived at a single solution yet.

[jablko]

It's a proposed solution: #37774 (comment), but you're right: It's awaiting feedback (issue and proposal).

I think it's right because:

1. Publishing ambient const enums is a current problem:
   • Create React App enforces isolatedModules: true (reportedly?)
   • Pain points with const enum #37179 (comment)
2. Deconstifying .d.ts files in a build step requires author buyin. The only other mitigation today is to avoid const enums entirely, which has performance implications (though most often small?).
3. While two out of three ambient const enum problems have proposed remedies (a preserved modifier would address isolatedModules incompatibility, Allow inlined-value usage of type-only const enums #46556 (comment) addresses import elision), there remains a compile time vs. runtime pitfall to inlining inter-project enums (ambient const enums, roughly).

∴ tsc --declaration --preserveConstEnums should automatically not transform const enums -> ambient const enums (deconstify ambient enums), I think.

• The cases where you want to inline inter-project enums (with consequent pitfall) are already addressed by preserveConstEnums: false.
• The cases where you want some consumers to inline enums and others not to are:
  1. More confusing than useful?
  2. Doable anyway in a build step, if you know what you're doing (like deconstification is today)?

The difference between this PR and a preserved modifier is that the modifier would presumably inline preserved inter-project const enums if isolatedModules is false. This PR is simpler (no new kind of enum) and desirable in the majority of and the entry-level cases (avoids pitfall, little or no downside?).

[sandersn]

Based on the discussion at the December 1st design meeting, we aren't convinced that improving const enums is worthwhile, since people should be using the feature less over time rather than more.

[jablko]

All of the const enum pitfalls remain if you don't use preserveConstEnums: true, which should discourage people (as much as today)? 😏

• Avoiding const enums is preferable, but the number of people who should truly use them isn't zero.
• In those few cases, the current workaround (a build step) isn't onerous, but this change is commensurately small, and strictly positive, I think?

[jablko]

I've added a commit to inline plain enums from referenced projects.

Currently you get different behavior consuming .ts files and consuming the .d.ts outputs of those same files: const enums in .ts files are (ordinarily) compatible with --isolatedModules (because it flips on --preserveConstEnums), whereas the .d.ts outputs aren't (ambient const enums are incompatible with single-file transpilation).

This came up again here. An example is you can't use const enums from referenced projects with --isolatedModules. You can use const enums from .ts files, but the CLI uses the .d.ts outputs for referenced projects. That error shows up in the CLI but not in the editor, which defaults to using .ts files.

This PR fixes that, but now you get different behavior between .ts files and their .d.ts outputs when built with --preserveConstEnums and consumed without --isolatedModules: const enums in .ts files are inlined whereas their plain enum .d.ts outputs aren't. The latest commit addresses that.

• With --isolatedModules, --preserveConstEnums and deconstification solve single-file transpilation compatibility by emitting plain enums. --isolatedModules already treats const enums as plain enums anyway (doesn't inline).
• Without --isolatedModules it solves other problems with publishing ambient const enums: Pain points with const enum #37179 (comment) (JS consumers, compile-time vs. runtime version compatibility).

Not inlining enums from referenced projects could be a problem, however. The latest commit solves that by inlining plain enums. Inlining all enums would open up compile-time vs. runtime version compatibility with published .d.ts outputs, but inlining plain enums from referenced projects is safe because they're part of the same build.

The logic is that we inline const enums from .ts files, we use deconstified .d.ts outputs for referenced projects, so inline plain enums from referenced projects.

[sandersn]

To help with PR housekeeping, I'm going to close this PR while it's still waiting on its bug to be accepted.