require() in generated JS code for a type only import holding a const enum

[Flarna]

TypeScript Version: 2.5.2 / nightly (2.6.0-dev.20170921)

Code
File foo.ts exports a const enum + a function:

export const enum ConstFooEnum {
  Some,
  Values,
  Here
};
export function fooFunc(): void { /* removed */ }

File index.ts which imports Foo.ts but uses only the const enum values:

import * as Foo from "./foo";

function check(x: Foo.ConstFooEnum): void {
  switch (x) {
    case Foo.ConstFooEnum.Some:
      break;
  }
}

Expected behavior:
Generated Javascript code should not require("./foo"). Docu (http://www.typescriptlang.org/docs/handbook/modules.html) indicates that type only imports should not result in a require().

Actual behavior:
Generated JS code contains line var Foo = require("./foo"); even Foo is not used anywhere.

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var Foo = require("./foo");
function check(x) {
    switch (x) {
        case 0 /* Some */:
            break;
    }
}

In above simple code sample I can workaround the issue by doing importing only the ConstFooEnum via import { ConstFooEnum } from "./foo"; but if the enum is inside some namespace (e.a. enum gets re-exported from an inner module) it's not that easy.

[mhegazy]

@weswigham can you take a look.

I do remember we did this for a reason.. something to do with namespace merging scenarios.. but do not remember what exactelly thought...

maybe @vladima remembers :)

[weswigham]

IIRC references to const enums are transpiled as normal enum references under isolatedModules (since there's no type info to work with), so to account for this they're still marked as value-space reads of the reference in order to have more consistent behavior between isolatedModules compilations and normal ones.

[mhegazy]

it should be an error to use const enums under --isolatedModules. i think it would make sense to not mark as referenced if we are compiling with --isolatedModules false.

[weswigham]

Looking at our code, I feel like there's intent that the import should be elided here, as the identifier is checked. I'm pretty sure if you wrote it as:

import {ConstFooEnum} from "./foo";

function check(x: ConstFooEnum): void {
  switch (x) {
    case ConstFooEnum.Some:
      break;
  }
}

it'd get elided.

[weswigham]

Yep. Using a destructuring import pattern causes ellision to occur as expected!

We do check that references to const enums and const enum only modules aren't considered value references; but in

import * as Foo from "./foo";

function check(x: Foo.ConstFooEnum): void {
  switch (x) {
    case Foo.ConstFooEnum.Some:
      break;
  }
}

we also check Foo in case Foo.ConstFooEnum.Some:. Foo is not a const enum only module or a const enum (it's a module alias), and so gets marked as a reference.

[Jessidhia]

@mhegazy I think it's fine to use const enums in --isolatedModules; what is bad is that you can export them.

[mhegazy]

That is true. not sure if there is much need for using const enums within a single file though. we can allow it, but so far i have not seen any requests for that.