From 6f506208f60f710952b4ed363e557cb8a36e92b4 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> Date: Tue, 10 Dec 2024 18:24:23 -0800 Subject: [PATCH] feat(unstable): support caching npm dependencies only as they're needed (#27300) Currently deno eagerly caches all npm packages in the workspace's npm resolution. So, for instance, running a file `foo.ts` that imports `npm:chalk` will also install all dependencies listed in `package.json` and all `npm` dependencies listed in the lockfile. This PR refactors things to give more control over when and what npm packages are automatically cached while building the module graph. After this PR, by default the current behavior is unchanged _except_ for `deno install --entrypoint`, which will only cache npm packages used by the given entrypoint. For the other subcommands, this behavior can be enabled with `--unstable-npm-lazy-caching` Fixes #25782. --------- Signed-off-by: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> Co-authored-by: Luca Casonato --- cli/args/flags.rs | 76 ++++++++++--------- cli/args/lockfile.rs | 7 +- cli/args/mod.rs | 35 +++++++-- cli/factory.rs | 1 + cli/graph_util.rs | 26 ++++++- cli/lsp/language_server.rs | 7 +- cli/lsp/resolver.rs | 7 +- cli/module_loader.rs | 1 + cli/npm/managed/mod.rs | 65 ++++++++++++---- cli/npm/managed/resolution.rs | 4 + cli/npm/managed/resolvers/common.rs | 6 +- cli/npm/managed/resolvers/global.rs | 18 ++++- cli/npm/managed/resolvers/local.rs | 12 ++- cli/npm/mod.rs | 1 + cli/resolver.rs | 23 +++++- cli/standalone/binary.rs | 1 + cli/standalone/mod.rs | 1 + cli/tools/bench/mod.rs | 6 +- cli/tools/compile.rs | 6 +- cli/tools/doc.rs | 6 +- cli/tools/info.rs | 7 +- cli/tools/installer.rs | 7 +- cli/tools/registry/pm/cache_deps.rs | 41 ++++++---- cli/tools/repl/session.rs | 4 +- cli/tools/run/mod.rs | 18 ++++- cli/tools/task.rs | 12 +++ cli/tools/test/mod.rs | 6 +- cli/worker.rs | 17 ++++- .../__test__.jsonc | 13 ++++ .../install-entrypoint.out | 6 ++ .../entrypoint_only_used_packages/install.out | 2 + .../entrypoint_only_used_packages/main.ts | 3 + .../package.json | 6 ++ .../run/jsx_import_source/__test__.jsonc | 1 + tests/specs/run/lazy_npm/__test__.jsonc | 9 +++ tests/specs/run/lazy_npm/deno.json | 3 + tests/specs/run/lazy_npm/main.out | 7 ++ tests/specs/run/lazy_npm/main.ts | 3 + tests/specs/run/lazy_npm/package.json | 6 ++ 39 files changed, 371 insertions(+), 109 deletions(-) create mode 100644 tests/specs/install/entrypoint_only_used_packages/__test__.jsonc create mode 100644 tests/specs/install/entrypoint_only_used_packages/install-entrypoint.out create mode 100644 tests/specs/install/entrypoint_only_used_packages/install.out create mode 100644 tests/specs/install/entrypoint_only_used_packages/main.ts create mode 100644 tests/specs/install/entrypoint_only_used_packages/package.json create mode 100644 tests/specs/run/lazy_npm/__test__.jsonc create mode 100644 tests/specs/run/lazy_npm/deno.json create mode 100644 tests/specs/run/lazy_npm/main.out create mode 100644 tests/specs/run/lazy_npm/main.ts create mode 100644 tests/specs/run/lazy_npm/package.json diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 1bc2135ea5..9418739564 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -245,7 +245,7 @@ pub struct InstallFlagsGlobal { } #[derive(Clone, Debug, Eq, PartialEq)] -pub enum InstallKind { +pub enum InstallFlags { Local(InstallFlagsLocal), Global(InstallFlagsGlobal), } @@ -257,11 +257,6 @@ pub enum InstallFlagsLocal { Entrypoints(Vec), } -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct InstallFlags { - pub kind: InstallKind, -} - #[derive(Clone, Debug, Eq, PartialEq)] pub struct JSONReferenceFlags { pub json: deno_core::serde_json::Value, @@ -600,6 +595,7 @@ pub struct UnstableConfig { pub bare_node_builtins: bool, pub detect_cjs: bool, pub sloppy_imports: bool, + pub npm_lazy_caching: bool, pub features: Vec, // --unstabe-kv --unstable-cron } @@ -4407,6 +4403,16 @@ impl CommandExt for Command { }) .help_heading(UNSTABLE_HEADING) .display_order(next_display_order()) + ).arg( + Arg::new("unstable-npm-lazy-caching") + .long("unstable-npm-lazy-caching") + .help("Enable unstable lazy caching of npm dependencies, downloading them only as needed (disabled: all npm packages in package.json are installed on startup; enabled: only npm packages that are actually referenced in an import are installed") + .env("DENO_UNSTABLE_NPM_LAZY_CACHING") + .value_parser(FalseyValueParser::new()) + .action(ArgAction::SetTrue) + .hide(true) + .help_heading(UNSTABLE_HEADING) + .display_order(next_display_order()), ); for granular_flag in crate::UNSTABLE_GRANULAR_FLAGS.iter() { @@ -4920,15 +4926,14 @@ fn install_parse( let module_url = cmd_values.next().unwrap(); let args = cmd_values.collect(); - flags.subcommand = DenoSubcommand::Install(InstallFlags { - kind: InstallKind::Global(InstallFlagsGlobal { + flags.subcommand = + DenoSubcommand::Install(InstallFlags::Global(InstallFlagsGlobal { name, module_url, args, root, force, - }), - }); + })); return Ok(()); } @@ -4937,22 +4942,19 @@ fn install_parse( allow_scripts_arg_parse(flags, matches)?; if matches.get_flag("entrypoint") { let entrypoints = matches.remove_many::("cmd").unwrap_or_default(); - flags.subcommand = DenoSubcommand::Install(InstallFlags { - kind: InstallKind::Local(InstallFlagsLocal::Entrypoints( - entrypoints.collect(), - )), - }); + flags.subcommand = DenoSubcommand::Install(InstallFlags::Local( + InstallFlagsLocal::Entrypoints(entrypoints.collect()), + )); } else if let Some(add_files) = matches .remove_many("cmd") .map(|packages| add_parse_inner(matches, Some(packages))) { - flags.subcommand = DenoSubcommand::Install(InstallFlags { - kind: InstallKind::Local(InstallFlagsLocal::Add(add_files)), - }) + flags.subcommand = DenoSubcommand::Install(InstallFlags::Local( + InstallFlagsLocal::Add(add_files), + )) } else { - flags.subcommand = DenoSubcommand::Install(InstallFlags { - kind: InstallKind::Local(InstallFlagsLocal::TopLevel), - }); + flags.subcommand = + DenoSubcommand::Install(InstallFlags::Local(InstallFlagsLocal::TopLevel)); } Ok(()) } @@ -5998,6 +6000,8 @@ fn unstable_args_parse( flags.unstable_config.detect_cjs = matches.get_flag("unstable-detect-cjs"); flags.unstable_config.sloppy_imports = matches.get_flag("unstable-sloppy-imports"); + flags.unstable_config.npm_lazy_caching = + matches.get_flag("unstable-npm-lazy-caching"); if matches!(cfg, UnstableArgsConfig::ResolutionAndRuntime) { for granular_flag in crate::UNSTABLE_GRANULAR_FLAGS { @@ -8606,15 +8610,15 @@ mod tests { assert_eq!( r.unwrap(), Flags { - subcommand: DenoSubcommand::Install(InstallFlags { - kind: InstallKind::Global(InstallFlagsGlobal { + subcommand: DenoSubcommand::Install(InstallFlags::Global( + InstallFlagsGlobal { name: None, module_url: "jsr:@std/http/file-server".to_string(), args: vec![], root: None, force: false, - }), - }), + } + ),), ..Flags::default() } ); @@ -8628,15 +8632,15 @@ mod tests { assert_eq!( r.unwrap(), Flags { - subcommand: DenoSubcommand::Install(InstallFlags { - kind: InstallKind::Global(InstallFlagsGlobal { + subcommand: DenoSubcommand::Install(InstallFlags::Global( + InstallFlagsGlobal { name: None, module_url: "jsr:@std/http/file-server".to_string(), args: vec![], root: None, force: false, - }), - }), + } + ),), ..Flags::default() } ); @@ -8649,15 +8653,15 @@ mod tests { assert_eq!( r.unwrap(), Flags { - subcommand: DenoSubcommand::Install(InstallFlags { - kind: InstallKind::Global(InstallFlagsGlobal { + subcommand: DenoSubcommand::Install(InstallFlags::Global( + InstallFlagsGlobal { name: Some("file_server".to_string()), module_url: "jsr:@std/http/file-server".to_string(), args: svec!["foo", "bar"], root: Some("/foo".to_string()), force: true, - }), - }), + } + ),), import_map_path: Some("import_map.json".to_string()), no_remote: true, config_flag: ConfigFlag::Path("tsconfig.json".to_owned()), @@ -11211,9 +11215,9 @@ mod tests { ..Flags::default() }, "install" => Flags { - subcommand: DenoSubcommand::Install(InstallFlags { - kind: InstallKind::Local(InstallFlagsLocal::Add(flags)), - }), + subcommand: DenoSubcommand::Install(InstallFlags::Local( + InstallFlagsLocal::Add(flags), + )), ..Flags::default() }, _ => unreachable!(), diff --git a/cli/args/lockfile.rs b/cli/args/lockfile.rs index 6c1a2ca0ef..74eab78f1c 100644 --- a/cli/args/lockfile.rs +++ b/cli/args/lockfile.rs @@ -20,7 +20,6 @@ use crate::Flags; use crate::args::DenoSubcommand; use crate::args::InstallFlags; -use crate::args::InstallKind; use deno_lockfile::Lockfile; @@ -136,10 +135,8 @@ impl CliLockfile { if flags.no_lock || matches!( flags.subcommand, - DenoSubcommand::Install(InstallFlags { - kind: InstallKind::Global(..), - .. - }) | DenoSubcommand::Uninstall(_) + DenoSubcommand::Install(InstallFlags::Global(..)) + | DenoSubcommand::Uninstall(_) ) { return Ok(None); diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 0b049cf409..314c0ff17a 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -970,9 +970,7 @@ impl CliOptions { match self.sub_command() { DenoSubcommand::Cache(_) => GraphKind::All, DenoSubcommand::Check(_) => GraphKind::TypesOnly, - DenoSubcommand::Install(InstallFlags { - kind: InstallKind::Local(_), - }) => GraphKind::All, + DenoSubcommand::Install(InstallFlags::Local(_)) => GraphKind::All, _ => self.type_check_mode().as_graph_kind(), } } @@ -1549,11 +1547,11 @@ impl CliOptions { DenoSubcommand::Check(check_flags) => { Some(files_to_urls(&check_flags.files)) } - DenoSubcommand::Install(InstallFlags { - kind: InstallKind::Global(flags), - }) => Url::parse(&flags.module_url) - .ok() - .map(|url| vec![Cow::Owned(url)]), + DenoSubcommand::Install(InstallFlags::Global(flags)) => { + Url::parse(&flags.module_url) + .ok() + .map(|url| vec![Cow::Owned(url)]) + } DenoSubcommand::Doc(DocFlags { source_files: DocSourceFileFlag::Paths(paths), .. @@ -1689,6 +1687,7 @@ impl CliOptions { "detect-cjs", "fmt-component", "fmt-sql", + "lazy-npm-caching", ]) .collect(); @@ -1767,6 +1766,19 @@ impl CliOptions { ), } } + + pub fn unstable_npm_lazy_caching(&self) -> bool { + self.flags.unstable_config.npm_lazy_caching + || self.workspace().has_unstable("npm-lazy-caching") + } + + pub fn default_npm_caching_strategy(&self) -> NpmCachingStrategy { + if self.flags.unstable_config.npm_lazy_caching { + NpmCachingStrategy::Lazy + } else { + NpmCachingStrategy::Eager + } + } } /// Resolves the path to use for a local node_modules folder. @@ -1981,6 +1993,13 @@ fn load_env_variables_from_env_file(filename: Option<&Vec>) { } } +#[derive(Debug, Clone, Copy)] +pub enum NpmCachingStrategy { + Eager, + Lazy, + Manual, +} + #[cfg(test)] mod test { use pretty_assertions::assert_eq; diff --git a/cli/factory.rs b/cli/factory.rs index 6937b750f9..f08bf7e4b1 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -984,6 +984,7 @@ impl CliFactory { cli_options.sub_command().clone(), self.create_cli_main_worker_options()?, self.cli_options()?.otel_config(), + self.cli_options()?.default_npm_caching_strategy(), )) } diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 22117990d2..b655dda0f6 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -4,6 +4,7 @@ use crate::args::config_to_deno_graph_workspace_member; use crate::args::jsr_url; use crate::args::CliLockfile; use crate::args::CliOptions; +pub use crate::args::NpmCachingStrategy; use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS; use crate::cache; use crate::cache::FetchCacher; @@ -218,6 +219,7 @@ pub struct CreateGraphOptions<'a> { pub is_dynamic: bool, /// Specify `None` to use the default CLI loader. pub loader: Option<&'a mut dyn Loader>, + pub npm_caching: NpmCachingStrategy, } pub struct ModuleGraphCreator { @@ -246,10 +248,11 @@ impl ModuleGraphCreator { &self, graph_kind: GraphKind, roots: Vec, + npm_caching: NpmCachingStrategy, ) -> Result { let mut cache = self.module_graph_builder.create_graph_loader(); self - .create_graph_with_loader(graph_kind, roots, &mut cache) + .create_graph_with_loader(graph_kind, roots, &mut cache, npm_caching) .await } @@ -258,6 +261,7 @@ impl ModuleGraphCreator { graph_kind: GraphKind, roots: Vec, loader: &mut dyn Loader, + npm_caching: NpmCachingStrategy, ) -> Result { self .create_graph_with_options(CreateGraphOptions { @@ -265,6 +269,7 @@ impl ModuleGraphCreator { graph_kind, roots, loader: Some(loader), + npm_caching, }) .await } @@ -317,6 +322,7 @@ impl ModuleGraphCreator { graph_kind: deno_graph::GraphKind::All, roots, loader: Some(&mut publish_loader), + npm_caching: self.options.default_npm_caching_strategy(), }) .await?; self.graph_valid(&graph)?; @@ -376,6 +382,7 @@ impl ModuleGraphCreator { graph_kind, roots, loader: None, + npm_caching: self.options.default_npm_caching_strategy(), }) .await?; @@ -565,7 +572,8 @@ impl ModuleGraphBuilder { }; let cli_resolver = &self.resolver; let graph_resolver = self.create_graph_resolver()?; - let graph_npm_resolver = cli_resolver.create_graph_npm_resolver(); + let graph_npm_resolver = + cli_resolver.create_graph_npm_resolver(options.npm_caching); let maybe_file_watcher_reporter = self .maybe_file_watcher_reporter .as_ref() @@ -592,6 +600,7 @@ impl ModuleGraphBuilder { resolver: Some(&graph_resolver), locker: locker.as_mut().map(|l| l as _), }, + options.npm_caching, ) .await } @@ -602,6 +611,7 @@ impl ModuleGraphBuilder { roots: Vec, loader: &'a mut dyn deno_graph::source::Loader, options: deno_graph::BuildOptions<'a>, + npm_caching: NpmCachingStrategy, ) -> Result<(), AnyError> { // ensure an "npm install" is done if the user has explicitly // opted into using a node_modules directory @@ -612,7 +622,13 @@ impl ModuleGraphBuilder { .unwrap_or(false) { if let Some(npm_resolver) = self.npm_resolver.as_managed() { - npm_resolver.ensure_top_level_package_json_install().await?; + let already_done = + npm_resolver.ensure_top_level_package_json_install().await?; + if !already_done && matches!(npm_caching, NpmCachingStrategy::Eager) { + npm_resolver + .cache_packages(crate::npm::PackageCaching::All) + .await?; + } } } @@ -701,7 +717,9 @@ impl ModuleGraphBuilder { let parser = self.parsed_source_cache.as_capturing_parser(); let cli_resolver = &self.resolver; let graph_resolver = self.create_graph_resolver()?; - let graph_npm_resolver = cli_resolver.create_graph_npm_resolver(); + let graph_npm_resolver = cli_resolver.create_graph_npm_resolver( + self.cli_options.default_npm_caching_strategy(), + ); graph.build_fast_check_type_graph( deno_graph::BuildFastCheckTypeGraphOptions { diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 0caaa94107..3c4cb0930e 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -270,7 +270,12 @@ impl LanguageServer { open_docs: &open_docs, }; let graph = module_graph_creator - .create_graph_with_loader(GraphKind::All, roots.clone(), &mut loader) + .create_graph_with_loader( + GraphKind::All, + roots.clone(), + &mut loader, + graph_util::NpmCachingStrategy::Eager, + ) .await?; graph_util::graph_valid( &graph, diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 363ad43700..28c7b04fc9 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -133,7 +133,8 @@ impl LspScopeResolver { cache.for_specifier(config_data.map(|d| d.scope.as_ref())), config_data.and_then(|d| d.lockfile.clone()), ))); - let npm_graph_resolver = cli_resolver.create_graph_npm_resolver(); + let npm_graph_resolver = cli_resolver + .create_graph_npm_resolver(crate::graph_util::NpmCachingStrategy::Eager); let maybe_jsx_import_source_config = config_data.and_then(|d| d.maybe_jsx_import_source_config()); let graph_imports = config_data @@ -343,7 +344,9 @@ impl LspResolver { file_referrer: Option<&ModuleSpecifier>, ) -> WorkerCliNpmGraphResolver { let resolver = self.get_scope_resolver(file_referrer); - resolver.resolver.create_graph_npm_resolver() + resolver + .resolver + .create_graph_npm_resolver(crate::graph_util::NpmCachingStrategy::Eager) } pub fn as_is_cjs_resolver( diff --git a/cli/module_loader.rs b/cli/module_loader.rs index c5f80d68e0..5e4ff875dc 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -156,6 +156,7 @@ impl ModuleLoadPreparer { graph_kind: graph.graph_kind(), roots: roots.to_vec(), loader: Some(&mut cache), + npm_caching: self.options.default_npm_caching_strategy(), }, ) .await?; diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs index 5ed25f8272..2c6e6d318a 100644 --- a/cli/npm/managed/mod.rs +++ b/cli/npm/managed/mod.rs @@ -296,6 +296,12 @@ pub fn create_managed_in_npm_pkg_checker( Arc::new(ManagedInNpmPackageChecker { root_dir }) } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PackageCaching<'a> { + Only(Cow<'a, [PackageReq]>), + All, +} + /// An npm resolver where the resolution is managed by Deno rather than /// the user bringing their own node_modules (BYONM) on the file system. pub struct ManagedCliNpmResolver { @@ -420,19 +426,44 @@ impl ManagedCliNpmResolver { /// Adds package requirements to the resolver and ensures everything is setup. /// This includes setting up the `node_modules` directory, if applicable. - pub async fn add_package_reqs( + pub async fn add_and_cache_package_reqs( &self, packages: &[PackageReq], ) -> Result<(), AnyError> { self - .add_package_reqs_raw(packages) + .add_package_reqs_raw( + packages, + Some(PackageCaching::Only(packages.into())), + ) .await .dependencies_result } - pub async fn add_package_reqs_raw( + pub async fn add_package_reqs_no_cache( &self, packages: &[PackageReq], + ) -> Result<(), AnyError> { + self + .add_package_reqs_raw(packages, None) + .await + .dependencies_result + } + + pub async fn add_package_reqs( + &self, + packages: &[PackageReq], + caching: PackageCaching<'_>, + ) -> Result<(), AnyError> { + self + .add_package_reqs_raw(packages, Some(caching)) + .await + .dependencies_result + } + + pub async fn add_package_reqs_raw<'a>( + &self, + packages: &[PackageReq], + caching: Option>, ) -> AddPkgReqsResult { if packages.is_empty() { return AddPkgReqsResult { @@ -449,7 +480,9 @@ impl ManagedCliNpmResolver { } } if result.dependencies_result.is_ok() { - result.dependencies_result = self.cache_packages().await; + if let Some(caching) = caching { + result.dependencies_result = self.cache_packages(caching).await; + } } result @@ -491,16 +524,20 @@ impl ManagedCliNpmResolver { pub async fn inject_synthetic_types_node_package( &self, ) -> Result<(), AnyError> { + let reqs = &[PackageReq::from_str("@types/node").unwrap()]; // add and ensure this isn't added to the lockfile self - .add_package_reqs(&[PackageReq::from_str("@types/node").unwrap()]) + .add_package_reqs(reqs, PackageCaching::Only(reqs.into())) .await?; Ok(()) } - pub async fn cache_packages(&self) -> Result<(), AnyError> { - self.fs_resolver.cache_packages().await + pub async fn cache_packages( + &self, + caching: PackageCaching<'_>, + ) -> Result<(), AnyError> { + self.fs_resolver.cache_packages(caching).await } pub fn resolve_pkg_folder_from_deno_module( @@ -545,18 +582,18 @@ impl ManagedCliNpmResolver { /// Ensures that the top level `package.json` dependencies are installed. /// This may set up the `node_modules` directory. /// - /// Returns `true` if any changes (such as caching packages) were made. - /// If this returns `false`, `node_modules` has _not_ been set up. + /// Returns `true` if the top level packages are already installed. A + /// return value of `false` means that new packages were added to the NPM resolution. pub async fn ensure_top_level_package_json_install( &self, ) -> Result { if !self.top_level_install_flag.raise() { - return Ok(false); // already did this + return Ok(true); // already did this } let pkg_json_remote_pkgs = self.npm_install_deps_provider.remote_pkgs(); if pkg_json_remote_pkgs.is_empty() { - return Ok(false); + return Ok(true); } // check if something needs resolving before bothering to load all @@ -570,14 +607,16 @@ impl ManagedCliNpmResolver { log::debug!( "All package.json deps resolvable. Skipping top level install." ); - return Ok(false); // everything is already resolvable + return Ok(true); // everything is already resolvable } let pkg_reqs = pkg_json_remote_pkgs .iter() .map(|pkg| pkg.req.clone()) .collect::>(); - self.add_package_reqs(&pkg_reqs).await.map(|_| true) + self.add_package_reqs_no_cache(&pkg_reqs).await?; + + Ok(false) } pub async fn cache_package_info( diff --git a/cli/npm/managed/resolution.rs b/cli/npm/managed/resolution.rs index 66cc6a7428..73c5c31caf 100644 --- a/cli/npm/managed/resolution.rs +++ b/cli/npm/managed/resolution.rs @@ -255,6 +255,10 @@ impl NpmResolution { .read() .as_valid_serialized_for_system(system_info) } + + pub fn subset(&self, package_reqs: &[PackageReq]) -> NpmResolutionSnapshot { + self.snapshot.read().subset(package_reqs) + } } async fn add_package_reqs_to_snapshot( diff --git a/cli/npm/managed/resolvers/common.rs b/cli/npm/managed/resolvers/common.rs index 332756daa4..68e95fb39a 100644 --- a/cli/npm/managed/resolvers/common.rs +++ b/cli/npm/managed/resolvers/common.rs @@ -11,6 +11,7 @@ use std::path::PathBuf; use std::sync::Arc; use std::sync::Mutex; +use super::super::PackageCaching; use async_trait::async_trait; use deno_ast::ModuleSpecifier; use deno_core::anyhow::Context; @@ -57,7 +58,10 @@ pub trait NpmPackageFsResolver: Send + Sync { specifier: &ModuleSpecifier, ) -> Result, AnyError>; - async fn cache_packages(&self) -> Result<(), AnyError>; + async fn cache_packages<'a>( + &self, + caching: PackageCaching<'a>, + ) -> Result<(), AnyError>; #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"] fn ensure_read_permission<'a>( diff --git a/cli/npm/managed/resolvers/global.rs b/cli/npm/managed/resolvers/global.rs index 2b48c3d2fc..4e79941af6 100644 --- a/cli/npm/managed/resolvers/global.rs +++ b/cli/npm/managed/resolvers/global.rs @@ -8,6 +8,7 @@ use std::path::PathBuf; use std::sync::Arc; use crate::colors; +use crate::npm::managed::PackageCaching; use crate::npm::CliNpmCache; use crate::npm::CliNpmTarballCache; use async_trait::async_trait; @@ -150,10 +151,19 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver { ) } - async fn cache_packages(&self) -> Result<(), AnyError> { - let package_partitions = self - .resolution - .all_system_packages_partitioned(&self.system_info); + async fn cache_packages<'a>( + &self, + caching: PackageCaching<'a>, + ) -> Result<(), AnyError> { + let package_partitions = match caching { + PackageCaching::All => self + .resolution + .all_system_packages_partitioned(&self.system_info), + PackageCaching::Only(reqs) => self + .resolution + .subset(&reqs) + .all_system_packages_partitioned(&self.system_info), + }; cache_packages(&package_partitions.packages, &self.tarball_cache).await?; // create the copy package folders diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs index d383a5413f..1e83717f15 100644 --- a/cli/npm/managed/resolvers/local.rs +++ b/cli/npm/managed/resolvers/local.rs @@ -17,6 +17,7 @@ use std::sync::Arc; use crate::args::LifecycleScriptsConfig; use crate::colors; +use crate::npm::managed::PackageCaching; use crate::npm::CliNpmCache; use crate::npm::CliNpmTarballCache; use async_trait::async_trait; @@ -253,9 +254,16 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver { )) } - async fn cache_packages(&self) -> Result<(), AnyError> { + async fn cache_packages<'a>( + &self, + caching: PackageCaching<'a>, + ) -> Result<(), AnyError> { + let snapshot = match caching { + PackageCaching::All => self.resolution.snapshot(), + PackageCaching::Only(reqs) => self.resolution.subset(&reqs), + }; sync_resolution_with_fs( - &self.resolution.snapshot(), + &snapshot, &self.cache, &self.npm_install_deps_provider, &self.progress_bar, diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index 48d90d7dd0..b39e0a340d 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -41,6 +41,7 @@ pub use self::managed::CliManagedInNpmPkgCheckerCreateOptions; pub use self::managed::CliManagedNpmResolverCreateOptions; pub use self::managed::CliNpmResolverManagedSnapshotOption; pub use self::managed::ManagedCliNpmResolver; +pub use self::managed::PackageCaching; pub type CliNpmTarballCache = deno_npm_cache::TarballCache; pub type CliNpmCache = deno_npm_cache::NpmCache; diff --git a/cli/resolver.rs b/cli/resolver.rs index 15ca4aa2b6..f5c3f68f36 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -32,6 +32,7 @@ use std::path::PathBuf; use std::sync::Arc; use thiserror::Error; +use crate::args::NpmCachingStrategy; use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS; use crate::node::CliNodeCodeTranslator; use crate::npm::CliNpmResolver; @@ -240,11 +241,15 @@ impl CliResolver { // todo(dsherret): move this off CliResolver as CliResolver is acting // like a factory by doing this (it's beyond its responsibility) - pub fn create_graph_npm_resolver(&self) -> WorkerCliNpmGraphResolver { + pub fn create_graph_npm_resolver( + &self, + npm_caching: NpmCachingStrategy, + ) -> WorkerCliNpmGraphResolver { WorkerCliNpmGraphResolver { npm_resolver: self.npm_resolver.as_ref(), found_package_json_dep_flag: &self.found_package_json_dep_flag, bare_node_builtins_enabled: self.bare_node_builtins_enabled, + npm_caching, } } @@ -304,6 +309,7 @@ pub struct WorkerCliNpmGraphResolver<'a> { npm_resolver: Option<&'a Arc>, found_package_json_dep_flag: &'a AtomicFlag, bare_node_builtins_enabled: bool, + npm_caching: NpmCachingStrategy, } #[async_trait(?Send)] @@ -373,7 +379,20 @@ impl<'a> deno_graph::source::NpmResolver for WorkerCliNpmGraphResolver<'a> { Ok(()) }; - let result = npm_resolver.add_package_reqs_raw(package_reqs).await; + let result = npm_resolver + .add_package_reqs_raw( + package_reqs, + match self.npm_caching { + NpmCachingStrategy::Eager => { + Some(crate::npm::PackageCaching::All) + } + NpmCachingStrategy::Lazy => { + Some(crate::npm::PackageCaching::Only(package_reqs.into())) + } + NpmCachingStrategy::Manual => None, + }, + ) + .await; NpmResolvePkgReqsResult { results: result diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs index 632f27da6f..dec1d89110 100644 --- a/cli/standalone/binary.rs +++ b/cli/standalone/binary.rs @@ -779,6 +779,7 @@ impl<'a> DenoCompileBinaryWriter<'a> { detect_cjs: self.cli_options.unstable_detect_cjs(), sloppy_imports: self.cli_options.unstable_sloppy_imports(), features: self.cli_options.unstable_features(), + npm_lazy_caching: self.cli_options.unstable_npm_lazy_caching(), }, otel_config: self.cli_options.otel_config(), }; diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index 53efab2964..22e0b6d115 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -924,6 +924,7 @@ pub async fn run(data: StandaloneData) -> Result { serve_host: None, }, metadata.otel_config, + crate::args::NpmCachingStrategy::Lazy, ); // Initialize v8 once from the main thread. diff --git a/cli/tools/bench/mod.rs b/cli/tools/bench/mod.rs index 1d49fa061d..5983590531 100644 --- a/cli/tools/bench/mod.rs +++ b/cli/tools/bench/mod.rs @@ -538,7 +538,11 @@ pub async fn run_benchmarks_with_watch( )?; let graph = module_graph_creator - .create_graph(graph_kind, collected_bench_modules.clone()) + .create_graph( + graph_kind, + collected_bench_modules.clone(), + crate::graph_util::NpmCachingStrategy::Eager, + ) .await?; module_graph_creator.graph_valid(&graph)?; let bench_modules = &graph.roots; diff --git a/cli/tools/compile.rs b/cli/tools/compile.rs index 4fa9963683..5cd19ecf71 100644 --- a/cli/tools/compile.rs +++ b/cli/tools/compile.rs @@ -69,7 +69,11 @@ pub async fn compile( // create a module graph with types information in it. We don't want to // store that in the binary so create a code only module graph from scratch. module_graph_creator - .create_graph(GraphKind::CodeOnly, module_roots) + .create_graph( + GraphKind::CodeOnly, + module_roots, + crate::graph_util::NpmCachingStrategy::Eager, + ) .await? } else { graph diff --git a/cli/tools/doc.rs b/cli/tools/doc.rs index 9a24e458ac..647a36dc48 100644 --- a/cli/tools/doc.rs +++ b/cli/tools/doc.rs @@ -131,7 +131,11 @@ pub async fn doc( |_| true, )?; let graph = module_graph_creator - .create_graph(GraphKind::TypesOnly, module_specifiers.clone()) + .create_graph( + GraphKind::TypesOnly, + module_specifiers.clone(), + crate::graph_util::NpmCachingStrategy::Eager, + ) .await?; graph_exit_integrity_errors(&graph); diff --git a/cli/tools/info.rs b/cli/tools/info.rs index f0cd37772d..f58732a904 100644 --- a/cli/tools/info.rs +++ b/cli/tools/info.rs @@ -123,7 +123,12 @@ pub async fn info( let mut loader = module_graph_builder.create_graph_loader(); loader.enable_loading_cache_info(); // for displaying the cache information let graph = module_graph_creator - .create_graph_with_loader(GraphKind::All, vec![specifier], &mut loader) + .create_graph_with_loader( + GraphKind::All, + vec![specifier], + &mut loader, + crate::graph_util::NpmCachingStrategy::Eager, + ) .await?; // write out the lockfile if there is one diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index df5981e6e7..d7c484beba 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -9,7 +9,6 @@ use crate::args::Flags; use crate::args::InstallFlags; use crate::args::InstallFlagsGlobal; use crate::args::InstallFlagsLocal; -use crate::args::InstallKind; use crate::args::TypeCheckMode; use crate::args::UninstallFlags; use crate::args::UninstallKind; @@ -339,11 +338,11 @@ pub async fn install_command( flags: Arc, install_flags: InstallFlags, ) -> Result<(), AnyError> { - match install_flags.kind { - InstallKind::Global(global_flags) => { + match install_flags { + InstallFlags::Global(global_flags) => { install_global(flags, global_flags).await } - InstallKind::Local(local_flags) => { + InstallFlags::Local(local_flags) => { if let InstallFlagsLocal::Add(add_flags) = &local_flags { check_if_installs_a_single_package_globally(Some(add_flags))?; } diff --git a/cli/tools/registry/pm/cache_deps.rs b/cli/tools/registry/pm/cache_deps.rs index dbec8a3b3f..814c76cb27 100644 --- a/cli/tools/registry/pm/cache_deps.rs +++ b/cli/tools/registry/pm/cache_deps.rs @@ -6,6 +6,7 @@ use std::sync::Arc; use crate::factory::CliFactory; use crate::graph_container::ModuleGraphContainer; use crate::graph_container::ModuleGraphUpdatePermit; +use crate::graph_util::CreateGraphOptions; use deno_core::error::AnyError; use deno_core::futures::stream::FuturesUnordered; use deno_core::futures::StreamExt; @@ -18,18 +19,16 @@ pub async fn cache_top_level_deps( ) -> Result<(), AnyError> { let npm_resolver = factory.npm_resolver().await?; let cli_options = factory.cli_options()?; - let root_permissions = factory.root_permissions_container()?; if let Some(npm_resolver) = npm_resolver.as_managed() { - if !npm_resolver.ensure_top_level_package_json_install().await? { - if let Some(lockfile) = cli_options.maybe_lockfile() { - lockfile.error_if_changed()?; - } - - npm_resolver.cache_packages().await?; + npm_resolver.ensure_top_level_package_json_install().await?; + if let Some(lockfile) = cli_options.maybe_lockfile() { + lockfile.error_if_changed()?; } } // cache as many entries in the import map as we can let resolver = factory.workspace_resolver().await?; + + let mut maybe_graph_error = Ok(()); if let Some(import_map) = resolver.maybe_import_map() { let jsr_resolver = if let Some(resolver) = jsr_resolver { resolver @@ -122,19 +121,29 @@ pub async fn cache_top_level_deps( } drop(info_futures); - factory - .module_load_preparer() - .await? - .prepare_module_load( + let graph_builder = factory.module_graph_builder().await?; + graph_builder + .build_graph_with_npm_resolution( graph, - &roots, - false, - deno_config::deno_json::TsTypeLib::DenoWorker, - root_permissions.clone(), - None, + CreateGraphOptions { + loader: None, + graph_kind: graph.graph_kind(), + is_dynamic: false, + roots: roots.clone(), + npm_caching: crate::graph_util::NpmCachingStrategy::Manual, + }, ) .await?; + maybe_graph_error = graph_builder.graph_roots_valid(graph, &roots); + } + + if let Some(npm_resolver) = npm_resolver.as_managed() { + npm_resolver + .cache_packages(crate::npm::PackageCaching::All) + .await?; } + maybe_graph_error?; + Ok(()) } diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index 26e1eeac2f..02594f1519 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -727,7 +727,9 @@ impl ReplSession { let has_node_specifier = resolved_imports.iter().any(|url| url.scheme() == "node"); if !npm_imports.is_empty() || has_node_specifier { - npm_resolver.add_package_reqs(&npm_imports).await?; + npm_resolver + .add_and_cache_package_reqs(&npm_imports) + .await?; // prevent messages in the repl about @types/node not being cached if has_node_specifier { diff --git a/cli/tools/run/mod.rs b/cli/tools/run/mod.rs index 8fab544eca..d3f7b093d4 100644 --- a/cli/tools/run/mod.rs +++ b/cli/tools/run/mod.rs @@ -198,13 +198,23 @@ pub async fn eval_command( } pub async fn maybe_npm_install(factory: &CliFactory) -> Result<(), AnyError> { + let cli_options = factory.cli_options()?; // ensure an "npm install" is done if the user has explicitly // opted into using a managed node_modules directory - if factory.cli_options()?.node_modules_dir()? - == Some(NodeModulesDirMode::Auto) - { + if cli_options.node_modules_dir()? == Some(NodeModulesDirMode::Auto) { if let Some(npm_resolver) = factory.npm_resolver().await?.as_managed() { - npm_resolver.ensure_top_level_package_json_install().await?; + let already_done = + npm_resolver.ensure_top_level_package_json_install().await?; + if !already_done + && matches!( + cli_options.default_npm_caching_strategy(), + crate::graph_util::NpmCachingStrategy::Eager + ) + { + npm_resolver + .cache_packages(crate::npm::PackageCaching::All) + .await?; + } } } Ok(()) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index fc1410aa0e..25d1d66710 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -440,6 +440,13 @@ impl<'a> TaskRunner<'a> { kill_signal: KillSignal, argv: &'a [String], ) -> Result { + if let Some(npm_resolver) = self.npm_resolver.as_managed() { + npm_resolver.ensure_top_level_package_json_install().await?; + npm_resolver + .cache_packages(crate::npm::PackageCaching::All) + .await?; + } + let cwd = match &self.task_flags.cwd { Some(path) => canonicalize_path(&PathBuf::from(path)) .context("failed canonicalizing --cwd")?, @@ -450,6 +457,7 @@ impl<'a> TaskRunner<'a> { self.npm_resolver, self.node_resolver, )?; + self .run_single(RunSingleOptions { task_name, @@ -473,6 +481,9 @@ impl<'a> TaskRunner<'a> { // ensure the npm packages are installed if using a managed resolver if let Some(npm_resolver) = self.npm_resolver.as_managed() { npm_resolver.ensure_top_level_package_json_install().await?; + npm_resolver + .cache_packages(crate::npm::PackageCaching::All) + .await?; } let cwd = match &self.task_flags.cwd { @@ -492,6 +503,7 @@ impl<'a> TaskRunner<'a> { self.npm_resolver, self.node_resolver, )?; + for task_name in &task_names { if let Some(script) = scripts.get(task_name) { let exit_code = self diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 6357ebcae2..2e46bdd4da 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -1716,7 +1716,11 @@ pub async fn run_tests_with_watch( &cli_options.permissions_options(), )?; let graph = module_graph_creator - .create_graph(graph_kind, test_modules) + .create_graph( + graph_kind, + test_modules, + crate::graph_util::NpmCachingStrategy::Eager, + ) .await?; module_graph_creator.graph_valid(&graph)?; let test_modules = &graph.roots; diff --git a/cli/worker.rs b/cli/worker.rs index 161d8bcc21..81b8cd2f83 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -50,6 +50,7 @@ use tokio::select; use crate::args::CliLockfile; use crate::args::DenoSubcommand; +use crate::args::NpmCachingStrategy; use crate::args::StorageKeyResolver; use crate::errors; use crate::npm::CliNpmResolver; @@ -154,6 +155,7 @@ struct SharedWorkerState { options: CliMainWorkerOptions, subcommand: DenoSubcommand, otel_config: Option, // `None` means OpenTelemetry is disabled. + default_npm_caching_strategy: NpmCachingStrategy, } impl SharedWorkerState { @@ -425,6 +427,7 @@ impl CliMainWorkerFactory { subcommand: DenoSubcommand, options: CliMainWorkerOptions, otel_config: Option, + default_npm_caching_strategy: NpmCachingStrategy, ) -> Self { Self { shared: Arc::new(SharedWorkerState { @@ -448,6 +451,7 @@ impl CliMainWorkerFactory { options, subcommand, otel_config, + default_npm_caching_strategy, }), } } @@ -487,8 +491,19 @@ impl CliMainWorkerFactory { NpmPackageReqReference::from_specifier(&main_module) { if let Some(npm_resolver) = shared.npm_resolver.as_managed() { + let reqs = &[package_ref.req().clone()]; npm_resolver - .add_package_reqs(&[package_ref.req().clone()]) + .add_package_reqs( + reqs, + if matches!( + shared.default_npm_caching_strategy, + NpmCachingStrategy::Lazy + ) { + crate::npm::PackageCaching::Only(reqs.into()) + } else { + crate::npm::PackageCaching::All + }, + ) .await?; } diff --git a/tests/specs/install/entrypoint_only_used_packages/__test__.jsonc b/tests/specs/install/entrypoint_only_used_packages/__test__.jsonc new file mode 100644 index 0000000000..9d24eb4de4 --- /dev/null +++ b/tests/specs/install/entrypoint_only_used_packages/__test__.jsonc @@ -0,0 +1,13 @@ +{ + "tempDir": true, + "steps": [ + { + "args": "install --unstable-npm-lazy-caching --entrypoint main.ts", + "output": "install-entrypoint.out" + }, + { + "args": "install", + "output": "install.out" + } + ] +} diff --git a/tests/specs/install/entrypoint_only_used_packages/install-entrypoint.out b/tests/specs/install/entrypoint_only_used_packages/install-entrypoint.out new file mode 100644 index 0000000000..5f6c8247a5 --- /dev/null +++ b/tests/specs/install/entrypoint_only_used_packages/install-entrypoint.out @@ -0,0 +1,6 @@ +[UNORDERED_START] +Download http://localhost:4260/@denotest%2fadd +Download http://localhost:4260/@denotest%2fsay-hello +Download http://localhost:4260/@denotest/add/1.0.0.tgz +[UNORDERED_END] +Initialize @denotest/add@1.0.0 diff --git a/tests/specs/install/entrypoint_only_used_packages/install.out b/tests/specs/install/entrypoint_only_used_packages/install.out new file mode 100644 index 0000000000..5ea41bed4c --- /dev/null +++ b/tests/specs/install/entrypoint_only_used_packages/install.out @@ -0,0 +1,2 @@ +Download http://localhost:4260/@denotest/say-hello/1.0.0.tgz +Initialize @denotest/say-hello@1.0.0 diff --git a/tests/specs/install/entrypoint_only_used_packages/main.ts b/tests/specs/install/entrypoint_only_used_packages/main.ts new file mode 100644 index 0000000000..1ca631410f --- /dev/null +++ b/tests/specs/install/entrypoint_only_used_packages/main.ts @@ -0,0 +1,3 @@ +import { add } from "@denotest/add"; + +console.log(add(1, 2)); diff --git a/tests/specs/install/entrypoint_only_used_packages/package.json b/tests/specs/install/entrypoint_only_used_packages/package.json new file mode 100644 index 0000000000..2623bcc82e --- /dev/null +++ b/tests/specs/install/entrypoint_only_used_packages/package.json @@ -0,0 +1,6 @@ +{ + "dependencies": { + "@denotest/add": "1.0.0", + "@denotest/say-hello": "1.0.0" + } +} diff --git a/tests/specs/run/jsx_import_source/__test__.jsonc b/tests/specs/run/jsx_import_source/__test__.jsonc index 55a895fc8f..cbda2dd32e 100644 --- a/tests/specs/run/jsx_import_source/__test__.jsonc +++ b/tests/specs/run/jsx_import_source/__test__.jsonc @@ -1,4 +1,5 @@ { + "tempDir": true, "tests": { "jsx_import_source_error": { "args": "run --config jsx/deno-jsx-error.jsonc --check jsx_import_source_no_pragma.tsx", diff --git a/tests/specs/run/lazy_npm/__test__.jsonc b/tests/specs/run/lazy_npm/__test__.jsonc new file mode 100644 index 0000000000..8212addd5c --- /dev/null +++ b/tests/specs/run/lazy_npm/__test__.jsonc @@ -0,0 +1,9 @@ +{ + "tempDir": true, + "steps": [ + { + "args": "run --unstable-npm-lazy-caching -A main.ts", + "output": "main.out" + } + ] +} diff --git a/tests/specs/run/lazy_npm/deno.json b/tests/specs/run/lazy_npm/deno.json new file mode 100644 index 0000000000..fbd70ec480 --- /dev/null +++ b/tests/specs/run/lazy_npm/deno.json @@ -0,0 +1,3 @@ +{ + "nodeModulesDir": "auto" +} diff --git a/tests/specs/run/lazy_npm/main.out b/tests/specs/run/lazy_npm/main.out new file mode 100644 index 0000000000..d1aab68e90 --- /dev/null +++ b/tests/specs/run/lazy_npm/main.out @@ -0,0 +1,7 @@ +[UNORDERED_START] +Download http://localhost:4260/@denotest%2fadd +Download http://localhost:4260/@denotest%2fsay-hello +Download http://localhost:4260/@denotest/add/1.0.0.tgz +[UNORDERED_END] +Initialize @denotest/add@1.0.0 +3 diff --git a/tests/specs/run/lazy_npm/main.ts b/tests/specs/run/lazy_npm/main.ts new file mode 100644 index 0000000000..1ca631410f --- /dev/null +++ b/tests/specs/run/lazy_npm/main.ts @@ -0,0 +1,3 @@ +import { add } from "@denotest/add"; + +console.log(add(1, 2)); diff --git a/tests/specs/run/lazy_npm/package.json b/tests/specs/run/lazy_npm/package.json new file mode 100644 index 0000000000..2623bcc82e --- /dev/null +++ b/tests/specs/run/lazy_npm/package.json @@ -0,0 +1,6 @@ +{ + "dependencies": { + "@denotest/add": "1.0.0", + "@denotest/say-hello": "1.0.0" + } +}