Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lsp: diagnostic on module with no imports or exports #10484

Closed
kitsonk opened this issue May 3, 2021 · 10 comments
Closed

lsp: diagnostic on module with no imports or exports #10484

kitsonk opened this issue May 3, 2021 · 10 comments
Labels
lsp related to the language server suggestion suggestions for new features (yet to be agreed)

Comments

@kitsonk
Copy link
Contributor

kitsonk commented May 3, 2021

Something like "warning, this file doesn't contain any imports/exports and is therefore falsely considered global"

Originally posted by @KotlinIsland in denoland/vscode_deno#405 (comment)


TypeScript has a design limitation where scripts are considered global scripts when they lack any import or export statements. While this does not cause any issues with the CLI, it does mean these symbols "leak" into the global context when using the LSP. We could provide a diagnostic that indicates this, with a quick fix which would be to append export {} to the file.

@kitsonk kitsonk added lsp related to the language server suggestion suggestions for new features (yet to be agreed) labels May 3, 2021
@DetachHead
Copy link

DetachHead commented Oct 30, 2021

the isolatedModules compiler option should cover this, but it doesn't seem to be supported anymore? #12599

@KotlinIsland
Copy link

Does Deno support scripts at all? If not you could just secretly inject export {} so the LSP thinks it's a module

@kitsonk
Copy link
Contributor Author

kitsonk commented Oct 30, 2021

We should avoid "magically" augmenting the source code.

As stated, we should provide a quick fix that allows the author to fix it themselves.

@DetachHead
Copy link

imo it's also bad to "magically" half-enable a compiler flag. if there's a reason that isolatedModules works differently in deno can the documentation be updated to explain it?

@KotlinIsland
Copy link

KotlinIsland commented Oct 31, 2021

Wouldn't adding a diagnostic like this just be the exact same as not disabling the error message in the first place?

"'input.ts' 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."

@kitsonk
Copy link
Contributor Author

kitsonk commented Oct 31, 2021

We disable error messages equally at the moment across the language server and when checking on the CLI. The diagnostic is meaningless on the CLI, the reason why it is disabled, and misleading from the language server as it would complain about the file being a script which doesn't work with isolated modules, something the user has no control over.

So to implement the feature we would have to determine which context we are in, suppress it in one and rewrite it in another as well as inject a code action.

@KotlinIsland
Copy link

KotlinIsland commented Oct 31, 2021

Get typescript to add a "scripts-are-modules" mode that all it does is injects a export {} during analysis. Then use that.

@kitsonk
Copy link
Contributor Author

kitsonk commented Oct 31, 2021

The whole TypeScript issue is covered at microsoft/TypeScript#18232, including your suggestion, so it isn't like it hasn't been discussed before. We also have #9593 that covers the topic as well.

@kitsonk
Copy link
Contributor Author

kitsonk commented Sep 29, 2022

This should no longer be an issue, as we force module consideration on TypeScript.

@kitsonk kitsonk closed this as completed Sep 29, 2022
@KotlinIsland
Copy link

KotlinIsland commented Sep 29, 2022

@kitsonk Awesome news! I'm interested in this, could you link to some more info please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lsp related to the language server suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

3 participants