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

fix(lsp): ignore errors on ambient module imports #27855

Merged
merged 11 commits into from
Jan 30, 2025

Conversation

nathanwhit
Copy link
Member

@nathanwhit nathanwhit commented Jan 28, 2025

This makes it so imports of ambient modules (e.g. $app/environment in svelte, any virtual module in vite, or other module provided by a bundler) don't error in the LSP.

The way this works is that when we request diagnostics from TSC, we also respond with the list of ambient modules. Then, in the diagnostics code, we save diagnostics (produced by deno) that may be invalidated as an ambient module and wait to publish the diagnostics until we've received the ambient modules from TSC.

The actual ambient modules you get from TSC can contain globs, e.g. *.css. So when we get new ambient modules, we compile them all into a regex and check erroring imports against that regex. Ambient modules should change rarely, so in most cases we should be using a pre-compiled regex, which executes in linear time (wrt the specifier length).

TODO:

@nathanwhit nathanwhit changed the title fix(lsp): don't surface resolution errors for ambient modules fix(lsp): ignore errors on ambient module imports Jan 28, 2025
cli/lsp/tsc.rs Outdated Show resolved Hide resolved
cli/tsc/98_lsp.js Show resolved Hide resolved
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, let's land with described limitations as this is a huge improvement already 👍

@nathanwhit nathanwhit enabled auto-merge (squash) January 29, 2025 23:46
@nathanwhit nathanwhit merged commit b7456fe into denoland:main Jan 30, 2025
18 checks passed
bartlomieju pushed a commit that referenced this pull request Jan 30, 2025
This makes it so imports of ambient modules (e.g. `$app/environment` in
svelte, any virtual module in vite, or other module provided by a
bundler) don't error in the LSP.

The way this works is that when we request diagnostics from TSC, we also
respond with the list of ambient modules. Then, in the diagnostics code,
we save diagnostics (produced by deno) that may be invalidated as an
ambient module and wait to publish the diagnostics until we've received
the ambient modules from TSC.

The actual ambient modules you get from TSC can contain globs, e.g.
`*.css`. So when we get new ambient modules, we compile them all into a
regex and check erroring imports against that regex. Ambient modules
should change rarely, so in most cases we should be using a pre-compiled
regex, which executes in linear time (wrt the specifier length).

TODO:
- Ideally we should only publish once, right now we publish with the
filtered specifiers and then the TSC ones
- deno check (#27633)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants