Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 16 additions & 60 deletions src/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,11 @@ use deno_semver::VersionReq;
use deno_semver::jsr::JsrDepPackageReq;
use deno_semver::jsr::JsrPackageNvReference;
use deno_semver::jsr::JsrPackageReqReference;
use deno_semver::npm::NpmPackageNvReference;
use deno_semver::npm::NpmPackageReqReference;
use deno_semver::package::PackageNv;
use deno_semver::package::PackageNvReference;
use deno_semver::package::PackageReq;
use deno_semver::package::PackageReqReferenceParseError;
use deno_semver::package::PackageSubPath;
use futures::FutureExt;
use futures::future::LocalBoxFuture;
use futures::stream::FuturesOrdered;
Expand Down Expand Up @@ -1226,7 +1224,6 @@ impl Module {
Module::Js(module) => &module.specifier,
Module::Json(module) => &module.specifier,
Module::Wasm(module) => &module.specifier,
#[allow(deprecated)]
Module::Npm(module) => &module.specifier,
Module::Node(module) => &module.specifier,
Module::External(module) => &module.specifier,
Expand Down Expand Up @@ -1343,25 +1340,16 @@ static EMPTY_DEPS: std::sync::OnceLock<IndexMap<String, Dependency>> =
std::sync::OnceLock::new();

/// An npm package entrypoint.
///
/// Note that deno_graph does NOT store the resolved version of npm specifiers.
/// That needs to be retreived from the npm snapshot, which is the single
/// source of truth on that matter.
#[derive(Debug, Clone, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct NpmModule {
// We need to change the NpmModule to instead only store an NpmReqReference
// and then map the req reference through the npm snapshot instead (have a single
// source of truth). This is necessary because deno_npm now does deduplication
// and so it will update on the fly the req to nv mapping, but then these
// properties in deno_graph won't get updated.
#[deprecated(
since = "0.103.1",
note = "Map the npm package req ref through the npm snapshot instead."
)]
pub specifier: ModuleSpecifier,
#[deprecated(
since = "0.103.1",
note = "Map the npm package req ref through the npm snapshot instead."
)]
#[serde(skip_serializing)]
pub nv_reference: NpmPackageNvReference,
pub pkg_req_ref: NpmPackageReqReference,
}

