You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
isolatedModules doesn't respect enabled preserveConstEnum option what the project might be build with #37774
The compiler should respect/understand that the package/module was compiled with enabled preserveConstEnums compiler option.
Use Cases
Let's say we have a project written in TypeScript, which uses const enums.
The project uses them because the main const enum's advantage is inlining values instead of runtime accessing to the value.
In other hand the maintainers understand that const enums compiles in nothing in JS code, and that means that consumers can't use a const enums in their JS code (in some cases in TS code either, see above). Thus, they decided to enable preserveConstEnums compiler option to leave all const enums in JS code and allow consumers use them as well.
Up to this moment everything is good. But the next who appears on the stage is isolatedModules (say hello to create-react-app, webpack's transpile-only mode, transpileModule compiler's API and so on). This compiler option is used to detect that imported module could be transpiler without compilation into JS code and everything will work well. One of checks for this option is that you don't import const enums (error TS2748: Cannot access ambient const enums when the '--isolatedModules' flag is provided).
To summarize, if the project/package is compiled with enabled preserveConstEnums, in isolatedModules mode the compiler shouldn't fail with error about unavailability of using const enums, because that's not actually true.
Examples
Quite possible we need to provide that information to the compiler somehow (by providing tsconfig.export.json or in package.json for instance), but I don't even know how it would be done.
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.)
It seems like arguably preserveConstEnum should just emit enum instead of const enum to the .d.ts, though I'm sure that would break some other scenario
CRA hard enforces the isolatedModules: true flag nowadays. It used to be possible to extend the tsconfig to override this but now it looks like eject or replace the react-scripts file specific to enforcing tsconfig. Neither is desirable when the issue is strictly the way preserveConstEnum is behaving. Running into this issue right now so curious. Thanks!
We’ve discussed this some in the past and I feel like I must be forgetting some of the complexities that came up. I just jotted down some straw-man proposals, and the simplest one feels fairly straightforward:
preserved modifier for const enums
// @Filename: /node_modules/foo/index.d.tsexportpreservedconstenumFoo{Bar=1}// @Filename: /index.ts// @isolatedModules: trueimport{Foo}from"foo";Foo.Bar;// ok
Only valid in ambient contexts, automatically added to declaration emit under --preserveConstEnums.
A preserved const enum can be imported and used under --isolatedModules because its usage need not be inlined.
If needed, a compiler flag could be added that controls whether preserved const enums from node_modules / other projects / declaration files get inlined or not. (We’ve had some concerns before about inlining enum values from declaration files that could get out of sync with implementations, but I want to hear the case for that again before I’d be enthusiastic about adding a flag.)
I tossed around another proposal combining Ryan Cavanaugh (@RyanCavanaugh)’s original suggestion that const enums could be emitted to declaration files as regular enums with a compiler option --inlineEnums that could offer local control over what enums get inlined, but ultimately I think that level of control would be more confusing than useful. Part of our hesitation on doing anything about this is that enums are already confusing, so we want a solution to be as simple and explainable as possible.
Reacted by Максим Иванов, Evgeniy Timokhov, Fonger, Pete Gonzalez, David Zearing and Jacob Ley
There are three issues with exported ambient const enums, I think:
They're incompatible with isolatedModules.
∴ tsc --declaration stops downstream consumers from using isolatedModules, roughly. Specifically, by transforming non-ambient const enums (.ts files) into ambient const enums (.d.ts files), it precipitates the following trilemma:
upstream project can use const enums
downstream consumer can use those enums' values
downstream consumer can use isolatedModules
Choose any two. 😞
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:
Import elision. importsNotUsedAsValues: "preserve" preserves imports for inlined enum values, but ambient const enums (.d.ts files) may or may not have runtime .js files. The usual way to unambiguously elide imports, import type, doesn't allow const enum values, currently.
Taking the opposite approach (eliding imports for inlined enum values) would invalidate importsNotUsedAsValues: "preserve" for ambient const enums with runtime .js files.
Same impact as A except upstream projects can inline enum values. Works with and without isolatedModules. Consumers can't inline enum values (:+1: good, per #2).
C. Want consumers to inline enum values, with consequent isolatedModules incompatibility, maybe because all participating projects are part of an internal ecosystem/composite project where versions are synchronized? Then preserveConstEnums: false is for you!
Know and accept the implications: #1, #2 and #3.
No change: Same as today.
X. Want some consumers to inline enum values and others not to, or another advanced scenario? Out of scope, for now:
local control over what enums get inlined, but ultimately I think that level of control would be more confusing than useful. Part of our hesitation on doing anything about this is that enums are already confusing, so we want a solution to be as simple and explainable as possible.
Note: Existing preserveConstEnums and isolatedModules behavior
Because I TIL'ed :-)
Turns off inlining
Preserves imports
Transforms enums -> .js
preserveConstEnums
❌
❌
✔️
isolatedModules
✔️
✔️
✔️
I.e. isolatedModules implies preserveConstEnums, !preserveConstEnums implies !isolatedModules.
I've been giving significant thought to injecting a custom transformer into the pipeline that converts this:
// Current const enumexportconstenumAstKind{None='None',Script='Script',AndIf='AndIf',Command='Command',CompoundWord='CompoundWord',VariableExpansion='VariableExpansion',Text='Text'}
Into this:
// Provide values at runtime, use an object literal to allow minifiers to drop unused values and/or inline (if in the same scope, at least)exportconstAstKind={None: 'None',Script: 'Script',AndIf: 'AndIf',Command: 'Command',CompoundWord: 'CompoundWord',VariableExpansion: 'VariableExpansion',Text: 'Text'}asconst;// Allow the values to be used in a type positionexportdeclarenamespaceAstKind{exporttypeNone=typeofAstKind.None;exporttypeScript=typeofAstKind.Script;exporttypeAndIf=typeofAstKind.AndIf;exporttypeCommand=typeofAstKind.Command;exporttypeCompoundWord=typeofAstKind.CompoundWord;exporttypeVariableExpansion=typeofAstKind.VariableExpansion;exporttypeText=typeofAstKind.Text;}// Allow for the enum itself ot be used in a type positionexporttypeAstKind=typeofAstKind[keyoftypeofAstKind];
Also further enables the following, which you can't do with a const enum:
import{typeAstKind}from'./AstKind';// Type checks properlyconstz: AstKind.Text='Text';constz: AstKind='Text';
but still prevents, for example:
constx: AstKind.AndIf=AstKind.Command;// Error, not assignableconsty: AstKind='Unknown';// Error, not assignable
The one restriction of current const enum that gets lost is that it allows cross-assignment of enums as long as the values are compatible, but this is as much a feature as it is a bug, since it is what allows type-only imports to the enum.
It is possible to restore the assignability restriction while still allowing the raw string literal assignment via something like:
Though unfortunately that probably has deleterious effects on type-checking performance. It is also a lot less readable than an enum OtherEnum { ... } declaration.
We run into this error at work because we define all our enums as const and our entire codebase is put together using project references.
The crux of the issue, from what I can tell, is that TS treats project references as an "opaque boundary" and ignores the tsconfig settings that may be set.
This is one of those "principle of least surprise" things I suppose - because there's no guarantee that a referenced project is built using the referenced tsconfig settings so in order to be least surprising (and cause fewer runtime crashes) it's better to assume the const enum is always elided.
However this does mean that in codebases that are built using the same config/built tool setup there's no way to convince TS that the pattern is in fact completely safe. So you're left with a typecheck run that's littered with TS2748 errors.
At Canva we work around this by setting noEmitOnError: true (along side isolatedModules: true, preserveConstEnums: true) and we don't run tsc directly and instead have a wrapper which deletes the TS2748 error from the output.
It's an ugly workaround because it means we have to build all our code using the wrapper. Thankfully VSCode doesn't show this specific error in the IDE (#57833) or else we'd be in a difficult position and have to either convert all enums to non-const enums or disable isolatedModules in our root config.
I wonder if there's a simple solution here to allow a user to convince TS that accessing const enum across the boundary is in fact safe? allowConstEnumAcrossBoundaryTrustMeBro: true?
Project references are not always considered as an opaque boundary. Module resolution in referenced .d.ts files, for example, happens according to the referenced project’s settings, not the importing project’s settings, and we’ve speculated at times that there may be other places in the checker where we should be fetching the referenced project’s settings but are currently using the currently checked project’s settings. It doesn’t seem out of the question to me that this error could be silenced when it can be determined to be safe due to the ambient file belonging to a project that uses preserveConstEnums.
I wanted something like this in #51530 so that we could use const enums within TS but still refer to them by value if needed; my issue is probably a dupe of the above request.
Activity
RyanCavanaugh commentedon Apr 3, 2020
It seems like arguably
preserveConstEnumshould just emitenuminstead ofconst enumto the.d.ts, though I'm sure that would break some other scenariotimocov commentedon Apr 3, 2020
Ryan Cavanaugh (@RyanCavanaugh) Yeah, at least a consumer who uses TypeScript without using isolatedModules couldn't get that "enums" inlined 🙂
ok-AK commentedon Nov 2, 2020
Any updates for this issue?
CRA hard enforces the
isolatedModules: trueflag nowadays. It used to be possible to extend the tsconfig to override this but now it looks like eject or replace the react-scripts file specific to enforcing tsconfig. Neither is desirable when the issue is strictly the waypreserveConstEnumis behaving. Running into this issue right now so curious. Thanks!splincode commentedon Feb 26, 2021
i have same issue
MLoughry commentedon May 19, 2021
I'm hitting this issue as well trying to enable the
isolatedModulesflag.MLoughry commentedon May 21, 2021
Ryan Cavanaugh (@RyanCavanaugh) I see this is tagged with
Awaiting more feedback. What feedback are you looking for?andrewbranch commentedon Aug 20, 2021
We’ve discussed this some in the past and I feel like I must be forgetting some of the complexities that came up. I just jotted down some straw-man proposals, and the simplest one feels fairly straightforward:
preservedmodifier for const enums--preserveConstEnums.preservedconst enum can be imported and used under--isolatedModulesbecause its usage need not be inlined.preservedconst enums from node_modules / other projects / declaration files get inlined or not. (We’ve had some concerns before about inlining enum values from declaration files that could get out of sync with implementations, but I want to hear the case for that again before I’d be enthusiastic about adding a flag.)I tossed around another proposal combining Ryan Cavanaugh (@RyanCavanaugh)’s original suggestion that const enums could be emitted to declaration files as regular enums with a compiler option
--inlineEnumsthat could offer local control over what enums get inlined, but ultimately I think that level of control would be more confusing than useful. Part of our hesitation on doing anything about this is that enums are already confusing, so we want a solution to be as simple and explainable as possible.jablko commentedon Nov 4, 2021
There are three issues with exported ambient const enums, I think:
They're incompatible with
isolatedModules.∴
tsc --declarationstops downstream consumers from usingisolatedModules, roughly. Specifically, by transforming non-ambient const enums (.tsfiles) into ambient const enums (.d.tsfiles), it precipitates the following trilemma:isolatedModulesChoose any two. 😞
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:
Import elision.
importsNotUsedAsValues: "preserve"preserves imports for inlined enum values, but ambient const enums (.d.tsfiles) may or may not have runtime.jsfiles. The usual way to unambiguously elide imports,import type, doesn't allow const enum values, currently.Taking the opposite approach (eliding imports for inlined enum values) would invalidate
importsNotUsedAsValues: "preserve"for ambient const enums with runtime.jsfiles.These are the solutions as I see them:
A. Don't use const enums entirely. Ban them with a linter.Avoids
#1and#2, sidesteps#3, but stops upstream projects from inlining enum values, with performance implications.Doable today.
B. Transform non-ambient const enums into ambient non-const enums, either with:preserveConstEnumsalone (Declare non-const enums if preserveConstEnums is true #46626).preserveConstEnums+ a build step (Remove const enums from API #5297 (comment), Const enum deconst'ing #5422 (comment)).TypeScript/Gulpfile.js
Line 144 in 1a981d1
Same impact as
Aexcept upstream projects can inline enum values. Works with and withoutisolatedModules. Consumers can't inline enum values (:+1: good, per#2).C. Want consumers to inline enum values, with consequentisolatedModulesincompatibility, maybe because all participating projects are part of an internal ecosystem/composite project where versions are synchronized? ThenpreserveConstEnums: falseis for you!Know and accept the implications:
#1,#2and#3.No change: Same as today.
X. Want some consumers to inline enum values and others not to, or another advanced scenario? Out of scope, for now:Note: Existing
preserveConstEnumsandisolatedModulesbehaviorBecause I TIL'ed :-)
.jspreserveConstEnumsisolatedModulesI.e.
isolatedModulesimpliespreserveConstEnums,!preserveConstEnumsimplies!isolatedModules.dmichon-msft commentedon Jan 27, 2022
I've been giving significant thought to injecting a custom transformer into the pipeline that converts this:
Into this:
This allows the following usage:
Also further enables the following, which you can't do with a const enum:
but still prevents, for example:
The one restriction of current
const enumthat gets lost is that it allows cross-assignment of enums as long as the values are compatible, but this is as much a feature as it is a bug, since it is what allows type-only imports to the enum.It is possible to restore the assignability restriction while still allowing the raw string literal assignment via something like:
Though unfortunately that probably has deleterious effects on type-checking performance. It is also a lot less readable than an
enum OtherEnum { ... }declaration.octogonz commentedon Apr 6, 2022
Maybe the keyword should be
inline. It seems that the underlying concept is:enumthinks it's a good idea for compilers to inline its membersThat is exactly the meaning of
inlinein C and C++.In fact
const enumis somewhat counterintuitive, given that enums are always "constants" (in common understanding).It would also simplify adoption. The message is: "search+replace
const enumwithinline enumeverywhere and you're good."And then later we could introduce
inlinefunctions. 😉bradzacher commentedon Mar 18, 2024
We run into this error at work because we define all our enums as
constand our entire codebase is put together using project references.The crux of the issue, from what I can tell, is that TS treats project references as an "opaque boundary" and ignores the tsconfig settings that may be set.
This is one of those "principle of least surprise" things I suppose - because there's no guarantee that a referenced project is built using the referenced tsconfig settings so in order to be least surprising (and cause fewer runtime crashes) it's better to assume the
const enumis always elided.However this does mean that in codebases that are built using the same config/built tool setup there's no way to convince TS that the pattern is in fact completely safe. So you're left with a typecheck run that's littered with TS2748 errors.
At Canva we work around this by setting
noEmitOnError: true(along sideisolatedModules: true, preserveConstEnums: true) and we don't runtscdirectly and instead have a wrapper which deletes the TS2748 error from the output.It's an ugly workaround because it means we have to build all our code using the wrapper. Thankfully VSCode doesn't show this specific error in the IDE (#57833) or else we'd be in a difficult position and have to either convert all enums to non-const enums or disable
isolatedModulesin our root config.I wonder if there's a simple solution here to allow a user to convince TS that accessing
const enumacross the boundary is in fact safe?allowConstEnumAcrossBoundaryTrustMeBro: true?andrewbranch commentedon Mar 18, 2024
Project references are not always considered as an opaque boundary. Module resolution in referenced
.d.tsfiles, for example, happens according to the referenced project’s settings, not the importing project’s settings, and we’ve speculated at times that there may be other places in the checker where we should be fetching the referenced project’s settings but are currently using the currently checked project’s settings. It doesn’t seem out of the question to me that this error could be silenced when it can be determined to be safe due to the ambient file belonging to a project that usespreserveConstEnums.jakebailey commentedon Mar 18, 2024
I wanted something like this in #51530 so that we could use const enums within TS but still refer to them by value if needed; my issue is probably a dupe of the above request.