isolatedModules not supported?

[DetachHead]

i see several merged PRs that seem to have enabled isolatedModules by default (#7327, #8050), but that doesn't seem to be the case?

//test.ts
const foo = 1

no error when running deno run test.ts, and when specifying the compiler option explicitly, it gets ignored:

deno run --config deno.json test.ts         
Unsupported compiler options in "C:\user\deno.json".
  The following options were ignored:
    isolatedModules
Check file:///C:/user/test.ts

[bartlomieju]

isolatedModules are enabled by default and can't be disabled because Deno uses SWC besides TypeScript which is a transpiler and requires isolatedModules to work properly.

[DetachHead]

@bartlomieju I'm trying to enable it not disable it, because it doesn't seem to be enabled

In my example, it doesn't warn that there's no import or export statements in the file

[bartlomieju]

@bartlomieju I'm trying to enable it not disable it, because it doesn't seem to be enabled

In my example, it doesn't warn that there's no import or export statements in the file

It is enabled by default:

deno/cli/emit.rs

Line 156 in 85a2943

"isolatedModules": true,

Why would it warn that there's no import or export?

[KotlinIsland]

This is what I would expect to see if if isolatedModules was enabled.
https://www.typescriptlang.org/play?isolatedModules=true#code/MYewdgzgLgBAhjAvDAjEA

[bartlomieju]

I see, this diagnostic is actually supressed in Deno:

deno/cli/tsc/99_main_compiler.js

Lines 143 to 146 in 85a2943

	// TS1208: All files must be modules when the '--isolatedModules' flag is
	// provided. We can ignore because we guarantee that all files are
	// modules.
	1208,

@kitsonk will have more context on that

[bartlomieju]

For more explanation see https://deno.land/manual/typescript/faqs#why-are-you-forcing-me-to-use-isolated-modules-why-can#39t-i-use-const-enums-with-deno-why-do-i-need-to-do-export-type

[kitsonk]

As the commen states, we ignore the diagnostic. What issue is that causing?

[DetachHead]

@kitsonk the issue is that when an import or an export is not present, typescript assumes all symbols declared in the file are global, which obviously fails at runtime. imo this is a massive flaw in typescript's design and as long as there is a compiler option to warn against it, it should be enabled by default (or at the very least, there should be an option to enable it). (related: #10484)

i think it's odd/confusing that isolatedModules is only partially enabled, with no option to "fully" enable it, as well as no documentation explaining this behavior (to my knowledge)

[nayeemrmn]

@DetachHead That is tracked in #9593. It's an acknowledged issue on TypeScript's side which is less of a design limitation and more of a technical one microsoft/TypeScript#18232 (comment). They want to fix it. I wouldn't say warning against that case is a pivotal role of isolatedModules, there are more important reasons we have that enabled. Deno emits and executes all JS as ES modules no matter what, so the TS1208 error message doesn't make sense for us to keep and may just be confusing.

typescript assumes all symbols declared in the file are global, which obviously fails at runtime

What do you mean it fails at runtime? This issue for Deno results in sometimes getting bogus diagnostics and symbol conflicts when type-checking, as far as we know. Are you getting some kind of emit bug as well?

[kitsonk]

What do you mean it fails at runtime? This issue for Deno results in sometimes getting bogus diagnostics and symbol conflicts when type-checking, as far as we know. Are you getting some kind of emit bug as well?

Correct. All the runtime code we are aware of works properly. The only "problem" is that if you have more than one file open in the editor that aren't considered "modules" their symbols "conflict", and you can get misleading diagnostics that say there is a problem when there isn't a problem (or you think you have access to variables that you don't).

Real world modules that don't have any exports or imports that are run under Deno are pretty rare in the wild. Generated bundles might be an exception, but most people don't develop on generated bundles.

If we stopped suppressing the diagnostics produced by isolated modules, people who do run single file modules with no imports or exports would break, and if we surfaced it in the language server, it would be more confusing than helpful. #10484 is a suggestion that would produce a useful diagnostic and code action in the language server, but we would still want to suppress such things from the CLI.

[DetachHead]

the issue i had is that in the following example:

//main.ts
console.log(foo)

//foo.ts
const foo = 1

the language server incorrectly assumes foo is available in the global scope, so we get no error in main.ts when we try to use it.

what i didn't realise is that this problem seems to only be specific to the language server, and when you actually run it the check correctly identifies that foo doesn't exist in main.ts

C:\user>deno run main.ts
Check file:///C:/user/main.ts
error: TS2304 [ERROR]: Cannot find name 'foo'.
console.log(foo)

but i don't understand why the language server behaves differently here? is that not just as much a source of confusion?

Deno emits and executes all JS as ES modules no matter what, so the TS1208 error message doesn't make sense for us to keep and may just be confusing.

in this case, i think @KotlinIsland's suggestion of injecting export {} seems like an appropriate solution #10484 (comment). i wouldn't consider it any more "magic" than turning non-modules into modules at runtime - it would make sense to me that the language server should do the same.

[kitsonk]

console.log(foo)

but i don't understand why the language server behaves differently here?

Because the language server subtly differs in how it determines the scope of a program, because the "root" module of a project is ambigious to a language server, as we don't express a deno run argument. So each open file is evaluated on its own, looking at any dependencies it has. If/when the deno.jsonc take a "root"/"main" argument, this would allow us to align the behaviours.

The discussion in #10484 stands and we should continue any discussion there.