From a7531361ef9bcc29b31ca73b68eeae707ea91f22 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> Date: Wed, 12 Jun 2024 16:06:41 -0700 Subject: [PATCH] fix(cli): Explicitly cache NPM packages during `deno install` (#24190) Fixes a regression introduced in https://github.com/denoland/deno/pull/24170, where we wouldn't actually set up the node modules dir on `deno install` if there was an up to date deno lockfile present. Previously we were relying on the fact that resolving pending module resolution called `cache_packages` (which sets up the node modules dir). When pending resolutions were removed, and the `resolve_pending` function with it, we also removed the `cache_packages` call needed to set up node modules. --- cli/module_loader.rs | 3 ++ .../__test__.jsonc | 38 +++++++++++++++++++ .../install_lockfile.out | 2 + .../npmrc_not_next_to_package_json/main.out | 1 + 4 files changed, 44 insertions(+) create mode 100644 tests/specs/install/future_install_node_modules/install_lockfile.out diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 1da3f1f2af1c36..5752c50aa25620 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -75,6 +75,9 @@ pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> { let npm_resolver = factory.npm_resolver().await?; if let Some(npm_resolver) = npm_resolver.as_managed() { npm_resolver.ensure_top_level_package_json_install().await?; + // TODO(nathanwhit): we call `cache_packages` if the lockfile is modified, + // so by calling it here it's possible we end up calling it twice + npm_resolver.cache_packages().await?; } // cache as many entries in the import map as we can if let Some(import_map) = factory.maybe_import_map().await? { diff --git a/tests/specs/install/future_install_node_modules/__test__.jsonc b/tests/specs/install/future_install_node_modules/__test__.jsonc index 5712165570428f..c705ad0f813ae6 100644 --- a/tests/specs/install/future_install_node_modules/__test__.jsonc +++ b/tests/specs/install/future_install_node_modules/__test__.jsonc @@ -25,6 +25,44 @@ } ] }, + "install_sets_up_node_modules_with_lockfile": { + "tempDir": true, + "envs": { + "DENO_FUTURE": "1" + }, + "steps": [ + { + "args": "install", + "output": "install.out" + }, + { + // check for lockfile + "args": [ + "eval", + "console.log(Deno.readTextFileSync('./deno.lock').trim())" + ], + "output": "deno.lock.out" + }, + { + // remove the lockfile + "args": [ + "eval", + "Deno.removeSync('node_modules', {recursive: true});" + ], + "output": "" + }, + { + // install, this time should use the lockfile + "args": "install", + "output": "install_lockfile.out" + }, + { + // make sure we created the node_modules dir + "args": ["eval", "Deno.statSync('node_modules')"], + "output": "" + } + ] + }, "install_errors_corrupted_lockfile": { "tempDir": true, "envs": { diff --git a/tests/specs/install/future_install_node_modules/install_lockfile.out b/tests/specs/install/future_install_node_modules/install_lockfile.out new file mode 100644 index 00000000000000..3e27e3d2079ae9 --- /dev/null +++ b/tests/specs/install/future_install_node_modules/install_lockfile.out @@ -0,0 +1,2 @@ +⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag. +Initialize @denotest/esm-basic@1.0.0 diff --git a/tests/specs/npm/npmrc_not_next_to_package_json/main.out b/tests/specs/npm/npmrc_not_next_to_package_json/main.out index 8f42fb6d817958..933d44f1c11519 100644 --- a/tests/specs/npm/npmrc_not_next_to_package_json/main.out +++ b/tests/specs/npm/npmrc_not_next_to_package_json/main.out @@ -4,3 +4,4 @@ [WILDCARD] Download http://localhost:4260/@denotest/esm-basic/1.0.0.tgz Initialize @denotest/esm-basic@1.0.0 +[WILDCARD]