feat(unstable): enable isolatedModules by default

[lucacasonato]

This enables the isolatedModules tsconfig option by default when using --unstable. This is a stepping stone towards #7326 in 1.5.0.

This does not apply to bundle, because outFile and isolatedModules are not compatible.

[lucacasonato]

@nayeemrmn pointed out that this would lead to error: TS1208 [ERROR]: All files must be modules when the '--isolatedModules' flag is provided. for all modules that don't have an import or export statement, because TypeScript does not consider them modules. (This is why test are failing right now).

I think we can hack around this issue by rewriting all TypeScript files that do not contain an import or export with a export {} at the end of the file. We already SWC parse all TypeScript files, so this doesn't seem super crazy. This should never be visible to the user. We can remove this hack once TypeScript figures this out upstream: microsoft/TypeScript#18232 (comment)

cc @bartlomieju @kitsonk

[kitsonk]

Let's not hack files. Try adding it to ignored diagnostics.

[lucacasonato]

Annoyingly all errors related to --isolatedModules use TS1208. I think we will need to do more specific filtering to only filter All files must be modules when the '--isolatedModules' flag is provided.

Related question: does this error stop TS from actually checking the file, or is it just a warning (or error) it emits?

[nayeemrmn]

Related question: does this error stop TS from actually checking the file, or is it just a warning (or error) it emits?

I see it alongside other errors:
Should be fine to ignore.

[nayeemrmn]

Annoyingly all errors related to --isolatedModules use TS1208.

What other error are you seeing that uses this code? I'm looking at:

error: TS1205 [ERROR]: Re-exporting a type when the '--isolatedModules' flag is provided requires using 'export type'.

[kitsonk]

It doesn't prevent type checking. We might have to grep the specific text though so the other isolated module related ones are exposed. We haven't done that before, but we should in this case.

[lucacasonato]

Actually, it seems matching the code to be TS1208 will be enough - I just got confused because the error message changed 3 times in the last few years, so I thought the code could have different messages. It used to be Cannot compile namespaces when the '--isolatedModules' flag is provided., now it is All files must be modules when the '--isolatedModules' flag is provided., and in 4.1 it will be '{0}' cannot be compiled under '--isolatedModules' because it is considered a global script file. Add an import, export, or an empty 'export {}' statement to make it a module.

[nayeemrmn]

Does this mean it can't be turned off? 😕

[lucacasonato]

Yes

[lucacasonato]

Although I guess this is not strictly required - I guess we can let you disable it. I wouldn't see why though.

[nayeemrmn]

Although I guess this is not strictly required - I guess we can let you disable it. I wouldn't see why though.

So people can suppress the breakages that would occur, for the short term at least.

For the long term... I'm not sure what our rule is for this. Do we allow any setting that isn't nonsensical in Deno? Or do we only allow settings justified with a use case? I think it was the former.

[lucacasonato]

A static slice of all the compiler options that should be ignored that either have no effect on the compilation or would cause the emit to not work in Deno.

I guess disabling isolatedModules could cause the emit not to work if we stop using tsc for emit, and just do stripping via swc. The nice thing about defaulting here is that we can guarantee with some certainty that the swc emit will look like a tsc emit. This would also be useful for bundling.

I guess for this first pass we can remove this from the list so people can suppress breakages, and then once we actually have some code that makes the assumption that all typescript code works with isolatedModules, we can re-add it.

[kitsonk]

The rule is "anything that has no impact or would have an undesired impact on the emit." It doesn't have to break the emit.

That being said, introducing it in unstable is to get people to adapt to it, and there is certainly a use case where you want an unstable API but you can resolve all the isolatedModules fixes. We should add it as an ignored option at some point post 1.5.0, like 1.6.0, IMO, but allow it to be changed up to that point.

[bartlomieju]

LGTM 🚀

[iugo]

How to set isolatedModules to false for deno bundle or Deno.bundle()?

[lucacasonato]

You can not. Our infrastructure only works with isolatedModules: true.