fix(cli): Support deno.lock with only package.json present + fix DENO…

…_FUTURE install interactions with lockfile (denoland#23918)

Fixes denoland#23571.

Previously, we required a `deno.json` to be present (or the `--lock`
flag) in order for us to resolve a `deno.lock` file. This meant that if
you were using deno in an npm-first project deno wouldn't use a
lockfile.

Additionally, while I was fixing that, I discovered there were a couple
bugs keeping the future `install` command from using a lockfile.

With this PR, `install` will actually resolve the lockfile (or create
one if not present), and update it if it's not up-to-date. This also
speeds up `deno install`, as we can use the lockfile to skip work during
npm resolution.

cli/args/lockfile.rs

@@ -3,23 +3,30 @@
 use std::path::PathBuf;
 
 use deno_core::error::AnyError;
+use deno_runtime::deno_node::PackageJson;
 
 use crate::args::ConfigFile;
 use crate::Flags;
 
 use super::DenoSubcommand;
+use super::InstallFlags;
+use super::InstallKind;
 
 pub use deno_lockfile::Lockfile;
 pub use deno_lockfile::LockfileError;
 
 pub fn discover(
  flags: &Flags,
  maybe_config_file: Option<&ConfigFile>,
+ maybe_package_json: Option<&PackageJson>,
 ) -> Result<Option<Lockfile>, AnyError> {
  if flags.no_lock
  || matches!(
  flags.subcommand,
- DenoSubcommand::Install(_) | DenoSubcommand::Uninstall(_)
+ DenoSubcommand::Install(InstallFlags {
+ kind: InstallKind::Global(..),
+ ..
+ }) | DenoSubcommand::Uninstall(_)
  )
  {
  return Ok(None);
@@ -38,7 +45,12 @@ pub fn discover(
  return Ok(None);
  }
  }
- None => return Ok(None),
+ None => match maybe_package_json {
+ Some(package_json) => {
+ package_json.path.parent().unwrap().join("deno.lock")
+ }
+ None => return Ok(None),
+ },
  },
  };
 

cli/args/mod.rs

@@ -852,8 +852,11 @@ impl CliOptions {
  maybe_package_json = discover_package_json(&flags, None, &initial_cwd)?;
  }
 
