feat(unstable): deno add subcommand
#22520
Conversation
|
We should also add the related |
bartlomieju
changed the title
deno add subcommanddeno add subcommand
cli/tools/registry/pm.rs
Outdated
| if version_req != "*" { | ||
| bail!("Specifying version constraints is currently not supported. Package: {}@{}", jsr_prefixed_name, version_req); | ||
| } |
There was a problem hiding this comment.
It would maybe be nice if the code used the same infrastructure that the jsr specifier completions uses in the lsp (but maybe with cache busting that happens on each run, i forget how it works). Then this would be more easily implemented. Maybe that could be done after #22612 lands
There was a problem hiding this comment.
I used code from @22612, but still left this part. We want to actually compute the best matching semver version here which I believe PackageSearchApi doesn't do.
| let mut import_list: Vec<(String, String)> = | ||
| existing_imports.into_iter().collect(); | ||
|
|
||
| import_list.sort_by(|(k1, _), (k2, _)| k1.cmp(k2)); |
There was a problem hiding this comment.
I don't think we should sort the user's import list. Instead maybe we should attempt to insert the dependency sorted from bottom up (ex. start at the bottom and keep decrementing the insert index until the previous item is less than the current)? People might have their imports sorted in some semantical way.
There was a problem hiding this comment.
I'm not sure it's worth doing for the first pass, it will get gnarly to handle that. Consider scenario like this:
"imports": {
"@denotest/add": "0.0.1",
"@foo/bar": "0.5.4",
"@david/dax": "0.2.1",
"express": "^4",
"@david/flag": "0.0.1"
}And calling deno add @denotest/add @luca/flag @david/dax @std/assert. We'd need to override existing @denotest/add (at first 1) entry then override "@david/dax" (at position 3) and then add "@luca/flag" and "@std/assert" and the end.
Since this is a first pass of the tool I think it's okay - once we have a way to mutate objects in jsonc-parser then we can tackle this in a more sophisticated way. WDYT?
There was a problem hiding this comment.
Ok, let's do this in a follow-up
| @@ -32,7 +32,7 @@ mod performance; | |||
| mod refactor; | |||
| mod registries; | |||
| mod repl; | |||
| mod search; | |||
| pub mod search; | |||
There was a problem hiding this comment.
In the future we should move these out of the lsp module to somewhere more common since we're going to be using this for more than the lsp now (cc @nayeemrmn)
There was a problem hiding this comment.
Looks like we're just using CliJsrSearchApi::versions() in jsr_find_package_and_select_version. I think we ought to be using JsrResolver::req_to_nv() there or something.. we can modify it to be usable with both fetch or cached-only logic. I'll look into it in the future.
There was a problem hiding this comment.
@nayeemrmn sounds like a good idea. Could you also improve deno add to handle npm: specifiers? For now I left it erroring (there are tests that verify it).
| let mut import_list: Vec<(String, String)> = | ||
| existing_imports.into_iter().collect(); | ||
|
|
||
| import_list.sort_by(|(k1, _), (k2, _)| k1.cmp(k2)); |
There was a problem hiding this comment.
Ok, let's do this in a follow-up
This commit adds "deno add" subcommand that has a basic support for adding
"jsr:" packages to "deno.json" file.
This currently doesn't support "npm:" specifiers and specifying version constraints.
Screen.Recording.2024-02-21.at.22.07.07.mov