-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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): improved npm specifier to import map entry mapping #22016
fix(lsp): improved npm specifier to import map entry mapping #22016
Conversation
matches | ||
.push(format!("{}{}", entry.raw_key, key_sub_path)); | ||
} | ||
} | ||
} | ||
} | ||
} |
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.
This code now goes from import map entry to npm package req and from that figures out a match instead of going from pkg req -> specifier, then calling import_map.lookup on the possible specifiers. We can no longer rely on a mapping from PkgReq -> specifier because we normalize and eliminate duplicate PackageReqs based on their RangeSetAndTag since denoland/deno_semver@98f9174
} else if let Some(file_stem) = file_name.strip_suffix(".d.mts") { | ||
search_paths | ||
.push(specifier_path.with_file_name(format!("{}.mjs", file_stem))); | ||
} |
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.
Found this situation while writing the test.
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.
Wow, that's a tricky one
@@ -1342,7 +1342,7 @@ dependencies = [ | |||
"import_map", | |||
"indexmap", | |||
"log", | |||
"monch", | |||
"monch 0.4.3", |
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.
This duplicate monch dep will be fixed in denoland/deno_graph#361
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
…d#22016) Upgrades to the latest deno_semver
Extracted out from #22004
Upgrades to the latest deno_semver