- let maybe_lock_file =
- lockfile::discover(&flags, maybe_config_file.as_ref())?;
+ let maybe_lock_file = lockfile::discover(
+ &flags,
+ maybe_config_file.as_ref(),
+ maybe_package_json.as_ref(),
+ )?;
  Self::new(
  flags,
  initial_cwd,

cli/tools/installer.rs

@@ -265,6 +265,10 @@ async fn install_local(
  let factory = CliFactory::from_flags(flags)?;
  crate::module_loader::load_top_level_deps(&factory).await?;
 
+ if let Some(lockfile) = factory.cli_options().maybe_lockfile() {
+ lockfile.lock().write()?;
+ }
+
  Ok(())
 }
 

tests/integration/npm_tests.rs

@@ -2982,15 +2982,16 @@ fn cjs_export_analysis_import_cjs_directly_relative_import() {
 }
 
 itest!(imports_package_json {
- args: "run --node-modules-dir=false npm/imports_package_json/main.js",
+ args:
+ "run --no-lock --node-modules-dir=false npm/imports_package_json/main.js",
  output: "npm/imports_package_json/main.out",
  envs: env_vars_for_npm_tests(),
  http_server: true,
 });
 
 itest!(imports_package_json_import_not_defined {
  args:
- "run --node-modules-dir=false npm/imports_package_json/import_not_defined.js",
+ "run --no-lock --node-modules-dir=false npm/imports_package_json/import_not_defined.js",
  output: "npm/imports_package_json/import_not_defined.out",
  envs: env_vars_for_npm_tests(),
  exit_code: 1,
@@ -2999,23 +3000,23 @@ itest!(imports_package_json_import_not_defined {
 
 itest!(imports_package_json_sub_path_import_not_defined {
  args:
- "run --node-modules-dir=false npm/imports_package_json/sub_path_import_not_defined.js",
+ "run --no-lock --node-modules-dir=false npm/imports_package_json/sub_path_import_not_defined.js",
  output: "npm/imports_package_json/sub_path_import_not_defined.out",
  envs: env_vars_for_npm_tests(),
  exit_code: 1,
  http_server: true,
 });
 
 itest!(different_nested_dep_node_modules_dir_false {
- args: "run --quiet --node-modules-dir=false npm/different_nested_dep/main.js",
+ args: "run --quiet --no-lock --node-modules-dir=false npm/different_nested_dep/main.js",
  output: "npm/different_nested_dep/main.out",
  envs: env_vars_for_npm_tests(),
  exit_code: 0,
  http_server: true,
 });
 
 itest!(different_nested_dep_node_modules_dir_true {
- args: "run --quiet --node-modules-dir=true main.js",
+ args: "run --no-lock --quiet --node-modules-dir=true main.js",
  output: "npm/different_nested_dep/main.out",
  copy_temp_dir: Some("npm/different_nested_dep/"),
  cwd: Some("npm/different_nested_dep/"),

tests/specs/install/future_install_local_deno/__test__.jsonc

@@ -12,6 +12,14 @@
  // ensure deps are actually cached
  "args": "run --cached-only main.js",
  "output": ""
+ },
+ {
+ // check for lockfile
+ "args": [
+ "eval",
+ "console.log(Deno.readTextFileSync('./deno.lock').trim())"
+ ],
+ "output": "deno.lock.out"
  }
  ]
 }

tests/specs/install/future_install_local_deno/deno.lock.out

@@ -0,0 +1,22 @@
+{
+ "version": "3",
+ "packages": {
+ "specifiers": {
+ "jsr:@denotest/add": "jsr:@denotest/[email protected]",
+ "npm:@denotest/esm-basic@^1.0.0": "npm:@denotest/[email protected]"
+ },
+ "jsr": {
+ "@denotest/[email protected]": [WILDCARD]
+ },
+ "npm": {
+ "@denotest/[email protected]": [WILDCARD]
+ }
+ },
+ "remote": [WILDCARD],
+ "workspace": {
+ "dependencies": [
+ "jsr:@denotest/add",
+ "npm:@denotest/esm-basic@^1.0.0"
+ ]
+ }
+}

tests/specs/lockfile/only_package_json/__test__.jsonc

@@ -0,0 +1,16 @@
+{
+ "tempDir": true,
+ "steps": [
+ {
+ "args": "cache index.js",
+ "output": "cache.out"
+ },
+ {
+ "args": [
+ "eval",
+ "console.log(Deno.readTextFileSync('./deno.lock').trim())"
+ ],
+ "output": "deno.lock.out"
+ }
+ ]
+}

tests/specs/lockfile/only_package_json/cache.out

@@ -0,0 +1,3 @@
+Download http:https://localhost:4260/@denotest/esm-basic
+Download http:https://localhost:4260/@denotest/esm-basic/1.0.0.tgz
+Initialize @denotest/[email protected]

tests/specs/lockfile/only_package_json/deno.lock.out

@@ -0,0 +1,19 @@
+{
+ "version": "3",
+ "packages": {
+ "specifiers": {
+ "npm:@denotest/esm-basic": "npm:@denotest/[email protected]"
+ },
+ "npm": {
+ "@denotest/[email protected]": [WILDCARD]
+ }
+ },
+ "remote": {},
+ "workspace": {
+ "packageJson": {
+ "dependencies": [
+ "npm:@denotest/esm-basic"
+ ]
+ }
+ }
+}

tests/specs/lockfile/only_package_json/index.js

@@ -0,0 +1 @@
+import * as basic from "@denotest/esm-basic";

tests/specs/lockfile/only_package_json/package.json

@@ -0,0 +1,7 @@
+{
+ "name": "lockfile_only_package_json",
+ "dependencies": {
+ "@denotest/esm-basic": "*"
+ },
+ "type": "module"
+}