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

Allow importing both type entity and value entity for "const enum" through "import type" #40344

Open
5 tasks done
FuuuOverclocking opened this issue Sep 1, 2020 · 16 comments
Open
5 tasks done
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@FuuuOverclocking
Copy link

FuuuOverclocking commented Sep 1, 2020

Search Terms

const enum, import, importsNotUsedAsValues

Scenario

I am a library developer, and there is a really large types.ts file in my project, which contains many interface, type, and const enum, defining a large part of types of the project, just like you do (typescript/src/compiler/types.ts).

Recently I turned on the compiler option importsNotUsedAsValues, and found some problems about const enum.

Original fileA.ts

import {
    TypeA, TypeB, TypeC,
    ConstEnumD, ConstEnumE,
} from './types.ts';

------------------

Current fileA.ts

import type {
    TypeA, TypeB, TypeC,
} from './types.ts';
import {
    ConstEnumD, ConstEnumE,
} from './types.ts';

Because the entities of enum belong to both type scope and value scope, I have to import them separately if I want to use them as values.

Now I have to separate the import of type const enum from a lot of names file by file, which is really a lot of work.

What's worse, after turning on importsNotUsedAsValues, TypeSciprt unnecessarily write require('./types.js') into fileA.js though the generated code did not access any property of require('./types.js'). In other words, it makes types.ts seems to have side effects. And this feature may affect the file reference relation of the project and lead to incorrect Rollup order.

In my mind, accessing the const enum entity in value scope is a kind of special behavior and is something that will be replaced in compile-time. There should be a way to import both type entity and value entity of const enum from a file without really requiring the file, especially when the file really has side effects.

Solution

There are 2 ways to solve this problem:

1. import both type and value for const enum through import type

import type { ConstEnumA } from './types';
//       Type↓        Value↓
const a: ConstEnumA = ConstEnumA.propA;

// ts not emit "  require('./types')  "

I prefer this method because I will not need to greatly change my code.

Breaking change?

Name conflicts may occur if there was already a name in value scope which was same with 'ConstEnumA',
but I think this is very very rare and will be prevented by linter usually. An extra compiler option can be provided to solve this.

2. check const enum in value-import

// importsNotUsedAsValues is still on

import { ConstEnumA } from './types';
//       Type↓        Value↓
const a: ConstEnumA = ConstEnumA.propA;

// ts find just "ConstEnumA" is used and it is `const enum`
// not emit "  require('./types')  "

Breaking change?

Yes. Now TS do not emit require('...') though value-import exists in the code.

Examples

A complete example of Solution 1:

import type {
    TypeA,
    TypeB,
    TypeC,
    ConstEnumA,
    TypeD,
    TypeE,
    TypeF,
    ConstEnumB,
    TypeG,
    TypeH,
    TypeI,
    ConstEnumC,
    ConstEnumD,
    TypeJ,
    TypeK,
} from './types';

const b: ConstEnumB = ConstEnumB.propA;
// compiler: OK, 0 error(s). not emit "  require('./types.js')  "

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@FuuuOverclocking FuuuOverclocking changed the title Allow to import both type entity and value entity by import type { ConstEnumType } from .. Allow importing both type entity and value entity for const enum by type-import Sep 1, 2020
@FuuuOverclocking FuuuOverclocking changed the title Allow importing both type entity and value entity for const enum by type-import Allow importing both type entity and value entity for "const enum" by type-import Sep 1, 2020
@FuuuOverclocking
Copy link
Author

I'm a English learner. I would be happy to correct my expression if someone willing to remind me.

@FuuuOverclocking FuuuOverclocking changed the title Allow importing both type entity and value entity for "const enum" by type-import Allow importing both type entity and value entity for "const enum" through "import type" Sep 1, 2020
@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Sep 3, 2020
@RyanCavanaugh
Copy link
Member

I don't see any tractable way to square this circle. import type needs to mean "don't include the value side"; that's its definition. And import { ConstEnum } from 'x' needs to do at least as much importing as import { } from'x' to maintain the definition of import. const enum and import type just aren't really compatible features and I'd advise to avoid const enum in module positions for this reason.

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Sep 3, 2020
@RyanCavanaugh
Copy link
Member

