Certain resolution-dependent enum emit isn't correctly flagged as an error under isolatedModules

[magic-akari]

🔎 Search Terms

• isolatedModules
• enum
• cross module
• export

🕗 Version & Regression Information

• This is a crash
• This changed between versions ______ and _______
• This changed in commit or PR _______
• This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________
• I was unable to test this on prior versions because _______

Test on Playground with version 3.9.7 and version 5.2.2, got the same incorrect result.

⏯ Playground Link

Workbench Repro

💻 Code

// @showEmit
// @isolatedModules: true
// @filename: foo.ts
export enum Foo {
    A = 10
}

// @filename: index.ts
import { Foo } from "./foo";
export enum Bar {
    B = Foo.A,
    C
}

Workbench Repro

🙁 Actual behavior

Please notice the value of Bar.C

import { Foo } from "./foo";
export var Bar;
(function (Bar) {
    Bar[Bar["B"] = 10] = "B";
    Bar[Bar["C"] = 11] = "C";
})(Bar || (Bar = {}));

🙂 Expected behavior

Emit a warning or an error, and generate code that does not rely on other imports.

Example:

import { Foo } from "./foo";
export var Bar;
(function(Bar) {
    Bar[Bar["B"] = Foo.A] = "B";
    Bar[Bar["C"] = void 0] = "C";
})(Bar || (Bar = {}));

The Bar.C is void 0, since it cannot be refered.

Additional information about the issue

• The type of imported TS file, with enumeration increasing to undefined swc-project/swc#8147

[fatcerberus]

Emit a warning or an error, and generate code that does not rely on other imports.

Just to be clear, isolatedModules doesn't mean "never emit code that relies on an import", it means "correct emit doesn't require resolving imports". This should probably still be an error under that reasoning though, since the emit requires the actual value of Foo.A.

[RyanCavanaugh]

isolatedModules means that a non-resolving transpiler can correctly emit any file in the project. Arguably, a non-resolving transpiler can correctly emit index.ts:

import { Foo } from "./foo";
export var Bar;
(function(Bar) {
    Bar[Bar["B"] = Foo.A] = "B";
    Bar[Bar["C"] = Foo.A + 1] = "C";
})(Bar || (Bar = {}));

So the bug here is either that:

• ts.transpileModule isn't emitting the addition expression
• or we say that transpilers shouldn't do the addition, and C should be an error

I'm leaning toward the first option for the sake of compat and convenience.

[magic-akari]

I don't believe Foo.A + 1 is correct. You can do this only if you know Foo.A is a number.

// @filename: foo.ts
export enum Foo{
    A = "10"
}

// @filename: index.ts
import { Foo } from "./foo";
export enum Bar {
    B = Foo.A,
    C
}

In the above example, tsc will return void 0 for Bar.C, while Foo.A + 1 will produce the string "101"

// @filename: foo.ts
export var Foo = {
    A: 10
};

// @filename: index.ts
import { Foo } from "./foo";
export enum Bar {
    B = Foo.A,
    C
}

In this example, Foo.A + 1 gets 11, while tsc returns the value of void 0.

[RyanCavanaugh]

C is an error, so its correct emit is not guaranteed. Correct transpilation (in the presence of no errors) a motivating scenario behind the breaking changes we made in #50528

[magic-akari]

C is an error, so its correct emit is not guaranteed. Correct transpilation (in the presence of no errors) a motivating scenario behind the breaking changes we made in #50528

You are right.
If people always handle errors and do not expect consistent results from erroneous inputs, then things would be much simpler.

However, as far as I know, there are still people who try to use any to bypass error checking, but expect to get the same results as the TypeScript compiler.

One possible input:

// @filename: foo.ts
export enum Foo {
    A = 10,
}
(Foo.A as any) = "10"

// @filename: index.ts
import { Foo } from "./foo";
export enum Bar {
    B = Foo.A,
    C
}

• Ability to disable TS enum value inlining? swc-project/swc#8114

[magic-akari]

CC @nicolo-ribaudo @evanw

Maybe same issue in Babel and esbuild (transform mode)

[jakebailey]

