Use isolatedModules

[ry]

This is going to break a lot of code, however its important for sanity when running with --no-check (type stripping).

I propose we do this in a minor release like 1.5.0. Deno is still very much under development and the semver only applies to the Deno namespace and only without the --unstable flag.

[nayeemrmn]

There's one annoying behaviour of isolatedModules which doesn't make sense for Deno to exhibit:

console.log("Hello, world!");

error: TS1208 [ERROR]: All files must be modules when the '--isolatedModules' flag is provided.

So any toy module which doesn't use import/export needs export {}; to disambiguate from traditional scripts, even though Deno doesn't run those.

Ideally, we would wait for another option to make the compiler assume every source is a module and enable that at the same time (microsoft/TypeScript#27535). Maybe this will be the behaviour of --isolatedModules itself (microsoft/TypeScript#18232 (comment))!. This is a blocker IMO. If we're in a hurry, I'm also in favour of modifying tsc ourselves.

[kitsonk]

I really don't want to get into modifying tsc directly.

Ultimately it is a valid "problem" with the module specification, that you can't disambiguate scripts and modules. There is a proposal to solve this: https://github.com/tc39/proposal-modules-pragma though it doesn't have a lot of traction and movement.

I think realistically it is something that will be encountered rarely with Deno. I suspect we can ignored TS1208 like we do with other diagnostics with no ill effect. If we don't want to ignore it, we should rewrite it to indicate that they might want to use export {} to ensure wider compatibility.

[nayeemrmn]

Ultimately it is a valid "problem" with the module specification, that you can't disambiguate scripts and modules.

That problem isn't applicable to us. The host environment should already be able to provide this information out-of-band. It's just TS not allowing us to supply that.

I suspect we can ignored TS1208 like we do with other diagnostics with no ill effect.

That would be great (that's along the lines of what I meant to modify).

[lucacasonato]

Status update: 1.4.0 is going to enable this by default in --unstable with override in tsconfig being possible.

[nayeemrmn]

It's looking like we'll need to enable preserveConstEnums along with isolatedModules.

Currently, isolatedModules emits will break at runtime when you import and use a const enum. Ideally doing this would cause an isolatedModules related error, but it can't because it doesn't know that the import is a const enum on the other side because of isolatedModules 😅

#7326 (comment)

[bartlomieju]

@nayeemrmn isn't this diagnostic returned in such case? https://github.com/microsoft/TypeScript/blob/4a3b1957727625720eb48a83ee2dca42c55eda17/src/compiler/diagnosticMessages.json#L2858-L2861

[nayeemrmn]

@bartlomieju That's only for ambient ones i.e. declared globally with declare, I guess.

[bartlomieju]

@bartlomieju That's only for ambient ones i.e. declared globally with declare, I guess.

Then we should definitely enable that option as well and add a test case.

CC @lucacasonato

[nayeemrmn]

Wait, looking at the emits, I think isolatedModules already implies preserveConstEnums, and the bug is actually that the import is being stripped for some reason??

Repro:
temp2.ts:

export const enum a {
  b = 1,
  c = 2,
};

temp.ts:

import { a } from "./temp2.ts";
console.log(a.b);

temp2.ts.js with isolatedModules (already knows to preserve the const enum):

export var a;
(function (a) {
    a[a["b"] = 1] = "b";
    a[a["c"] = 2] = "c";
})(a || (a = {}));
;

temp.ts.js with isolatedModules (the import is left out, even though no inlining is done??):

console.log(a.b);
export {};

I can't find docs about this implication either. Must be a TS bug.

[kitsonk]

It doesn't imply const enum, const enums are not supported by transpilers. They are a type directed emit. I should be throwing.

[nayeemrmn]

@kitsonk The behaviour is it does imply preserveConstEnums. It's not documented anywhere so maybe it was snuck in by someone to recover more often. preserveConstEnums makes const enum emit like a regular enum so it's not strictly a type-directed emit. That much is documented.

[kitsonk]

I know what it does...

You are saying isolatedModules implicitly enables preserveConstEnums. I am saying it shouldn't, that instead it should error. I incorrectly assumed by imply you meant that the feature of isolated modules implies that const enums should be preserved. Language is imprecise.