Talking offline with @andrewbranch, this does seem like something where we could just ignore the warning we "should" issue when a value reference occurs. This would still break the transpile scenarios that import type is designed for, but obviously you're not doing that anyway

@FuuuOverclocking
Copy link
Author

FuuuOverclocking commented Sep 6, 2020

I think we can look at this problem in a different way. Let's review the scenario in which importsNotUsedAsValues was proposed. Programmers complain that they must write import "someModule"; again to ensure that modules with side effects are imported, otherwise the import may be automatically filtered out.

Then the import type syntax was invented, which can clearly tell the compiler which imports are just TS level and and which imports must be actually written to JS files. That is the key point. We usually only regard the type system as TS level things. Once the value side is involved, we think it is JS level. But in fact, there are still some incoherent things like const enum. It is like constexpr in C++, which is on the value side but will be determined at compile time.

So, I think we only need to change the definition of import type slightly, not as statements that only import types, but as import (something only related to )type(script), then everything can be explained, of course, import type can import values.

@andrewbranch
Copy link
Member

andrewbranch commented Sep 8, 2020

That is a pretty good description of how import type already works, but we don’t recognize const enum usage in value space as an allowable usage. It would be possible to add it as an exception, since it doesn’t require a reference to the symbol in JS emit. In fact, I believe @ajafff suggested this early on (#36003). We opted to wait and see if there was demand for it. So far, the demand has been very low (this is the first request since @ajafff’s, as far as I know), but the downsides of implementing it seem small.

@jdom
Copy link
Member

jdom commented Feb 17, 2021

Since you are gauging demand for it, we are also running into this in the Azure Portal. We have a ton of dynamically loaded modules (where import type is useful) and use const enums a lot. We have to workaround this issue with non-ideal approaches that could lead to someone mistakenly importing a module explicitly and causing tons of bundling side-effects

@MLoughry
Copy link

As an additional data point, I'm investigating adopting Vite as a dev build tool for Outlook. However, it builds each module individually, so I need to enable "isolatedModules": "true" to ensure the dev build doesn't result in runtime errors. Between this issue and #37774 (some of our dependencies use const enum with "preserveConstEnum": "true", which I cannot control), I cannot enable isolatedModules in the main build.

devtools-bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this issue Jul 21, 2021
…sManager

This change extracts the IssuesManager Events enum into a separate file
to break the circular dependency of values between IssuesManager.ts and
SourceFrameIssuesManager.ts. This helps reduce existing circular
dependencies, in theory moving us slightly closer to being able to
warn about such cycles at build-time to avoid unexpected errors.

This particular cycle recently led to a runtime error when attempting to
subclass IssuesManager to introduce additional functionality. This
subclass was included in the cycle as SourceFrameIssuesManager was
updated to import the subclass instead of IssuesManager. At the time
the Events enum for IssuesManager was non-const, causing the circular
dependency to exist post-build. The error occurred because the extends
relationship of the subclass was evaluated during module initialization.

Since then the Events enum has been converted to const which eliminates
the runtime error. However, eliminating even this cycle is still
beneficial. The compile-time only relationship is not obvious and
cannot otherwise be communicated explicitly until values from const enums
can be used with `import type` per
microsoft/TypeScript#40344.

Bug: 1229328
Change-Id: I033080c77f22584aa0527bcd0559698341d200f5
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3040178
Commit-Queue: Tony Ross <[email protected]>
Reviewed-by: Sigurd Schneider <[email protected]>
Reviewed-by: Tim van der Lippe <[email protected]>
@dimensia
Copy link

Adding this ability would really benefit us where I work at as well.

@SohumB
Copy link

SohumB commented Aug 31, 2021

Just adding a datapoint here as well. Our project makes extensive use of a large number of enums in a dependency, and this is directly impeding our efforts to bundle-split that dependency until it's actually needed, instead of just being needed to grab a constant string.

@skylarmb
Copy link

skylarmb commented Oct 27, 2021

I am in a similar boat where our types come from an extremely large auto-generated package (60mb+ of JS, 8mb of types). I can use import type to import pure types without impacting bundle size, but that completely excludes us from being able to use the enums defined in the types file. Also because default typescript enums are not type safe due to simply being type number (#32690, #36756), there is no possible way to safely use the imported enum type.

// ERROR: 'Foo' cannot be used as a value because it was imported using 'import type'.
const fooSomeValue = Foo.SOME_VALUE;
// This provides zero type safety. Any number could be assigned as a value here.
const fooSomeValue: Foo.SOME_VALUE = 1;

@skylarmb
Copy link

Here is a novel workaround I came up with. Be aware that this is hacky and may not be to your personal tastes, but just posting in order to share.

// Suppose this enum was imported using `import type` and thus we cannot directly use it as a value
enum TestEnum {
  FOO = "FOO",
  BAR = "BAR",
  BAZ = "BAZ",
}

/**
 * Due to https://github.com/microsoft/TypeScript/issues/40344, we cannot declare an object that uses
 * TestEnum as a value, like so
 * {
 *   a: TestEnum.FOO,
 *   b: TestEnum.BAR
 *   ...
 * }
 *
 * So instead, lets make an exhaustive map of the enum type, which is the opposite of the desired mapping
 * above. A compiler error will be thrown if any values are added/removed from TestEnum, so we can ensure
 * we are somewhat type safe here.
 */
const initialMap: Record<TestEnum, string> = {
  FOO: "a",
  BAR: "b",
  BAZ: "c",
};

/**
 * Now, invert that mapping so we now have a map of the your type to enum values, which emulates using
 * the enum as a value.
 */
const invertedMap: Record<string, TestEnum> = Object.keys(initialMap).reduce((res, key) => {
  // We have to ignore the typescript error here as we are definitely doing some janky stuff
  // @ts-ignore
  res[initialMap[key]] = key;
  return res;
}, {} as Record<string, TestEnum>);

// Tada! We have a mapping of string to enum value while only using the enum as a type rather than a value
console.log(invertedMap["b"]); // Logs: 'BAR'

// NOTE: This works well for string enums, but number enums will have their values cast to strings by `Object.keys`
// You could `parseInt()` on these at your own risk...

@jablko
Copy link
Contributor

jablko commented Nov 1, 2021

Do you need ambient const enums (e.g. const enums in .d.ts files) or would a non-const enum work as well?

  1. Fundamentally ambient const enums (values -- types are fine) are incompatible with isolatedModules.
  2. If isolatedModules is false, you can use ambient const enum values but can't use type-only const enum values (regardless). Is this inconsistent? Are there other self-contained ambient values (entities without .js implementations, necessarily)? Design Meeting Notes, 2/24/2021 #42991 (comment)

My take:

Copy link

This was definitely a surprise to me as the const enum values are inlined everywhere at compile time, and so really their usage is by design decoupled from the origin module at runtime. Having to try to work out ways around this.

@jbalinski
Copy link

We would love this change. Here is the scenario where it comes into play for us.

We generate d.ts files from our C# classes. The generated typing files output const enums from our C# enums. We reference these enums all over our typescript code. With this approach we do not need to keep two versions of our enums in sync (one for C# and one for typescript).

Since converting our typescript to modules, our options for compiling our project using these const enums is now very limited. We are currently using Webpack which is able to handle the compilation. However, small changes to files can take a while to rebuild when using webpack. The ts-loader transpileOnly option, though wildly faster, is not able to inline the values of these ambient const enums and obviously breaks things. The fork-ts-checker-webpack-plugin suffers from the same issue for presumably the same reason.

A way forward with this would be greatly appreciated.

@SavkaTaras
Copy link

SavkaTaras commented Feb 23, 2023

Hello there,
I hope you all are doing well.

It would benefit us as well to allow export const enum Example {} be able to be consumed as type { Example } and to get Object.values(Example).

Otherwise we would have to duplicate the values.

Thank you,
Taras

@dsogari
Copy link

dsogari commented Feb 4, 2024

I'm coming rather late to the discussion, but let me add a suggestion. Since the argument for not implementing this feature is that it would change the semantics of either type-import or value-import, why not introduce a new syntax for importing and exporting compile-time constants? For example:

import const { MyEnum1 } from 'x';
import { const MyEnum2 } from 'x';

and:

export const { MyEnum1 };
export { const MyEnum2 };

This would make very clear the distinction between TS types, TS constants and JS bindings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.