From 698ae4bfed1d4a88002be41cdbc24601ac71ddc1 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 17 Oct 2022 09:16:19 -0400 Subject: [PATCH] feat(unstable/npm): support providing npm dist-tag in npm package specifier (#16293) --- Cargo.lock | 4 +- cli/Cargo.toml | 2 +- cli/npm/resolution.rs | 82 ++++++++++++--------- cli/npm/semver/mod.rs | 8 +- cli/npm/semver/specifier.rs | 64 ++++++++++++++-- cli/tests/testdata/npm/dual_cjs_esm/main.ts | 4 +- 6 files changed, 112 insertions(+), 52 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0644d3ce29..2d06a30ed6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2894,9 +2894,9 @@ dependencies = [ [[package]] name = "monch" -version = "0.2.0" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "350537f27a69018269e534582e2f1ec532ea7078b06485fdd4db0509bd70feb8" +checksum = "c5e2e282addadb529bb31700f7d184797382fa2eb18384986aad78d117eaf0c4" [[package]] name = "naga" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 094a32d53b..ecc7cd32e3 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -85,7 +85,7 @@ jsonc-parser = { version = "=0.21.0", features = ["serde"] } libc = "=0.2.126" log = { version = "=0.4.17", features = ["serde"] } mitata = "=0.0.7" -monch = "=0.2.0" +monch = "=0.2.1" notify = "=5.0.0" once_cell = "=1.14.0" os_pipe = "=1.0.1" diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs index 497f409065..d53b677142 100644 --- a/cli/npm/resolution.rs +++ b/cli/npm/resolution.rs @@ -25,8 +25,8 @@ use super::semver::SpecifierVersionReq; /// The version matcher used for npm schemed urls is more strict than /// the one used by npm packages and so we represent either via a trait. pub trait NpmVersionMatcher { + fn tag(&self) -> Option<&str>; fn matches(&self, version: &NpmVersion) -> bool; - fn is_latest(&self) -> bool; fn version_text(&self) -> String; } @@ -124,15 +124,24 @@ impl std::fmt::Display for NpmPackageReq { } impl NpmVersionMatcher for NpmPackageReq { - fn matches(&self, version: &NpmVersion) -> bool { + fn tag(&self) -> Option<&str> { match &self.version_req { - Some(req) => req.matches(version), - None => version.pre.is_empty(), + Some(version_req) => version_req.tag(), + None => Some("latest"), } } - fn is_latest(&self) -> bool { - self.version_req.is_none() + fn matches(&self, version: &NpmVersion) -> bool { + match self.version_req.as_ref() { + Some(req) => { + assert_eq!(self.tag(), None); + match req.range() { + Some(range) => range.satisfies(version), + None => false, + } + } + None => version.pre.is_empty(), + } } fn version_text(&self) -> String { @@ -365,10 +374,10 @@ impl NpmResolution { pub async fn add_package_reqs( &self, - mut packages: Vec, + mut package_reqs: Vec, ) -> Result<(), AnyError> { // multiple packages are resolved in alphabetical order - packages.sort_by(|a, b| a.name.cmp(&b.name)); + package_reqs.sort_by(|a, b| a.name.cmp(&b.name)); // only allow one thread in here at a time let _permit = self.update_sempahore.acquire().await.unwrap(); @@ -377,29 +386,29 @@ impl NpmResolution { // go over the top level packages first, then down the // tree one level at a time through all the branches - for package_ref in packages { - if snapshot.package_reqs.contains_key(&package_ref) { + for package_req in package_reqs { + if snapshot.package_reqs.contains_key(&package_req) { // skip analyzing this package, as there's already a matching top level package continue; } // inspect the list of current packages if let Some(version) = - snapshot.resolve_best_package_version(&package_ref.name, &package_ref) + snapshot.resolve_best_package_version(&package_req.name, &package_req) { - snapshot.package_reqs.insert(package_ref, version); + snapshot.package_reqs.insert(package_req, version); continue; // done, no need to continue } // no existing best version, so resolve the current packages - let info = self.api.package_info(&package_ref.name).await?; + let info = self.api.package_info(&package_req.name).await?; let version_and_info = get_resolved_package_version_and_info( - &package_ref.name, - &package_ref, + &package_req.name, + &package_req, info, None, )?; let id = NpmPackageId { - name: package_ref.name.clone(), + name: package_req.name.clone(), version: version_and_info.version.clone(), }; let dependencies = version_and_info @@ -418,12 +427,12 @@ impl NpmResolution { ); snapshot .packages_by_name - .entry(package_ref.name.clone()) + .entry(package_req.name.clone()) .or_default() .push(version_and_info.version.clone()); snapshot .package_reqs - .insert(package_ref, version_and_info.version); + .insert(package_req, version_and_info.version); } // now go down through the dependencies by tree depth @@ -573,8 +582,8 @@ fn get_resolved_package_version_and_info( parent: Option<&NpmPackageId>, ) -> Result { let mut maybe_best_version: Option = None; - if version_matcher.is_latest() { - if let Some(version) = info.dist_tags.get("latest") { + if let Some(tag) = version_matcher.tag() { + if let Some(version) = info.dist_tags.get(tag) { match info.versions.get(version) { Some(info) => { return Ok(VersionAndInfo { @@ -584,26 +593,29 @@ fn get_resolved_package_version_and_info( } None => { bail!( - "Could not find version '{}' referenced in dist-tag 'latest'.", + "Could not find version '{}' referenced in dist-tag '{}'.", version, + tag, ) } } + } else { + bail!("Could not find dist-tag '{}'.", tag,) } - } - - for (_, version_info) in info.versions.into_iter() { - let version = NpmVersion::parse(&version_info.version)?; - if version_matcher.matches(&version) { - let is_best_version = maybe_best_version - .as_ref() - .map(|best_version| best_version.version.cmp(&version).is_lt()) - .unwrap_or(true); - if is_best_version { - maybe_best_version = Some(VersionAndInfo { - version, - info: version_info, - }); + } else { + for (_, version_info) in info.versions.into_iter() { + let version = NpmVersion::parse(&version_info.version)?; + if version_matcher.matches(&version) { + let is_best_version = maybe_best_version + .as_ref() + .map(|best_version| best_version.version.cmp(&version).is_lt()) + .unwrap_or(true); + if is_best_version { + maybe_best_version = Some(VersionAndInfo { + version, + info: version_info, + }); + } } } } diff --git a/cli/npm/semver/mod.rs b/cli/npm/semver/mod.rs index dd6ca03dbc..90352817fd 100644 --- a/cli/npm/semver/mod.rs +++ b/cli/npm/semver/mod.rs @@ -174,12 +174,12 @@ pub struct NpmVersionReq { } impl NpmVersionMatcher for NpmVersionReq { - fn matches(&self, version: &NpmVersion) -> bool { - self.satisfies(version) + fn tag(&self) -> Option<&str> { + None } - fn is_latest(&self) -> bool { - false + fn matches(&self, version: &NpmVersion) -> bool { + self.satisfies(version) } fn version_text(&self) -> String { diff --git a/cli/npm/semver/specifier.rs b/cli/npm/semver/specifier.rs index 220e0a6018..c3e7f716b0 100644 --- a/cli/npm/semver/specifier.rs +++ b/cli/npm/semver/specifier.rs @@ -10,13 +10,18 @@ use super::errors::with_failure_handling; use super::range::Partial; use super::range::VersionRange; use super::range::XRange; -use super::NpmVersion; + +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] +enum SpecifierVersionReqInner { + Range(VersionRange), + Tag(String), +} /// Version requirement found in npm specifiers. #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct SpecifierVersionReq { raw_text: String, - range: VersionRange, + inner: SpecifierVersionReqInner, } impl std::fmt::Display for SpecifierVersionReq { @@ -32,15 +37,46 @@ impl SpecifierVersionReq { }) } - pub fn matches(&self, version: &NpmVersion) -> bool { - self.range.satisfies(version) + pub fn range(&self) -> Option<&VersionRange> { + match &self.inner { + SpecifierVersionReqInner::Range(range) => Some(range), + SpecifierVersionReqInner::Tag(_) => None, + } + } + + pub fn tag(&self) -> Option<&str> { + match &self.inner { + SpecifierVersionReqInner::Range(_) => None, + SpecifierVersionReqInner::Tag(tag) => Some(tag.as_str()), + } } } fn parse_npm_specifier(input: &str) -> ParseResult { - map(version_range, |range| SpecifierVersionReq { - raw_text: input.to_string(), - range, + map_res(version_range, |result| { + let (new_input, range_result) = match result { + Ok((input, range)) => (input, Ok(range)), + // use an empty string because we'll consider the tag + Err(err) => ("", Err(err)), + }; + Ok(( + new_input, + SpecifierVersionReq { + raw_text: input.to_string(), + inner: match range_result { + Ok(range) => SpecifierVersionReqInner::Range(range), + Err(err) => { + // npm seems to be extremely lax on what it supports for a dist-tag (any non-valid semver range), + // so just make any error here be a dist tag unless it starts or ends with whitespace + if input.trim() != input { + return Err(err); + } else { + SpecifierVersionReqInner::Tag(input.to_string()) + } + } + }, + }, + )) })(input) } @@ -164,6 +200,8 @@ fn part(input: &str) -> ParseResult<&str> { #[cfg(test)] mod tests { + use crate::npm::semver::NpmVersion; + use super::*; struct VersionReqTester(SpecifierVersionReq); @@ -174,7 +212,11 @@ mod tests { } fn matches(&self, version: &str) -> bool { - self.0.matches(&NpmVersion::parse(version).unwrap()) + self + .0 + .range() + .map(|r| r.satisfies(&NpmVersion::parse(version).unwrap())) + .unwrap_or(false) } } @@ -250,4 +292,10 @@ mod tests { assert!(!tester.matches("0.1.0")); assert!(!tester.matches("1.0.0")); } + + #[test] + fn parses_tag() { + let latest_tag = SpecifierVersionReq::parse("latest").unwrap(); + assert_eq!(latest_tag.tag().unwrap(), "latest"); + } } diff --git a/cli/tests/testdata/npm/dual_cjs_esm/main.ts b/cli/tests/testdata/npm/dual_cjs_esm/main.ts index 6d973affb8..4f3b796678 100644 --- a/cli/tests/testdata/npm/dual_cjs_esm/main.ts +++ b/cli/tests/testdata/npm/dual_cjs_esm/main.ts @@ -1,5 +1,5 @@ -import { getKind } from "npm:@denotest/dual-cjs-esm"; -import * as cjs from "npm:@denotest/dual-cjs-esm/cjs/main.cjs"; +import { getKind } from "npm:@denotest/dual-cjs-esm@latest"; // test out @latest dist tag +import * as cjs from "npm:@denotest/dual-cjs-esm@latest/cjs/main.cjs"; console.log(getKind()); console.log(cjs.getKind());