I would think that the moment you're doing something unsafe like casting an enum or ts-ignoreing, all bets are off about actual runtime behavior. The only "safe" thing I think exists is to assume one can use a const enum as a value unsafely if preserveConstEnums is enabled (esbuild will keep them around), but that idea only works for local enums.

[magic-akari]

@RyanCavanaugh Sorry to bother you again.
I am trying to implement this, but it does not work.

// @showEmit
// @isolatedModules: true
// @filename: foo.ts
export enum Foo {
    "Infinity" = 1,
    A = 1 / 0,
}

// @filename: index.ts
import { Foo } from "./foo";
export enum Bar {
    B = Foo.Infinity,
    C
}

Workbench Repro

The tsc says that the Bar.C is 2.
But I got A1 through Bar.B + 1,
or 1A through 1 + Bar.B.

Babel playground cc @nicolo-ribaudo

[nicolo-ribaudo]

That's a separate bug -- TSC should probably disallow "Infinity" and "NaN" as keys the same way as it disallows "3".

[magic-akari]

That's a separate bug -- TSC should probably disallow "Infinity" and "NaN" as keys the same way as it disallows "3".

I am trying to fix this. But I found that "Infinity" "-Infinity" and "NaN" were intentionally excluded.

https://github.com/microsoft/TypeScript/blob/6424e181d4d7460913e730249a078b176fc8aaf0/src/compiler/checker.ts#L44730-L44735

[nicolo-ribaudo]

It was accepted as a bug at #48956.

[RyanCavanaugh]

TL;DR from #56164.

Under isolatedModules only, the following should be errors:

• When an enum initializer expression that isn't a string literal has a string type
• When an enum member without an initialization expression follows an enum member with an initialization expression that isn't a numeric literal

[nicolo-ribaudo]

(from the meeting notes)

On the other hand, there is nothing preventing compilers from making these enums "work".

Note that Babel currently transforms

enum Bar {
    B = Foo.A,
    C
}

to

var Bar = function (Bar) {
  Bar[Bar["B"] = Foo.A] = "B";
  Bar[Bar["C"] = 1 + Bar["B"]] = "C";
  return Bar;
}(Bar || {});

Is this what was meant by "work"? Or is this correct only under the new proposed behavior?

[RyanCavanaugh]

Is this what was meant by "work"

Yes, though under the agreed-on rules, C would be an error

[typescript-bot]

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of the repro in the issue body running against the nightly TypeScript.

---

Issue body code block by @magic-akari

👍 Compiled
Emit:

import { Foo } from "./foo";
export var Bar;
(function (Bar) {
    Bar[Bar["B"] = 10] = "B";
    Bar[Bar["C"] = 11] = "C";
})(Bar || (Bar = {}));

Historical Information

Version	Reproduction Outputs
4.8.2, 4.9.3, 5.0.2, 5.1.3, 5.2.2	👍 Compiled Emit: import { Foo } from "./foo"; export var Bar; (function (Bar) { Bar[Bar["B"] = 10] = "B"; Bar[Bar["C"] = 11] = "C"; })(Bar || (Bar = {}));

[typescript-bot]

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of this repro running against the nightly TypeScript.

---

Comment by @magic-akari

👍 Compiled
Emit:

import { Foo } from "./foo";
export var Bar;
(function (Bar) {
    Bar[Bar["B"] = 1] = "B";
    Bar[Bar["C"] = 2] = "C";
})(Bar || (Bar = {}));

Historical Information

Version	Reproduction Outputs
4.8.2, 4.9.3, 5.0.2, 5.1.3, 5.2.2	👍 Compiled Emit: import { Foo } from "./foo"; export var Bar; (function (Bar) { Bar[Bar["B"] = 1] = "B"; Bar[Bar["C"] = 2] = "C"; })(Bar || (Bar = {}));

[frigus02]

• When an enum initializer expression that isn't a string literal has a string type

Could we allow template expressions? E.g. the following is guaranteed to have a string value. How do you feel about allowing that and not generating a reverse mapping for it?

enum Foo { A = `${Bar.A}` }

Edit: Never mind. Turns out in our code base this pattern happens so rarely, this doesn't feel worth the effort.