Skip to content

Commit

Permalink
feat(unstable): support caching npm dependencies only as they're need…
Browse files Browse the repository at this point in the history 
…ed (#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 <hello@lcas.dev>
  • Loading branch information
@nathanwhit @lucacasonato
nathanwhit and lucacasonato authored Dec 11, 2024
1 parent dd42a64 commit 6f50620
Show file tree
Hide file tree
Showing 39 changed files with 370 additions and 108 deletions.
76 changes: 40 additions & 36 deletions cli/args/flags.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ pub struct InstallFlagsGlobal {
}

#[derive(Clone, Debug, Eq, PartialEq)]
pub enum InstallKind {
pub enum InstallFlags {
Local(InstallFlagsLocal),
Global(InstallFlagsGlobal),
}
Expand All @@ -257,11 +257,6 @@ pub enum InstallFlagsLocal {
Entrypoints(Vec<String>),
}

#[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,
Expand Down Expand Up @@ -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<String>, // --unstabe-kv --unstable-cron
}

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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(());
}
Expand All @@ -4937,22 +4942,19 @@ fn install_parse(
allow_scripts_arg_parse(flags, matches)?;
if matches.get_flag("entrypoint") {
let entrypoints = matches.remove_many::<String>("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(())
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}
);
Expand All @@ -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()
}
);
Expand All @@ -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()),
Expand Down Expand Up @@ -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!(),
Expand Down
7 changes: 2 additions & 5 deletions cli/args/lockfile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use crate::Flags;

use crate::args::DenoSubcommand;
use crate::args::InstallFlags;
use crate::args::InstallKind;

use deno_lockfile::Lockfile;

Expand Down Expand Up @@ -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);
Expand Down
35 changes: 27 additions & 8 deletions cli/args/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}
Expand Down Expand Up @@ -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),
..
Expand Down Expand Up @@ -1689,6 +1687,7 @@ impl CliOptions {
"detect-cjs",
"fmt-component",
"fmt-sql",
"lazy-npm-caching",
])
.collect();

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -1981,6 +1993,13 @@ fn load_env_variables_from_env_file(filename: Option<&Vec<String>>) {
}
}

#[derive(Debug, Clone, Copy)]
pub enum NpmCachingStrategy {
Eager,
Lazy,
Manual,
}

#[cfg(test)]
mod test {
use pretty_assertions::assert_eq;
Expand Down
1 change: 1 addition & 0 deletions cli/factory.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
))
}

Expand Down
26 changes: 22 additions & 4 deletions cli/graph_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -246,10 +248,11 @@ impl ModuleGraphCreator {
&self,
graph_kind: GraphKind,
roots: Vec<ModuleSpecifier>,
npm_caching: NpmCachingStrategy,
) -> Result<deno_graph::ModuleGraph, AnyError> {
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
}

Expand All @@ -258,13 +261,15 @@ impl ModuleGraphCreator {
graph_kind: GraphKind,
roots: Vec<ModuleSpecifier>,
loader: &mut dyn Loader,
npm_caching: NpmCachingStrategy,
) -> Result<ModuleGraph, AnyError> {
self
.create_graph_with_options(CreateGraphOptions {
is_dynamic: false,
graph_kind,
roots,
loader: Some(loader),
npm_caching,
})
.await
}
Expand Down Expand Up @@ -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)?;
Expand Down Expand Up @@ -376,6 +382,7 @@ impl ModuleGraphCreator {
graph_kind,
roots,
loader: None,
npm_caching: self.options.default_npm_caching_strategy(),
})
.await?;

Expand Down Expand Up @@ -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()
Expand All @@ -592,6 +600,7 @@ impl ModuleGraphBuilder {
resolver: Some(&graph_resolver),
locker: locker.as_mut().map(|l| l as _),
},
options.npm_caching,
)
.await
}
Expand All @@ -602,6 +611,7 @@ impl ModuleGraphBuilder {
roots: Vec<ModuleSpecifier>,
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
Expand All @@ -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?;
}
}
}

Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 6f50620

Please sign in to comment.