[@thi.ng/api] types incompatible with typescript --isolatedModules

[Bnaya]

When consuming @thi.ng/api, and the consuming project has isolatedModules: true

There's tsc errors:

node_modules/@thi.ng/api/api.d.ts:150:6 - error TS2748: Cannot access ambient const enums when the '--isolatedModules' flag is provided.

150     [Type.U8]: number;
         ~~~~

isolatedModules is important for compatibility with babel transpiled typescript, and in general.

The solution would be to not use const enums, and even apply isolatedModules on the project

[postspectacular]

Hi @Bnaya - thanks for reporting this, though it's the first time I've even heard of that option and am a bit surprised to hear that this is recommend usage for Babel and no one else seems to have had issues with that thus far. From a brief scanning of TS issues about that option, it also seems that it currently still has quite some unresolved issues, especially regarding degraded performance. Furthermore, we can't apply isolatedModules to the project, since that doesn't allow using the emitDeclaration build flag at the same time... go figure...

In general, I also really don't want to give up const enums since they're not just faster than normal enums (esp. in tight loops), but also used in various other packages of this repo...

Have you, by any chance, tried other packages, e.g. adjacency, color, diff, math, pixel, rstream etc. I guess most of them will cause similar issues, since they all depend on thi.ng/api...

[postspectacular]

Forgot to mention: A few weeks back, I've completely restructured the api package, but haven't released it yet. I should maybe publish a pre-release version so you can check if that new structure makes any difference...

https://github.com/thi-ng/umbrella/tree/develop/packages/api/src

Do you have test repo somewhere to reproduce the issue?

[Bnaya]

You can just run:
yarn tsc --isolatedModules true in any package that depends on api, or in api itself, and see the errors.

Basically what isolatedModules is doing is ensuring that there are no type-directed emits, and each file can be separately separately. is has no emit effect.
babel needs that because he just stripping the types, and transpile every file individually and can't relay on them for emit.
In const enums unlike enums, the values are inlined, so you have cross-files compile time dependency.

I understand the appeal in const enums, but if typescript would have come out today, i believe they wouldn't add them to the language, as all of the other type-directed emits features reference to that claim

typescript 3.7 allows isolatedModules + declarations,
but for pre 3.7 its possible to do one of the following:
set isolatedModules: false, and as part of CI or tests, pass --isolatedModules true as an additional check.
Set isolatedModules: true, and when you want to emit declarations, pass --isolatedModules false --declarations true

Regarding the performance degradation, it will be fixed in 3.7.

[Bnaya]

A replacement for const enums can be literal types,

type U8 = 0;
type U8C = 1;
type U16 = 2;
type I16 = 3;

export type TypeNotConstEnum = U8 | U8C | U16 | I16;

const valid: TypeNotConstEnum = 3;
const invalid: TypeNotConstEnum = 50;

it's of course harder to write

[postspectacular]

Thanks for the all the info @Bnaya! - I have been using this approach (via type aliases) in http://thi.ng/shader-ast, and you're right, that's probably a better way to go... will keep you posted!

[postspectacular]

Just posting this great explanation here for future ref (though I still feel there must a better way to retain the succinctness of const enums and satisfy isolated modules). The issue with the above type defs is that these can't be used as values. However, the following would/should work, actually:

const U8 = 0;
const U8C = 1;
const U16 = 2;
const U32 = 3;

const F32 = 4;
const F64 = 5;

type Uint = typeof U8 | typeof U8C | typeof U16 | typeof U32;
type Float = typeof F32 | typeof F64;

function uintFunc(type: Uint) {}

function floatFunc(type: Float) {}

uintFunc(U8); // ok
uintFunc(F32); // fail

floatFunc(U8); // fail
floatFunc(F32); // ok

Playground link

[postspectacular]

@Bnaya the other thing you can use (for now) is this babel plugin: babel-plugin-const-enum. I've tried with this config and works well:

{
  "presets": ["@babel/preset-typescript"],
  "plugins": [
    ["const-enum", { "transform": "constObject" }]
  ]
}

[Bnaya]

Features like const enums are not in the future happy-path of TypeScript, as it blocks single files transpilation and It's some kind of type-directed emit
(You need information from another file in order to perform the transpilation)

I believe that the future recommendations of typescript would be to add an optimiser pass to do this kind of inlining, but the DX is not as nice

[postspectacular]

@Bnaya agreed - and I've started updating some of them now, but this will probably cause some breaking changes once I got through all of the packages. e.g. in the webgl package, there're large numbers of const enums and i think for legibility/comprehension reasons, those will need to retain some form of prefix in their new names... doing those change on a new feature branch, so we can discuss in a bit.... will keep you posted!

[postspectacular]

@Bnaya Looking at all the various sites where const enums are used in this project, I've decided to defer this quite massive refactoring and just switch to "normal" enums for now. I also enabled isolatedModules for all packages now. Should there're be any unexpected issues (e.g. perf) becoming obvious, then let's revisit those places/modules in the future... the current changes shouldn't cause any API breakage & will prep a new release of all packages later today...

[Bnaya]

Thanks!!:)
For future refrence
https://github.com/angular/tsickle
That's what google are using to to clousure compiler advance optimazation to typescript code
Related:
https://github.com/theseanl/tscc/blob/master/README.md

[postspectacular]

Thanks again, @Bnaya 💯 - new releases of all packages are up now & am closing this for now...