/// Represents a module which is not statically analyzed and is only available
Expand Down Expand Up @@ -2200,8 +2188,6 @@ pub struct ModuleGraph {
pub imports: IndexMap<ModuleSpecifier, GraphImport>,
pub redirects: BTreeMap<ModuleSpecifier, ModuleSpecifier>,
#[serde(skip_serializing)]
pub npm_packages: IndexSet<PackageNv>,
#[serde(skip_serializing)]
pub has_node_specifier: bool,
#[serde(rename = "packages")]
#[serde(skip_serializing_if = "PackageSpecifiers::is_empty")]
Expand All @@ -2220,7 +2206,6 @@ impl ModuleGraph {
module_slots: Default::default(),
imports: Default::default(),
redirects: Default::default(),
npm_packages: Default::default(),
has_node_specifier: false,
packages: Default::default(),
npm_dep_graph_result: Ok(()),
Expand Down Expand Up @@ -2417,7 +2402,6 @@ impl ModuleGraph {
}
new_graph.imports.clone_from(&self.imports);
new_graph.roots = roots.iter().map(|r| (*r).to_owned()).collect();
new_graph.npm_packages.clone_from(&self.npm_packages);
// todo(dsherret): it should be a bit smarter about this, but this is not terrible
new_graph.packages.clone_from(&self.packages);
new_graph.has_node_specifier = self.has_node_specifier;
Expand All @@ -2437,7 +2421,6 @@ impl ModuleGraph {
let mut seen_pending =
SeenPendingCollection::with_capacity(specifiers_count);
seen_pending.extend(self.roots.iter().cloned());
let mut found_nvs = HashSet::with_capacity(self.npm_packages.len());
let mut has_node_specifier = false;

let handle_dependencies =
Expand Down Expand Up @@ -2489,10 +2472,7 @@ impl ModuleGraph {
wasm_module.source_dts = Default::default();
handle_dependencies(&mut seen_pending, &mut wasm_module.dependencies);
}
Module::Npm(module) => {
#[allow(deprecated)]
found_nvs.insert(module.nv_reference.nv().clone());
}
Module::Npm(_) => {}
Module::Node(_) => {
has_node_specifier = true;
}
Expand All @@ -2507,7 +2487,6 @@ impl ModuleGraph {
self
.module_slots
.retain(|specifier, _| seen_pending.has_seen(specifier));
self.npm_packages.retain(|nv| found_nvs.contains(nv));
self
.redirects
.retain(|redirect, _| seen_pending.has_seen(redirect));
Expand Down Expand Up @@ -6344,7 +6323,6 @@ fn validate_jsr_specifier(
/// Pending information to insert into the module graph once
/// npm specifier resolution has been finalized.
struct NpmSpecifierBuildPendingInfo {
found_pkg_nvs: IndexSet<PackageNv>,
module_slots: HashMap<ModuleSpecifier, ModuleSlot>,
dependencies_resolution: Option<Result<(), Arc<dyn JsErrorClass>>>,
redirects: HashMap<ModuleSpecifier, ModuleSpecifier>,
Expand All @@ -6353,7 +6331,6 @@ struct NpmSpecifierBuildPendingInfo {
impl NpmSpecifierBuildPendingInfo {
pub fn with_capacity(capacity: usize) -> Self {
Self {
found_pkg_nvs: IndexSet::with_capacity(capacity),
module_slots: HashMap::with_capacity(capacity),
dependencies_resolution: None,
redirects: HashMap::with_capacity(capacity),
Expand Down Expand Up @@ -6451,11 +6428,10 @@ impl<'a> NpmSpecifierResolver<'a> {
let items = items_by_req.get(&req).unwrap();
for item in items {
match &resolution {
Ok(pkg_nv) => {
self.add_nv_for_item(
Ok(_) => {
self.add_req_ref_for_item(
item.specifier.clone(),
pkg_nv.clone(),
item.package_ref.sub_path().map(PackageSubPath::from_str),
item.package_ref.clone(),
);
}
Err(err) => {
Expand Down Expand Up @@ -6483,7 +6459,7 @@ impl<'a> NpmSpecifierResolver<'a> {
.await;
assert_eq!(result.results.len(), 1);
match result.results.remove(0) {
Ok(pkg_nv) => {
Ok(_) => {
if let Err(err) = result.dep_graph_result {
self.pending_info.module_slots.insert(
item.specifier.clone(),
Expand All @@ -6497,11 +6473,7 @@ impl<'a> NpmSpecifierResolver<'a> {
),
);
} else {
self.add_nv_for_item(
item.specifier,
pkg_nv,
item.package_ref.into_inner().sub_path,
);
self.add_req_ref_for_item(item.specifier, item.package_ref);
}
}
Err(err) => {
Expand All @@ -6521,31 +6493,16 @@ impl<'a> NpmSpecifierResolver<'a> {
}
}

fn add_nv_for_item(
fn add_req_ref_for_item(
&mut self,
specifier: ModuleSpecifier,
pkg_nv: PackageNv,
sub_path: Option<SmallStackString>,
pkg_req_ref: NpmPackageReqReference,
) {
let pkg_id_ref = NpmPackageNvReference::new(PackageNvReference {
nv: pkg_nv.clone(),
sub_path,
});
let resolved_specifier = pkg_id_ref.as_specifier();
if resolved_specifier != specifier {
self
.pending_info
.redirects
.insert(specifier.clone(), resolved_specifier.clone());
}
self.pending_info.found_pkg_nvs.insert(pkg_nv);
self.pending_info.module_slots.insert(
resolved_specifier.clone(),
specifier.clone(),
ModuleSlot::Module(Module::Npm(NpmModule {
#[allow(deprecated)]
specifier: resolved_specifier,
#[allow(deprecated)]
nv_reference: pkg_id_ref,
specifier,
pkg_req_ref,
})),
);
}
Expand All @@ -6563,7 +6520,6 @@ impl<'a> NpmSpecifierResolver<'a> {
graph.redirects.entry(key).or_insert(value);
}

graph.npm_packages.extend(pending_info.found_pkg_nvs);
if let Some(result) = pending_info.dependencies_resolution {
graph.npm_dep_graph_result = result;
}
Expand Down
11 changes: 9 additions & 2 deletions src/source/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::sync::Arc;
use std::time::SystemTime;

use async_trait::async_trait;
use deno_error::JsErrorBox;
use deno_error::JsErrorClass;
use deno_media_type::MediaType;
use deno_media_type::data_url::RawDataUrl;
Expand Down Expand Up @@ -472,6 +473,12 @@ pub enum ResolveError {
Other(#[from] deno_error::JsErrorBox),
}

impl ResolveError {
pub fn from_err<T: JsErrorClass>(err: T) -> Self {
Self::Other(JsErrorBox::from_err(err))
}
}

/// The kind of resolution currently being done by deno_graph.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum ResolutionKind {
Expand Down Expand Up @@ -559,9 +566,9 @@ pub struct NpmResolvePkgReqsResult {
/// The individual results of resolving the package requirements.
///
/// This MUST correspond to the indexes of the provided package requirements.
pub results: Vec<Result<PackageNv, NpmLoadError>>,
pub results: Vec<Result<(), NpmLoadError>>,
/// Result of resolving the entire dependency graph after the initial reqs
/// were resolved to NVs.
/// were resolved.
///
/// Don't run dependency graph resolution if there are any individual failures.
pub dep_graph_result: Result<(), Arc<dyn JsErrorClass>>,
Expand Down
5 changes: 1 addition & 4 deletions tests/helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,7 @@ impl NpmResolver for TestNpmResolver {
.iter()
.map(|pkg_req| {
match Version::parse_from_npm(&pkg_req.version_req.to_string()) {
Ok(version) => Ok(PackageNv {
name: pkg_req.name.clone(),
version,
}),
Ok(_) => Ok(()),
Err(err) => Err(NpmLoadError::PackageReqResolution(Arc::new(err))),
}
})
Expand Down
19 changes: 4 additions & 15 deletions tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use deno_graph::source::ResolveError;
use deno_semver::jsr::JsrDepPackageReq;
use deno_semver::package::PackageNv;
use deno_semver::package::PackageReq;
use indexmap::IndexSet;
use pretty_assertions::assert_eq;
use serde_json::json;
use sys_traits::FsCreateDirAll;
Expand Down Expand Up @@ -1050,10 +1049,6 @@ await import("https://example.com/main.ts");
graph.valid().unwrap();
graph.prune_types();
assert_eq!(graph.graph_kind(), GraphKind::CodeOnly);
assert_eq!(
graph.npm_packages,
IndexSet::from([PackageNv::from_str("chalk@1.0.0").unwrap()])
);
assert_eq!(
json!(graph),
json!({
Expand Down Expand Up @@ -1135,12 +1130,10 @@ await import("https://example.com/main.ts");
},
{
"kind": "npm",
"specifier": "npm:/chalk@1.0.0"
"specifier": "npm:chalk@1.0.0"
}
],
"redirects": {
"npm:chalk@1.0.0": "npm:/chalk@1.0.0"
}
"redirects": {}
})
);
}
Expand Down Expand Up @@ -1179,10 +1172,6 @@ async fn test_reload() {
.await;

graph.valid().unwrap();
assert_eq!(
graph.npm_packages,
IndexSet::from([PackageNv::from_str("chalk@1.0.0").unwrap()])
);
assert_eq!(
json!(graph),
json!({
Expand Down Expand Up @@ -1246,9 +1235,9 @@ async fn test_reload() {
"mediaType": "TypeScript",
"specifier": "file:///project/mod.ts"
},
{ "kind": "npm", "specifier": "npm:/chalk@1.0.0" }
{ "kind": "npm", "specifier": "npm:chalk@1.0.0" }
],
"redirects": { "npm:chalk@1.0.0": "npm:/chalk@1.0.0" }
"redirects": {}
})
);
}
Expand Down
5 changes: 2 additions & 3 deletions tests/specs/graph/fast_check/npm_types.txt
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,11 @@ import 'jsr:@scope/a'
},
{
"kind": "npm",
"specifier": "npm:/@denotest/test@0.2.0"
"specifier": "npm:@denotest/test@0.2.0"
}
],
"redirects": {
"jsr:@scope/a": "https://jsr.io/@scope/a/1.0.0/mod.ts",
"npm:@denotest/test@0.2.0": "npm:/@denotest/test@0.2.0"
"jsr:@scope/a": "https://jsr.io/@scope/a/1.0.0/mod.ts"
},
"packages": {
"@scope/a@*": "@scope/a@1.0.0"
Expand Down