denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
94.67k stars 5.25k forks source link

Deno panic when importing jsr package with @next label #22420

Closed cotyhamilton closed 7 months ago

cotyhamilton commented 7 months ago

deno 1.40.2 (release, aarch64-apple-darwin) v8 12.1.285.6 typescript 5.3.3

import { DenoKVAdapter } from "jsr:@cotyhamilton/lucia-adapter-denokv@next";
RUST_BACKTRACE=1 deno run --unstable-kv issue.ts

============================================================
Deno has panicked. This is a bug in Deno. Please report this
at https://github.com/denoland/deno/issues/new.
If you can reliably reproduce this panic, include the
reproduction steps and re-run with the RUST_BACKTRACE=1 env
var set and include the backtrace in your report.

Platform: macos aarch64
Version: 1.40.2
Args: ["deno", "run", "--unstable-kv", "issue.ts"]

thread 'main' panicked at /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/deno_semver-0.5.4/src/lib.rs:265:32:
programming error: cannot use matches with a tag: next
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: deno_semver::VersionReq::matches
   3: deno_graph::graph::Builder::resolve_jsr_version
   4: deno_graph::graph::Builder::fill::{{closure}}
   5: deno_graph::graph::ModuleGraph::build::{{closure}}
   6: deno::graph_util::ModuleGraphBuilder::build_graph_with_npm_resolution::{{closure}}
   7: deno::module_loader::ModuleLoadPreparer::prepare_module_load::{{closure}}
   8: <deno::module_loader::CliModuleLoader as deno_core::modules::loaders::ModuleLoader>::prepare_load::{{closure}}
   9: deno_core::modules::recursive_load::RecursiveModuleLoad::prepare::{{closure}}
  10: deno::worker::CliMainWorker::execute_main_module_possibly_with_npm::{{closure}}
  11: deno::worker::CliMainWorker::run::{{closure}}
  12: deno::tools::run::run_script::{{closure}}
  13: deno::spawn_subcommand::{{closure}}
  14: <deno_unsync::task::MaskFutureAsSend<F> as core::future::future::Future>::poll
  15: tokio::runtime::task::raw::poll
  16: deno::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Repro: https://github.com/cotyhamilton/deno-jsr-issue

irbull commented 7 months ago

This panics because semver won't allow a version to be compared with a tag. I think the API could be improved here, but if the API is carved in stone, then I think it should return false if a version is compared with a tag, it shouldn't panic.

irbull commented 7 months ago

After some discussion with @dsherret it was decided that:

I think we just need to change deno_graph to give a nice error message when loading with a tag.

and

I don't want to change the matches API because where it's used should have a VersionReq with a RangeSet and not a tag. If anything probably the code that's using matches should be working with RangeSets instead and call matches on that, but that's a very large refactor that I'm not sure is worth doing (or even should be done) and I wouldn't recommend taking it on at the moment because of the focus on shipping JSR.

I closed the attached PR. Let's see if we can fix this in deno_graph.