Inline "our own" regular enums

[jablko]

I'm curious what the impact would be on the TS codebase, of inlining "our own" regular enums, i.e. all enums except those from other packages?

That's what I've done in the first commit. We could then, optionally:

• rewrite our const enums -> regular enums, without affecting inlining
• skip the --preserveConstEnums option (all const enums having been rewritten, regular enums are preserved anyway)
• drop the gulpfile lines that rewrite const enums -> regular enums in our published .d.ts files

That's what I've done in the second commit --- or we could omit that change if there's some remaining distinction/advantage to const enums, apart from inlining?

To inline all "our own" enums, I changed the inlining condition as follows:

                 const member = symbol.valueDeclaration as EnumMember;
-                if (isEnumConst(member.parent)) {
+                if (!(member.flags & NodeFlags.Ambient) || isEnumConst(member.parent) || host.getFileIncludeReasons().get(getSourceFileOfNode(member).fileName as never)?.some(reason => reason.kind === FileIncludeKind.OutputFromProjectReference)) {
                     return getEnumMemberValue(member);
                 }

i.e. inline enums if:

• they're not ambient: If they're not ambient then they're in .ts files, which are presumably part of the current build?
• they're const: Ambient const enums don't exist at runtime, so we must inline them, whether they're from the current build or published by another package. Same as now.
• they're ambient, regular, but from a project reference. Project references are part of the current build, but we use their .d.ts files, so treat those ambient enums like non-ambient ones, to avoid different ambient/non-ambient behavior.

The advantage of inlining "our own" regular enums is that you have the option of avoiding const enums, which prevent project references from being built with --isolatedModules. That's because project references use .d.ts files, --declaration turns non-ambient const enums into ambient const enums, and ambient const enums are incompatible with single-file transpilation (--isolatedModules).

[typescript-bot]

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @amcasey, @mjbvz, @minestarks for you. Feel free to loop in other consumers/maintainers if necessary

[typescript-bot]

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

[sandersn]

Can you open a bug for this instead?
But I don't think we're likely to do anything with enums that makes them emit more code, to be honest.

[sandersn]

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