-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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: update lsp error message of 'relative import path' to 'use deno add' for npm/jsr packages #24524
Conversation
…add' for npm/jsr packages
Hello @lucacasonato , |
@nayeemrmn PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work when there's no import map? Also needs a test.
cli/lsp/diagnostics.rs
Outdated
if let ResolveError::Other(resolve_error, ..) = (*error).as_ref() { | ||
if let Some(ImportMapError::UnmappedBareSpecifier(specifier, _)) = resolve_error.downcast_ref::<ImportMapError>() { | ||
if specifier.chars().next().unwrap_or('\0') == '@'{ | ||
message = format!("Use [deno add {}] to add the dependency.", specifier); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the hint should be appended to the original message here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…add' for npm/jsr packages (#24524)
This PR addresses #24033
Problem
LSP shows a message saying that the relative import path isn't prefixed with / or ./ or ../, while the package could be from npm or jsr.
Fix
This type of message arises when a Resolution error is received within the
fn to_lsp_diagnostic
in cli/lsp/diagnostics.rs.Then a check is needed to see if the package name starts with @ in order to know if it's local or from npm/jsr.
After that the message can be updated to tell the user to use
deno add
in order to add the dependency.Screenshot of the fix