summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Sherret <dsherret@users.noreply.github.com>2023-04-12 08:36:11 -0400
committerGitHub <noreply@github.com>2023-04-12 08:36:11 -0400
commit0e3f62d4446ae7b9a64dacf7befcaecede118222 (patch)
treefc1cbbbb294e61bb61e8d8ed89fa50cc9e9efa34
parent806671af3345f403d122911d8a3f09a2994bb8c0 (diff)
fix(npm): cache bust npm specifiers more aggressively (#18636)
Part 1: #18622 Part 2: This PR Closes #16901 --------- Co-authored-by: Luca Casonato <hello@lcas.dev>
-rw-r--r--Cargo.lock16
-rw-r--r--cli/Cargo.toml8
-rw-r--r--cli/args/mod.rs4
-rw-r--r--cli/lsp/documents.rs6
-rw-r--r--cli/lsp/language_server.rs15
-rw-r--r--cli/npm/installer.rs74
-rw-r--r--cli/npm/mod.rs2
-rw-r--r--cli/npm/registry.rs50
-rw-r--r--cli/npm/resolution.rs8
-rw-r--r--cli/proc_state.rs8
-rw-r--r--cli/resolver.rs38
-rw-r--r--cli/tests/integration/npm_tests.rs201
-rw-r--r--cli/tools/info.rs2
-rw-r--r--cli/tools/vendor/test.rs4
14 files changed, 291 insertions, 145 deletions
diff --git a/Cargo.lock b/Cargo.lock
index 2f8b54628..d6874d452 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -935,9 +935,9 @@ dependencies = [
[[package]]
name = "deno_doc"
-version = "0.60.0"
+version = "0.61.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "029ec20ba7a3c9d55597db7afa20576367ea8d70371a97b84f9909014cfe110f"
+checksum = "ae1ba6a3137da0ed19838c09c6fb9c7a07af642786b298fc29e088cc5643e729"
dependencies = [
"cfg-if",
"deno_ast",
@@ -953,9 +953,9 @@ dependencies = [
[[package]]
name = "deno_emit"
-version = "0.18.0"
+version = "0.19.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8004481b057addda0779edd5adb47e5ac9db7ae431c879300d22d535cc83cfae"
+checksum = "c01676751a0ee50ebad80734735f9a28c6eabb164050034e10956b72af563941"
dependencies = [
"anyhow",
"base64 0.13.1",
@@ -1017,9 +1017,9 @@ dependencies = [
[[package]]
name = "deno_graph"
-version = "0.46.0"
+version = "0.47.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1fb5531f3c2be6926e51ce5888fcffa434ca83516c53d26563882533aee871d0"
+checksum = "3e81896f3abfe0c6410518cc0285155e6faa2aa87ca8da32fbf1670ef1254ea2"
dependencies = [
"anyhow",
"data-url",
@@ -1734,9 +1734,9 @@ dependencies = [
[[package]]
name = "eszip"
-version = "0.39.0"
+version = "0.40.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "207f6568e7dde0c18eb306af104c4e7fe91f77eb99afeffd13f9c7735de4bb4d"
+checksum = "5a0a0addd73b5077a769e23a914a68ec8862c310b6127e8383505f676684f65c"
dependencies = [
"anyhow",
"base64 0.21.0",
diff --git a/cli/Cargo.toml b/cli/Cargo.toml
index 45a9eaf9d..2767f5f3a 100644
--- a/cli/Cargo.toml
+++ b/cli/Cargo.toml
@@ -42,9 +42,9 @@ winres.workspace = true
[dependencies]
deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_graph", "module_specifier", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] }
deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
-deno_doc = "0.60.0"
-deno_emit = "0.18.0"
-deno_graph = "=0.46.0"
+deno_doc = "0.61.0"
+deno_emit = "0.19.0"
+deno_graph = "=0.47.1"
deno_lint = { version = "0.43.0", features = ["docs"] }
deno_lockfile.workspace = true
deno_npm = "0.2.0"
@@ -70,7 +70,7 @@ dprint-plugin-markdown = "=0.15.2"
dprint-plugin-typescript = "=0.84.0"
encoding_rs.workspace = true
env_logger = "=0.9.0"
-eszip = "=0.39.0"
+eszip = "=0.40.0"
fancy-regex = "=0.10.0"
flate2.workspace = true
fs3.workspace = true
diff --git a/cli/args/mod.rs b/cli/args/mod.rs
index 20c382622..7029d6614 100644
--- a/cli/args/mod.rs
+++ b/cli/args/mod.rs
@@ -64,8 +64,8 @@ use std::sync::Arc;
use crate::cache::DenoDir;
use crate::file_fetcher::FileFetcher;
+use crate::npm::CliNpmRegistryApi;
use crate::npm::NpmProcessState;
-use crate::npm::NpmRegistry;
use crate::util::fs::canonicalize_path_maybe_not_exists;
use crate::version;
@@ -746,7 +746,7 @@ impl CliOptions {
pub async fn resolve_npm_resolution_snapshot(
&self,
- api: &NpmRegistry,
+ api: &CliNpmRegistryApi,
) -> Result<Option<NpmResolutionSnapshot>, AnyError> {
if let Some(state) = &*NPM_PROCESS_STATE {
// TODO(bartlomieju): remove this clone
diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs
index abd91d7fd..7b6ebdb41 100644
--- a/cli/lsp/documents.rs
+++ b/cli/lsp/documents.rs
@@ -20,8 +20,8 @@ use crate::lsp::logging::lsp_warn;
use crate::node;
use crate::node::node_resolve_npm_reference;
use crate::node::NodeResolution;
+use crate::npm::CliNpmRegistryApi;
use crate::npm::NpmPackageResolver;
-use crate::npm::NpmRegistry;
use crate::npm::NpmResolution;
use crate::npm::PackageJsonDepsInstaller;
use crate::resolver::CliGraphResolver;
@@ -1166,7 +1166,7 @@ impl Documents {
maybe_import_map: Option<Arc<import_map::ImportMap>>,
maybe_config_file: Option<&ConfigFile>,
maybe_package_json: Option<&PackageJson>,
- npm_registry_api: NpmRegistry,
+ npm_registry_api: CliNpmRegistryApi,
npm_resolution: NpmResolution,
) {
fn calculate_resolver_config_hash(
@@ -1864,7 +1864,7 @@ console.log(b, "hello deno");
#[test]
fn test_documents_refresh_dependencies_config_change() {
- let npm_registry_api = NpmRegistry::new_uninitialized();
+ let npm_registry_api = CliNpmRegistryApi::new_uninitialized();
let npm_resolution =
NpmResolution::new(npm_registry_api.clone(), None, None);
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index c672f76f0..85bff7fbf 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -79,9 +79,9 @@ use crate::graph_util;
use crate::http_util::HttpClient;
use crate::lsp::urls::LspUrlKind;
use crate::npm::create_npm_fs_resolver;
+use crate::npm::CliNpmRegistryApi;
use crate::npm::NpmCache;
use crate::npm::NpmPackageResolver;
-use crate::npm::NpmRegistry;
use crate::npm::NpmResolution;
use crate::proc_state::ProcState;
use crate::tools::fmt::format_file;
@@ -145,7 +145,7 @@ pub struct Inner {
/// A lazily create "server" for handling test run requests.
maybe_testing_server: Option<testing::TestServer>,
/// Npm's registry api.
- npm_api: NpmRegistry,
+ npm_api: CliNpmRegistryApi,
/// Npm cache
npm_cache: NpmCache,
/// Npm resolution that is stored in memory.
@@ -417,8 +417,13 @@ impl LanguageServer {
fn create_lsp_structs(
dir: &DenoDir,
http_client: HttpClient,
-) -> (NpmRegistry, NpmCache, NpmPackageResolver, NpmResolution) {
- let registry_url = NpmRegistry::default_url();
+) -> (
+ CliNpmRegistryApi,
+ NpmCache,
+ NpmPackageResolver,
+ NpmResolution,
+) {
+ let registry_url = CliNpmRegistryApi::default_url();
let progress_bar = ProgressBar::new(ProgressBarStyle::TextOnly);
let npm_cache = NpmCache::from_deno_dir(
dir,
@@ -430,7 +435,7 @@ fn create_lsp_structs(
http_client.clone(),
progress_bar.clone(),
);
- let api = NpmRegistry::new(
+ let api = CliNpmRegistryApi::new(
registry_url.clone(),
npm_cache.clone(),
http_client,
diff --git a/cli/npm/installer.rs b/cli/npm/installer.rs
index 41d85a9b9..5a15494ab 100644
--- a/cli/npm/installer.rs
+++ b/cli/npm/installer.rs
@@ -1,33 +1,69 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+use std::future::Future;
use std::sync::Arc;
use deno_core::error::AnyError;
use deno_core::futures::stream::FuturesOrdered;
use deno_core::futures::StreamExt;
use deno_npm::registry::NpmRegistryApi;
+use deno_npm::registry::NpmRegistryPackageInfoLoadError;
+use deno_semver::npm::NpmPackageReq;
use crate::args::package_json::PackageJsonDeps;
use crate::util::sync::AtomicFlag;
-use super::NpmRegistry;
+use super::CliNpmRegistryApi;
use super::NpmResolution;
#[derive(Debug)]
struct PackageJsonDepsInstallerInner {
has_installed_flag: AtomicFlag,
- npm_registry_api: NpmRegistry,
+ npm_registry_api: CliNpmRegistryApi,
npm_resolution: NpmResolution,
package_deps: PackageJsonDeps,
}
+impl PackageJsonDepsInstallerInner {
+ pub fn reqs(&self) -> Vec<&NpmPackageReq> {
+ let mut package_reqs = self
+ .package_deps
+ .values()
+ .filter_map(|r| r.as_ref().ok())
+ .collect::<Vec<_>>();
+ package_reqs.sort(); // deterministic resolution
+ package_reqs
+ }
+
+ pub fn reqs_with_info_futures(
+ &self,
+ ) -> FuturesOrdered<
+ impl Future<
+ Output = Result<
+ (&NpmPackageReq, Arc<deno_npm::registry::NpmPackageInfo>),
+ NpmRegistryPackageInfoLoadError,
+ >,
+ >,
+ > {
+ let package_reqs = self.reqs();
+
+ FuturesOrdered::from_iter(package_reqs.into_iter().map(|req| {
+ let api = self.npm_registry_api.clone();
+ async move {
+ let info = api.package_info(&req.name).await?;
+ Ok::<_, NpmRegistryPackageInfoLoadError>((req, info))
+ }
+ }))
+ }
+}
+
/// Holds and controls installing dependencies from package.json.
#[derive(Debug, Clone, Default)]
pub struct PackageJsonDepsInstaller(Option<Arc<PackageJsonDepsInstallerInner>>);
impl PackageJsonDepsInstaller {
pub fn new(
- npm_registry_api: NpmRegistry,
+ npm_registry_api: CliNpmRegistryApi,
npm_resolution: NpmResolution,
deps: Option<PackageJsonDeps>,
) -> Self {
@@ -57,27 +93,23 @@ impl PackageJsonDepsInstaller {
return Ok(()); // already installed by something else
}
- let mut package_reqs = inner
- .package_deps
- .values()
- .filter_map(|r| r.as_ref().ok())
- .collect::<Vec<_>>();
- package_reqs.sort(); // deterministic resolution
-
- let mut req_with_infos =
- FuturesOrdered::from_iter(package_reqs.into_iter().map(|req| {
- let api = inner.npm_registry_api.clone();
- async move {
- let info = api.package_info(&req.name).await?;
- Ok::<_, AnyError>((req, info))
- }
- }));
+ let mut reqs_with_info_futures = inner.reqs_with_info_futures();
- while let Some(result) = req_with_infos.next().await {
+ while let Some(result) = reqs_with_info_futures.next().await {
let (req, info) = result?;
- inner
+ let result = inner
.npm_resolution
- .resolve_package_req_as_pending_with_info(req, &info)?;
+ .resolve_package_req_as_pending_with_info(req, &info);
+ if let Err(err) = result {
+ if inner.npm_registry_api.mark_force_reload() {
+ log::debug!("Failed to resolve package. Retrying. Error: {err:#}");
+ // re-initialize
+ inner.npm_registry_api.clear_memory_cache();
+ reqs_with_info_futures = inner.reqs_with_info_futures();
+ } else {
+ return Err(err.into());
+ }
+ }
}
Ok(())
diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs
index 95a0a3017..8433a8f0c 100644
--- a/cli/npm/mod.rs
+++ b/cli/npm/mod.rs
@@ -10,7 +10,7 @@ mod tarball;
pub use cache::should_sync_download;
pub use cache::NpmCache;
pub use installer::PackageJsonDepsInstaller;
-pub use registry::NpmRegistry;
+pub use registry::CliNpmRegistryApi;
pub use resolution::NpmResolution;
pub use resolvers::create_npm_fs_resolver;
pub use resolvers::NpmPackageResolver;
diff --git a/cli/npm/registry.rs b/cli/npm/registry.rs
index a87365c9c..b38cfa898 100644
--- a/cli/npm/registry.rs
+++ b/cli/npm/registry.rs
@@ -53,9 +53,9 @@ static NPM_REGISTRY_DEFAULT_URL: Lazy<Url> = Lazy::new(|| {
});
#[derive(Clone, Debug)]
-pub struct NpmRegistry(Option<Arc<NpmRegistryApiInner>>);
+pub struct CliNpmRegistryApi(Option<Arc<CliNpmRegistryApiInner>>);
-impl NpmRegistry {
+impl CliNpmRegistryApi {
pub fn default_url() -> &'static Url {
&NPM_REGISTRY_DEFAULT_URL
}
@@ -66,7 +66,7 @@ impl NpmRegistry {
http_client: HttpClient,
progress_bar: ProgressBar,
) -> Self {
- Self(Some(Arc::new(NpmRegistryApiInner {
+ Self(Some(Arc::new(CliNpmRegistryApiInner {
base_url,
cache,
force_reload_flag: Default::default(),
@@ -104,14 +104,18 @@ impl NpmRegistry {
///
/// Returns true if it was successfully set for the first time.
pub fn mark_force_reload(&self) -> bool {
- // never force reload the registry information if reloading is disabled
- if matches!(self.inner().cache.cache_setting(), CacheSetting::Only) {
+ // never force reload the registry information if reloading
+ // is disabled or if we're already reloading
+ if matches!(
+ self.inner().cache.cache_setting(),
+ CacheSetting::Only | CacheSetting::ReloadAll
+ ) {
return false;
}
self.inner().force_reload_flag.raise()
}
- fn inner(&self) -> &Arc<NpmRegistryApiInner> {
+ fn inner(&self) -> &Arc<CliNpmRegistryApiInner> {
// this panicking indicates a bug in the code where this
// wasn't initialized
self.0.as_ref().unwrap()
@@ -122,7 +126,7 @@ static SYNC_DOWNLOAD_TASK_QUEUE: Lazy<TaskQueue> =
Lazy::new(TaskQueue::default);
#[async_trait]
-impl NpmRegistryApi for NpmRegistry {
+impl NpmRegistryApi for CliNpmRegistryApi {
async fn package_info(
&self,
name: &str,
@@ -147,16 +151,17 @@ impl NpmRegistryApi for NpmRegistry {
}
}
+type CacheItemPendingResult =
+ Result<Option<Arc<NpmPackageInfo>>, Arc<AnyError>>;
+
#[derive(Debug)]
enum CacheItem {
- Pending(
- Shared<BoxFuture<'static, Result<Option<Arc<NpmPackageInfo>>, String>>>,
- ),
+ Pending(Shared<BoxFuture<'static, CacheItemPendingResult>>),
Resolved(Option<Arc<NpmPackageInfo>>),
}
#[derive(Debug)]
-struct NpmRegistryApiInner {
+struct CliNpmRegistryApiInner {
base_url: Url,
cache: NpmCache,
force_reload_flag: AtomicFlag,
@@ -166,7 +171,7 @@ struct NpmRegistryApiInner {
progress_bar: ProgressBar,
}
-impl NpmRegistryApiInner {
+impl CliNpmRegistryApiInner {
pub async fn maybe_package_info(
self: &Arc<Self>,
name: &str,
@@ -196,9 +201,15 @@ impl NpmRegistryApiInner {
let future = {
let api = self.clone();
let name = name.to_string();
- async move { api.load_package_info_from_registry(&name).await }
- .boxed()
- .shared()
+ async move {
+ api
+ .load_package_info_from_registry(&name)
+ .await
+ .map(|info| info.map(Arc::new))
+ .map_err(Arc::new)
+ }
+ .boxed()
+ .shared()
};
mem_cache
.insert(name.to_string(), CacheItem::Pending(future.clone()));
@@ -220,11 +231,11 @@ impl NpmRegistryApiInner {
Err(err) => {
// purge the item from the cache so it loads next time
self.mem_cache.lock().remove(name);
- Err(anyhow!("{}", err))
+ Err(anyhow!("{:#}", err))
}
}
} else {
- Ok(future.await.map_err(|err| anyhow!("{}", err))?)
+ Ok(future.await.map_err(|err| anyhow!("{:#}", err))?)
}
}
@@ -303,7 +314,7 @@ impl NpmRegistryApiInner {
async fn load_package_info_from_registry(
&self,
name: &str,
- ) -> Result<Option<Arc<NpmPackageInfo>>, String> {
+ ) -> Result<Option<NpmPackageInfo>, AnyError> {
self
.load_package_info_from_registry_inner(name)
.await
@@ -314,9 +325,6 @@ impl NpmRegistryApiInner {
name
)
})
- .map(|info| info.map(Arc::new))
- // make cloneable
- .map_err(|err| format!("{err:#}"))
}
async fn load_package_info_from_registry_inner(
diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs
index d012b4f08..7d1619b94 100644
--- a/cli/npm/resolution.rs
+++ b/cli/npm/resolution.rs
@@ -27,7 +27,7 @@ use deno_semver::npm::NpmPackageReqReference;
use crate::args::Lockfile;
-use super::registry::NpmRegistry;
+use super::registry::CliNpmRegistryApi;
/// Handles updating and storing npm resolution in memory where the underlying
/// snapshot can be updated concurrently. Additionally handles updating the lockfile
@@ -38,7 +38,7 @@ use super::registry::NpmRegistry;
pub struct NpmResolution(Arc<NpmResolutionInner>);
struct NpmResolutionInner {
- api: NpmRegistry,
+ api: CliNpmRegistryApi,
snapshot: RwLock<NpmResolutionSnapshot>,
update_queue: TaskQueue,
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
@@ -55,7 +55,7 @@ impl std::fmt::Debug for NpmResolution {
impl NpmResolution {
pub fn new(
- api: NpmRegistry,
+ api: CliNpmRegistryApi,
initial_snapshot: Option<NpmResolutionSnapshot>,
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
) -> Self {
@@ -247,7 +247,7 @@ impl NpmResolution {
}
async fn add_package_reqs_to_snapshot(
- api: &NpmRegistry,
+ api: &CliNpmRegistryApi,
// todo(18079): it should be possible to pass &[NpmPackageReq] in here
// and avoid all these clones, but the LSP complains because of its
// `Send` requirement
diff --git a/cli/proc_state.rs b/cli/proc_state.rs
index d3a0618ef..659e2d1db 100644
--- a/cli/proc_state.rs
+++ b/cli/proc_state.rs
@@ -26,9 +26,9 @@ use crate::http_util::HttpClient;
use crate::node;
use crate::node::NodeResolution;
use crate::npm::create_npm_fs_resolver;
+use crate::npm::CliNpmRegistryApi;
use crate::npm::NpmCache;
use crate::npm::NpmPackageResolver;
-use crate::npm::NpmRegistry;
use crate::npm::NpmResolution;
use crate::npm::PackageJsonDepsInstaller;
use crate::resolver::CliGraphResolver;
@@ -95,7 +95,7 @@ pub struct Inner {
pub resolver: Arc<CliGraphResolver>,
maybe_file_watcher_reporter: Option<FileWatcherReporter>,
pub node_analysis_cache: NodeAnalysisCache,
- pub npm_api: NpmRegistry,
+ pub npm_api: CliNpmRegistryApi,
pub npm_cache: NpmCache,
pub npm_resolver: NpmPackageResolver,
pub npm_resolution: NpmResolution,
@@ -233,14 +233,14 @@ impl ProcState {
let lockfile = cli_options.maybe_lock_file();
- let npm_registry_url = NpmRegistry::default_url().to_owned();
+ let npm_registry_url = CliNpmRegistryApi::default_url().to_owned();
let npm_cache = NpmCache::from_deno_dir(
&dir,
cli_options.cache_setting(),
http_client.clone(),
progress_bar.clone(),
);
- let npm_api = NpmRegistry::new(
+ let npm_api = CliNpmRegistryApi::new(
npm_registry_url.clone(),
npm_cache.clone(),
http_client.clone(),
diff --git a/cli/resolver.rs b/cli/resolver.rs
index 56bae25c1..9aaa9d8b6 100644
--- a/cli/resolver.rs
+++ b/cli/resolver.rs
@@ -1,27 +1,26 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
use deno_core::anyhow::anyhow;
-use deno_core::anyhow::bail;
use deno_core::error::AnyError;
use deno_core::futures::future;
use deno_core::futures::future::LocalBoxFuture;
use deno_core::futures::FutureExt;
use deno_core::ModuleSpecifier;
use deno_core::TaskQueue;
+use deno_graph::source::NpmPackageReqResolution;
use deno_graph::source::NpmResolver;
use deno_graph::source::Resolver;
use deno_graph::source::UnknownBuiltInNodeModuleError;
use deno_graph::source::DEFAULT_JSX_IMPORT_SOURCE_MODULE;
use deno_npm::registry::NpmRegistryApi;
use deno_runtime::deno_node::is_builtin_node_module;
-use deno_semver::npm::NpmPackageNv;
use deno_semver::npm::NpmPackageReq;
use import_map::ImportMap;
use std::sync::Arc;
use crate::args::package_json::PackageJsonDeps;
use crate::args::JsxImportSourceConfig;
-use crate::npm::NpmRegistry;
+use crate::npm::CliNpmRegistryApi;
use crate::npm::NpmResolution;
use crate::npm::PackageJsonDepsInstaller;
use crate::util::sync::AtomicFlag;
@@ -34,7 +33,7 @@ pub struct CliGraphResolver {
maybe_default_jsx_import_source: Option<String>,
maybe_jsx_import_source_module: Option<String>,
no_npm: bool,
- npm_registry_api: NpmRegistry,
+ npm_registry_api: CliNpmRegistryApi,
npm_resolution: NpmResolution,
package_json_deps_installer: PackageJsonDepsInstaller,
found_package_json_dep_flag: Arc<AtomicFlag>,
@@ -45,7 +44,7 @@ impl Default for CliGraphResolver {
fn default() -> Self {
// This is not ideal, but necessary for the LSP. In the future, we should
// refactor the LSP and force this to be initialized.
- let npm_registry_api = NpmRegistry::new_uninitialized();
+ let npm_registry_api = CliNpmRegistryApi::new_uninitialized();
let npm_resolution =
NpmResolution::new(npm_registry_api.clone(), None, None);
Self {
@@ -67,7 +66,7 @@ impl CliGraphResolver {
maybe_jsx_import_source_config: Option<JsxImportSourceConfig>,
maybe_import_map: Option<Arc<ImportMap>>,
no_npm: bool,
- npm_registry_api: NpmRegistry,
+ npm_registry_api: CliNpmRegistryApi,
npm_resolution: NpmResolution,
package_json_deps_installer: PackageJsonDepsInstaller,
) -> Self {
@@ -204,7 +203,7 @@ impl NpmResolver for CliGraphResolver {
fn load_and_cache_npm_package_info(
&self,
package_name: &str,
- ) -> LocalBoxFuture<'static, Result<(), String>> {
+ ) -> LocalBoxFuture<'static, Result<(), AnyError>> {
if self.no_npm {
// return it succeeded and error at the import site below
return Box::pin(future::ready(Ok(())));
@@ -224,7 +223,7 @@ impl NpmResolver for CliGraphResolver {
.package_info(&package_name)
.await
.map(|_| ())
- .map_err(|err| format!("{err:#}"));
+ .map_err(|err| err.into());
drop(permit);
result
}
@@ -234,17 +233,26 @@ impl NpmResolver for CliGraphResolver {
fn resolve_npm(
&self,
package_req: &NpmPackageReq,
- ) -> Result<NpmPackageNv, AnyError> {
+ ) -> NpmPackageReqResolution {
if self.no_npm {
- bail!("npm specifiers were requested; but --no-npm is specified")
+ return NpmPackageReqResolution::Err(anyhow!(
+ "npm specifiers were requested; but --no-npm is specified"
+ ));
}
- match self
+
+ let result = self
.npm_resolution
- .resolve_package_req_as_pending(package_req)
- {
- Ok(nv) => Ok(nv),
+ .resolve_package_req_as_pending(package_req);
+ match result {
+ Ok(nv) => NpmPackageReqResolution::Ok(nv),
Err(err) => {
- bail!("{:#} Try retrieving the latest npm package information by running with --reload", err)
+ if self.npm_registry_api.mark_force_reload() {
+ log::debug!("Restarting npm specifier resolution to check for new registry information. Error: {:#}", err);
+ self.npm_registry_api.clear_memory_cache();
+ NpmPackageReqResolution::ReloadRegistryInfo(err.into())
+ } else {
+ NpmPackageReqResolution::Err(err.into())
+ }
}
}
}
diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs
index 7cf402b16..35f3980e4 100644
--- a/cli/tests/integration/npm_tests.rs
+++ b/cli/tests/integration/npm_tests.rs
@@ -1590,6 +1590,23 @@ fn reload_info_not_found_cache_but_exists_remote() {
.remove(version);
}
+ fn remove_version_for_package(
+ deno_dir: &util::TempDir,
+ package: &str,
+ version: &str,
+ ) {
+ let registry_json_path =
+ format!("npm/localhost_4545/npm/registry/{}/registry.json", package);
+ let mut registry_json: Value =
+ serde_json::from_str(&deno_dir.read_to_string(&registry_json_path))
+ .unwrap();
+ remove_version(&mut registry_json, version);
+ deno_dir.write(
+ &registry_json_path,
+ serde_json::to_string(&registry_json).unwrap(),
+ );
+ }
+
// This tests that when a local machine doesn't have a version
// specified in a dependency that exists in the npm registry
let test_context = TestContextBuilder::for_npm()
@@ -1604,68 +1621,144 @@ fn reload_info_not_found_cache_but_exists_remote() {
);
// cache successfully to the deno_dir
- let output = test_context.new_command().args("cache main.ts").run();
- output.assert_matches_text(concat!(
- "Download http://localhost:4545/npm/registry/@denotest/esm-import-cjs-default\n",
- "Download http://localhost:4545/npm/registry/@denotest/cjs-default-export\n",
- "Download http://localhost:4545/npm/registry/@denotest/cjs-default-export/1.0.0.tgz\n",
- "Download http://localhost:4545/npm/registry/@denotest/esm-import-cjs-default/1.0.0.tgz\n",
- ));
-
- // modify the package information in the cache to remove the latest version
- let registry_json_path = "npm/localhost_4545/npm/registry/@denotest/cjs-default-export/registry.json";
- let mut registry_json: Value =
- serde_json::from_str(&deno_dir.read_to_string(registry_json_path)).unwrap();
- remove_version(&mut registry_json, "1.0.0");
- deno_dir.write(
- registry_json_path,
- serde_json::to_string(&registry_json).unwrap(),
- );
-
- // should error when `--cache-only` is used now because the version is not in the cache
let output = test_context
.new_command()
- .args("run --cached-only main.ts")
+ .args("cache main.ts npm:@denotest/esm-basic@1.0.0")
.run();
- output.assert_exit_code(1);
- output.assert_matches_text("error: Could not find npm package '@denotest/cjs-default-export' matching '^1.0.0'.\n");
-
- // now try running without it, it should download the package now
- let output = test_context.new_command().args("run main.ts").run();
output.assert_matches_text(concat!(
+ "Download http://localhost:4545/npm/registry/@denotest/esm-basic\n",
"Download http://localhost:4545/npm/registry/@denotest/esm-import-cjs-default\n",
"Download http://localhost:4545/npm/registry/@denotest/cjs-default-export\n",
- "Node esm importing node cjs\n[WILDCARD]",
+ "Download http://localhost:4545/npm/registry/@denotest/cjs-default-export/1.0.0.tgz\n",
+ "Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz\n",
+ "Download http://localhost:4545/npm/registry/@denotest/esm-import-cjs-default/1.0.0.tgz\n",
));
- output.assert_exit_code(0);
- // now remove the information for the top level package
- let registry_json_path = "npm/localhost_4545/npm/registry/@denotest/esm-import-cjs-default/registry.json";
- let mut registry_json: Value =
- serde_json::from_str(&deno_dir.read_to_string(registry_json_path)).unwrap();
- remove_version(&mut registry_json, "1.0.0");
- deno_dir.write(
- registry_json_path,
- serde_json::to_string(&registry_json).unwrap(),
- );
+ // test in dependency
+ {
+ // modify the package information in the cache to remove the latest version
+ remove_version_for_package(
+ deno_dir,
+ "@denotest/cjs-default-export",
+ "1.0.0",
+ );
+
+ // should error when `--cache-only` is used now because the version is not in the cache
+ let output = test_context
+ .new_command()
+ .args("run --cached-only main.ts")
+ .run();
+ output.assert_exit_code(1);
+ output.assert_matches_text("error: Could not find npm package '@denotest/cjs-default-export' matching '^1.0.0'.\n");
+
+ // now try running without it, it should download the package now
+ let output = test_context.new_command().args("run main.ts").run();
+ output.assert_matches_text(concat!(
+ "Download http://localhost:4545/npm/registry/@denotest/esm-import-cjs-default\n",
+ "Download http://localhost:4545/npm/registry/@denotest/cjs-default-export\n",
+ "Node esm importing node cjs\n[WILDCARD]",
+ ));
+ output.assert_exit_code(0);
+ }
- // should error again for --cached-only
- let output = test_context
- .new_command()
- .args("run --cached-only main.ts")
- .run();
- output.assert_exit_code(1);
- output.assert_matches_text(concat!(
- "error: Could not find npm package '@denotest/esm-import-cjs-default' matching '1.0.0'. Try retrieving the latest npm package information by running with --reload\n",
- " at file:///[WILDCARD]/main.ts:1:8\n",
- ));
+ // test in npm specifier
+ {
+ // now remove the information for the top level package
+ remove_version_for_package(
+ deno_dir,
+ "@denotest/esm-import-cjs-default",
+ "1.0.0",
+ );
+
+ // should error for --cached-only
+ let output = test_context
+ .new_command()
+ .args("run --cached-only main.ts")
+ .run();
+ output.assert_matches_text(concat!(
+ "error: Could not find npm package '@denotest/esm-import-cjs-default' matching '1.0.0'.\n",
+ " at file:///[WILDCARD]/main.ts:1:8\n",
+ ));
+ output.assert_exit_code(1);
+
+ // now try running, it should work
+ let output = test_context.new_command().args("run main.ts").run();
+ output.assert_matches_text(concat!(
+ "Download http://localhost:4545/npm/registry/@denotest/esm-import-cjs-default\n",
+ "Download http://localhost:4545/npm/registry/@denotest/cjs-default-export\n",
+ "Node esm importing node cjs\n[WILDCARD]",
+ ));
+ output.assert_exit_code(0);
+ }
- // now try running without it, it currently will error, but this should work in the future
- // todo(https://github.com/denoland/deno/issues/16901): fix this
- let output = test_context.new_command().args("run main.ts").run();
- output.assert_exit_code(1);
- output.assert_matches_text(concat!(
- "error: Could not find npm package '@denotest/esm-import-cjs-default' matching '1.0.0'. Try retrieving the latest npm package information by running with --reload\n",
- " at file:///[WILDCARD]/main.ts:1:8\n",
- ));
+ // test matched specifier in package.json
+ {
+ // write out a package.json and a new main.ts with a bare specifier
+ temp_dir.write("main.ts", "import '@denotest/esm-import-cjs-default';");
+ temp_dir.write(
+ "package.json",
+ r#"{ "dependencies": { "@denotest/esm-import-cjs-default": "1.0.0" }}"#,
+ );
+
+ // remove the top level package information again
+ remove_version_for_package(
+ deno_dir,
+ "@denotest/esm-import-cjs-default",
+ "1.0.0",
+ );
+
+ // should error for --cached-only
+ let output = test_context
+ .new_command()
+ .args("run --cached-only main.ts")
+ .run();
+ output.assert_matches_text(concat!(
+ "error: Could not find npm package '@denotest/esm-import-cjs-default' matching '1.0.0'.\n",
+ ));
+ output.assert_exit_code(1);
+
+ // now try running, it should work
+ let output = test_context.new_command().args("run main.ts").run();
+ output.assert_matches_text(concat!(
+ "Download http://localhost:4545/npm/registry/@denotest/esm-import-cjs-default\n",
+ "Download http://localhost:4545/npm/registry/@denotest/cjs-default-export\n",
+ "Initialize @denotest/cjs-default-export@1.0.0\n",
+ "Initialize @denotest/esm-import-cjs-default@1.0.0\n",
+ "Node esm importing node cjs\n[WILDCARD]",
+ ));
+ output.assert_exit_code(0);
+ }
+
+ // temp other dependency in package.json
+ {
+ // write out a package.json that has another dependency
+ temp_dir.write(
+ "package.json",
+ r#"{ "dependencies": { "@denotest/esm-import-cjs-default": "1.0.0", "@denotest/esm-basic": "1.0.0" }}"#,
+ );
+
+ // remove the dependency's version
+ remove_version_for_package(deno_dir, "@denotest/esm-basic", "1.0.0");
+
+ // should error for --cached-only
+ let output = test_context
+ .new_command()
+ .args("run --cached-only main.ts")
+ .run();
+ output.assert_matches_text(concat!(
+ "error: Could not find npm package '@denotest/esm-basic' matching '1.0.0'.\n",
+ ));
+ output.assert_exit_code(1);
+
+ // now try running, it should work and only initialize the new package
+ let output = test_context.new_command().args("run main.ts").run();
+ output.assert_matches_text(concat!(
+ "Download http://localhost:4545/npm/registry/@denotest/esm-basic\n",
+ "Download http://localhost:4545/npm/registry/@denotest/esm-import-cjs-default\n",
+ "Download http://localhost:4545/npm/registry/@denotest/cjs-default-export\n",
+ "Initialize @denotest/esm-basic@1.0.0\n",
+ "Node esm importing node cjs\n[WILDCARD]",
+ ));
+ output.assert_exit_code(0);
+ }
}
diff --git a/cli/tools/info.rs b/cli/tools/info.rs
index 566eb1387..3f11903ad 100644
--- a/cli/tools/info.rs
+++ b/cli/tools/info.rs
@@ -486,7 +486,7 @@ impl<'a> GraphDisplayContext<'a> {
colors::red("error:")
)
} else {
- writeln!(writer, "{} {}", colors::red("error:"), err)
+ writeln!(writer, "{} {:#}", colors::red("error:"), err)
}
}
Ok(None) => {
diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs
index 5b1f792c5..59bcb91f0 100644
--- a/cli/tools/vendor/test.rs
+++ b/cli/tools/vendor/test.rs
@@ -20,7 +20,7 @@ use deno_graph::ModuleGraph;
use import_map::ImportMap;
use crate::cache::ParsedSourceCache;
-use crate::npm::NpmRegistry;
+use crate::npm::CliNpmRegistryApi;
use crate::npm::NpmResolution;
use crate::npm::PackageJsonDepsInstaller;
use crate::resolver::CliGraphResolver;
@@ -264,7 +264,7 @@ async fn build_test_graph(
analyzer: &dyn deno_graph::ModuleAnalyzer,
) -> ModuleGraph {
let resolver = original_import_map.map(|m| {
- let npm_registry_api = NpmRegistry::new_uninitialized();
+ let npm_registry_api = CliNpmRegistryApi::new_uninitialized();
let npm_resolution =
NpmResolution::new(npm_registry_api.clone(), None, None);
let deps_installer = PackageJsonDepsInstaller::new(