feat(compile): support "bring your own node_modules" in deno compile (#21377)

Not tested thoroughly. This is a good start.

Closes #21350

cli/factory.rs

@@ -43,6 +43,7 @@ use crate::resolver::CliGraphResolverOptions;
 use crate::standalone::DenoCompileBinaryWriter;
 use crate::tools::check::TypeChecker;
 use crate::util::file_watcher::WatcherCommunicator;
+use crate::util::fs::canonicalize_path_maybe_not_exists;
 use crate::util::progress_bar::ProgressBar;
 use crate::util::progress_bar::ProgressBarStyle;
 use crate::worker::CliMainWorkerFactory;
@@ -306,9 +307,13 @@ impl CliFactory {
         create_cli_npm_resolver(if self.options.unstable_byonm() {
           CliNpmResolverCreateOptions::Byonm(CliNpmResolverByonmCreateOptions {
             fs: fs.clone(),
-            // todo(byonm): actually resolve this properly because the package.json
+            root_node_modules_dir: match self.options.node_modules_dir_path().clone() {
-            // might be in an ancestor directory
+              Some(node_modules_path) => node_modules_path,
-            root_node_modules_dir: self.options.initial_cwd().join("node_modules"),
+              // path needs to be canonicalized for node resolution
+              // (node_modules_dir_path above is already canonicalized)
+              None => canonicalize_path_maybe_not_exists(self.options.initial_cwd())?
+                .join("node_modules"),
+            },
           })
         } else {
           CliNpmResolverCreateOptions::Managed(CliNpmResolverManagedCreateOptions {

cli/npm/byonm.rs

@@ -19,7 +19,7 @@ use deno_semver::package::PackageReq;
 use crate::args::package_json::get_local_package_json_version_reqs;
 use crate::args::NpmProcessState;
 use crate::args::NpmProcessStateKind;
-use crate::util::fs::canonicalize_path_maybe_not_exists;
+use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs;
 use crate::util::path::specifier_to_file_path;
 
 use super::common::types_package_name;
@@ -188,8 +188,8 @@ impl CliNpmResolver for ByonmCliNpmResolver {
     InnerCliNpmResolverRef::Byonm(self)
   }
 
-  fn root_node_modules_path(&self) -> Option<std::path::PathBuf> {
+  fn root_node_modules_path(&self) -> Option<&PathBuf> {
-    Some(self.root_node_modules_dir.clone())
+    Some(&self.root_node_modules_dir)
   }
 
   fn resolve_pkg_folder_from_deno_module_req(
@@ -215,7 +215,10 @@ impl CliNpmResolver for ByonmCliNpmResolver {
               .unwrap()
               .join("node_modules")
               .join(key);
-            return Ok(canonicalize_path_maybe_not_exists(&package_path)?);
+            return Ok(canonicalize_path_maybe_not_exists_with_fs(
+              &package_path,
+              fs,
+            )?);
           }
         }
       }

cli/npm/managed/mod.rs

@@ -288,12 +288,8 @@ impl ManagedCliNpmResolver {
     pkg_id: &NpmPackageId,
   ) -> Result<PathBuf, AnyError> {
     let path = self.fs_resolver.package_folder(pkg_id)?;
-    let path = canonicalize_path_maybe_not_exists_with_fs(&path, |path| {
+    let path =
-      self
+      canonicalize_path_maybe_not_exists_with_fs(&path, self.fs.as_ref())?;
-        .fs
-        .realpath_sync(path)
-        .map_err(|err| err.into_io_error())
-    })?;
     log::debug!(
       "Resolved package folder of {} to {}",
       pkg_id.as_serialized(),
@@ -560,7 +556,7 @@ impl CliNpmResolver for ManagedCliNpmResolver {
         &self.progress_bar,
         self.api.base_url().clone(),
         npm_resolution,
-        self.root_node_modules_path(),
+        self.root_node_modules_path().map(ToOwned::to_owned),
         self.npm_system_info.clone(),
       ),
       self.global_npm_cache.clone(),
@@ -575,7 +571,7 @@ impl CliNpmResolver for ManagedCliNpmResolver {
     InnerCliNpmResolverRef::Managed(self)
   }
 
-  fn root_node_modules_path(&self) -> Option<PathBuf> {
+  fn root_node_modules_path(&self) -> Option<&PathBuf> {
     self.fs_resolver.node_modules_path()
   }
 

cli/npm/managed/resolvers/common.rs

@@ -29,7 +29,7 @@ pub trait NpmPackageFsResolver: Send + Sync {
   fn root_dir_url(&self) -> &Url;
 
   /// The local node_modules folder if it is applicable to the implementation.
-  fn node_modules_path(&self) -> Option<PathBuf>;
+  fn node_modules_path(&self) -> Option<&PathBuf>;
 
   fn package_folder(
     &self,

cli/npm/managed/resolvers/global.rs

@@ -75,7 +75,7 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
     self.cache.root_dir_url()
   }
 
-  fn node_modules_path(&self) -> Option<PathBuf> {
+  fn node_modules_path(&self) -> Option<&PathBuf> {
     None
   }
 

cli/npm/managed/resolvers/local.rs

@@ -122,12 +122,7 @@ impl LocalNpmPackageResolver {
     };
     // Canonicalize the path so it's not pointing to the symlinked directory
     // in `node_modules` directory of the referrer.
-    canonicalize_path_maybe_not_exists_with_fs(&path, |path| {
+    canonicalize_path_maybe_not_exists_with_fs(&path, self.fs.as_ref())
-      self
-        .fs
-        .realpath_sync(path)
-        .map_err(|err| err.into_io_error())
-    })
       .map(Some)
       .map_err(|err| err.into())
   }
@@ -139,8 +134,8 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
     &self.root_node_modules_url
   }
 
-  fn node_modules_path(&self) -> Option<PathBuf> {
+  fn node_modules_path(&self) -> Option<&PathBuf> {
-    Some(self.root_node_modules_path.clone())
+    Some(&self.root_node_modules_path)
   }
 
   fn package_folder(&self, id: &NpmPackageId) -> Result<PathBuf, AnyError> {

cli/npm/mod.rs

@@ -75,7 +75,7 @@ pub trait CliNpmResolver: NpmResolver {
     }
   }
 
-  fn root_node_modules_path(&self) -> Option<PathBuf>;
+  fn root_node_modules_path(&self) -> Option<&PathBuf>;
 
   fn resolve_pkg_folder_from_deno_module_req(
     &self,

cli/standalone/binary.rs

@@ -122,6 +122,18 @@ impl SerializablePackageJsonDeps {
   }
 }
 
+#[derive(Deserialize, Serialize)]
+pub enum NodeModules {
+  Managed {
+    /// Whether this uses a node_modules directory (true) or the global cache (false).
+    node_modules_dir: bool,
+    package_json_deps: Option<SerializablePackageJsonDeps>,
+  },
+  Byonm {
+    package_json_deps: Option<SerializablePackageJsonDeps>,
+  },
+}
+
 #[derive(Deserialize, Serialize)]
 pub struct Metadata {
   pub argv: Vec<String>,
@@ -136,9 +148,7 @@ pub struct Metadata {
   pub unsafely_ignore_certificate_errors: Option<Vec<String>>,
   pub maybe_import_map: Option<(Url, String)>,
   pub entrypoint: ModuleSpecifier,
-  /// Whether this uses a node_modules directory (true) or the global cache (false).
+  pub node_modules: Option<NodeModules>,
-  pub node_modules_dir: bool,
-  pub package_json_deps: Option<SerializablePackageJsonDeps>,
 }
 
 pub fn load_npm_vfs(root_dir_path: PathBuf) -> Result<FileBackedVfs, AnyError> {
@@ -490,21 +500,42 @@ impl<'a> DenoCompileBinaryWriter<'a> {
       .resolve_import_map(self.file_fetcher)
       .await?
       .map(|import_map| (import_map.base_url().clone(), import_map.to_json()));
-    let (npm_vfs, npm_files) = match self.npm_resolver.as_inner() {
+    let (npm_vfs, npm_files, node_modules) =
+      match self.npm_resolver.as_inner() {
         InnerCliNpmResolverRef::Managed(managed) => {
           let snapshot =
             managed.serialized_valid_snapshot_for_system(&self.npm_system_info);
           if !snapshot.as_serialized().packages.is_empty() {
             let (root_dir, files) = self.build_vfs()?.into_dir_and_files();
             eszip.add_npm_snapshot(snapshot);
-          (Some(root_dir), files)
+            (
+              Some(root_dir),
+              files,
+              Some(NodeModules::Managed {
+                node_modules_dir: self
+                  .npm_resolver
+                  .root_node_modules_path()
+                  .is_some(),
+                package_json_deps: self.package_json_deps_provider.deps().map(
+                  |deps| SerializablePackageJsonDeps::from_deps(deps.clone()),
+                ),
+              }),
+            )
           } else {
-          (None, Vec::new())
+            (None, Vec::new(), None)
           }
         }
         InnerCliNpmResolverRef::Byonm(_) => {
           let (root_dir, files) = self.build_vfs()?.into_dir_and_files();
-        (Some(root_dir), files)
+          (
+            Some(root_dir),
+            files,
+            Some(NodeModules::Byonm {
+              package_json_deps: self.package_json_deps_provider.deps().map(
+                |deps| SerializablePackageJsonDeps::from_deps(deps.clone()),
+              ),
+            }),
+          )
         }
       };
 
@@ -523,11 +554,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
       ca_data,
       entrypoint: entrypoint.clone(),
       maybe_import_map,
-      node_modules_dir: self.npm_resolver.root_node_modules_path().is_some(),
+      node_modules,
-      package_json_deps: self
-        .package_json_deps_provider
-        .deps()
-        .map(|deps| SerializablePackageJsonDeps::from_deps(deps.clone())),
     };
 
     write_binary_bytes(
@@ -545,7 +572,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
       InnerCliNpmResolverRef::Managed(npm_resolver) => {
         if let Some(node_modules_path) = npm_resolver.root_node_modules_path() {
           let mut builder = VfsBuilder::new(node_modules_path.clone())?;
-          builder.add_dir_recursive(&node_modules_path)?;
+          builder.add_dir_recursive(node_modules_path)?;
           Ok(builder)
         } else {
           // DO NOT include the user's registry url as it may contain credentials,
@@ -565,9 +592,19 @@ impl<'a> DenoCompileBinaryWriter<'a> {
           Ok(builder)
         }
       }
-      InnerCliNpmResolverRef::Byonm(_) => {
+      InnerCliNpmResolverRef::Byonm(npm_resolver) => {
-        // todo(#18967): should use the node_modules directory
+        // the root_node_modules directory will always exist for byonm
-        todo!()
+        let node_modules_path = npm_resolver.root_node_modules_path().unwrap();
+        let parent_path = node_modules_path.parent().unwrap();
+        let mut builder = VfsBuilder::new(parent_path.to_path_buf())?;
+        let package_json_path = parent_path.join("package.json");
+        if package_json_path.exists() {
+          builder.add_file_at_path(&package_json_path)?;
+        }
+        if node_modules_path.exists() {
+          builder.add_dir_recursive(node_modules_path)?;
+        }
+        Ok(builder)
       }
     }
   }

cli/standalone/mod.rs

@@ -16,6 +16,7 @@ use crate::module_loader::CliNodeResolver;
 use crate::module_loader::NpmModuleLoader;
 use crate::node::CliCjsCodeAnalyzer;
 use crate::npm::create_cli_npm_resolver;
+use crate::npm::CliNpmResolverByonmCreateOptions;
 use crate::npm::CliNpmResolverCreateOptions;
 use crate::npm::CliNpmResolverManagedCreateOptions;
 use crate::npm::CliNpmResolverManagedPackageJsonInstallerOption;
@@ -311,50 +312,41 @@ pub async fn run(
     .join("node_modules");
   let npm_cache_dir = NpmCacheDir::new(root_path.clone());
   let npm_global_cache_dir = npm_cache_dir.get_cache_location();
-  let (fs, vfs_root, maybe_node_modules_path, maybe_snapshot) =
+  let cache_setting = CacheSetting::Only;
-    if let Some(snapshot) = eszip.take_npm_snapshot() {
+  let (package_json_deps_provider, fs, npm_resolver, maybe_vfs_root) =
-      let vfs_root_dir_path = if metadata.node_modules_dir {
+    match metadata.node_modules {
+      Some(binary::NodeModules::Managed {
+        node_modules_dir,
+        package_json_deps,
+      }) => {
+        // this will always have a snapshot
+        let snapshot = eszip.take_npm_snapshot().unwrap();
+        let vfs_root_dir_path = if node_modules_dir {
           root_path
         } else {
           npm_cache_dir.registry_folder(&npm_registry_url)
         };
         let vfs = load_npm_vfs(vfs_root_dir_path.clone())
           .context("Failed to load npm vfs.")?;
-      let node_modules_path = if metadata.node_modules_dir {
+        let maybe_node_modules_path = if node_modules_dir {
           Some(vfs.root().to_path_buf())
         } else {
           None
         };
-      (
+        let package_json_deps_provider =
-        Arc::new(DenoCompileFileSystem::new(vfs))
+          Arc::new(PackageJsonDepsProvider::new(
-          as Arc<dyn deno_fs::FileSystem>,
+            package_json_deps.map(|serialized| serialized.into_deps()),
-        Some(vfs_root_dir_path),
-        node_modules_path,
-        Some(snapshot),
-      )
-    } else {
-      (
-        Arc::new(deno_fs::RealFs) as Arc<dyn deno_fs::FileSystem>,
-        None,
-        None,
-        None,
-      )
-    };
-
-  let has_node_modules_dir = maybe_node_modules_path.is_some();
-  let package_json_deps_provider = Arc::new(PackageJsonDepsProvider::new(
-    metadata
-      .package_json_deps
-      .map(|serialized| serialized.into_deps()),
           ));
+        let fs = Arc::new(DenoCompileFileSystem::new(vfs))
+          as Arc<dyn deno_fs::FileSystem>;
         let npm_resolver = create_cli_npm_resolver(
           CliNpmResolverCreateOptions::Managed(CliNpmResolverManagedCreateOptions {
-      snapshot: CliNpmResolverManagedSnapshotOption::Specified(maybe_snapshot),
+            snapshot: CliNpmResolverManagedSnapshotOption::Specified(Some(snapshot)),
             maybe_lockfile: None,
             fs: fs.clone(),
             http_client: http_client.clone(),
             npm_global_cache_dir,
-      cache_setting: CacheSetting::Only,
+            cache_setting,
             text_only_progress_bar: progress_bar,
             maybe_node_modules_path,
             package_json_installer:
@@ -366,6 +358,67 @@ pub async fn run(
           }),
         )
         .await?;
+        (
+          package_json_deps_provider,
+          fs,
+          npm_resolver,
+          Some(vfs_root_dir_path),
+        )
+      }
+      Some(binary::NodeModules::Byonm { package_json_deps }) => {
+        let vfs_root_dir_path = root_path;
+        let vfs = load_npm_vfs(vfs_root_dir_path.clone())
+          .context("Failed to load npm vfs.")?;
+        let node_modules_path = vfs.root().join("node_modules");
+        let package_json_deps_provider =
+          Arc::new(PackageJsonDepsProvider::new(
+            package_json_deps.map(|serialized| serialized.into_deps()),
+          ));
+        let fs = Arc::new(DenoCompileFileSystem::new(vfs))
+          as Arc<dyn deno_fs::FileSystem>;
+        let npm_resolver =
+          create_cli_npm_resolver(CliNpmResolverCreateOptions::Byonm(
+            CliNpmResolverByonmCreateOptions {
+              fs: fs.clone(),
+              root_node_modules_dir: node_modules_path,
+            },
+          ))
+          .await?;
+        (
+          package_json_deps_provider,
+          fs,
+          npm_resolver,
+          Some(vfs_root_dir_path),
+        )
+      }
+      None => {
+        let package_json_deps_provider =
+          Arc::new(PackageJsonDepsProvider::new(None));
+        let fs = Arc::new(deno_fs::RealFs) as Arc<dyn deno_fs::FileSystem>;
+        let npm_resolver = create_cli_npm_resolver(
+          CliNpmResolverCreateOptions::Managed(CliNpmResolverManagedCreateOptions {
+            snapshot: CliNpmResolverManagedSnapshotOption::Specified(None),
+            maybe_lockfile: None,
+            fs: fs.clone(),
+            http_client: http_client.clone(),
+            npm_global_cache_dir,
+            cache_setting,
+            text_only_progress_bar: progress_bar,
+            maybe_node_modules_path: None,
+            package_json_installer:
+              CliNpmResolverManagedPackageJsonInstallerOption::ConditionalInstall(
+                package_json_deps_provider.clone(),
+              ),
+            npm_registry_url,
+            npm_system_info: Default::default(),
+          }),
+        )
+        .await?;
+        (package_json_deps_provider, fs, npm_resolver, None)
+      }
+    };
+
+  let has_node_modules_dir = npm_resolver.root_node_modules_path().is_some();
   let node_resolver = Arc::new(NodeResolver::new(
     fs.clone(),
     npm_resolver.clone().into_npm_resolver(),
@@ -409,7 +462,7 @@ pub async fn run(
   let permissions = {
     let mut permissions = metadata.permissions;
     // if running with an npm vfs, grant read access to it
-    if let Some(vfs_root) = vfs_root {
+    if let Some(vfs_root) = maybe_vfs_root {
       match &mut permissions.allow_read {
         Some(vec) if vec.is_empty() => {
           // do nothing, already granted

cli/standalone/virtual_fs.rs

@@ -92,9 +92,7 @@ impl VfsBuilder {
       if file_type.is_dir() {
         self.add_dir_recursive_internal(&path)?;
       } else if file_type.is_file() {
-        let file_bytes = std::fs::read(&path)
+        self.add_file_at_path(&path)?;
-          .with_context(|| format!("Reading {}", path.display()))?;
-        self.add_file(&path, file_bytes)?;
       } else if file_type.is_symlink() {
         let target = util::fs::canonicalize_path(&path)
           .with_context(|| format!("Reading symlink {}", path.display()))?;
@@ -163,6 +161,12 @@ impl VfsBuilder {
     Ok(current_dir)
   }
 
+  pub fn add_file_at_path(&mut self, path: &Path) -> Result<(), AnyError> {
+    let file_bytes = std::fs::read(path)
+      .with_context(|| format!("Reading {}", path.display()))?;
+    self.add_file(path, file_bytes)
+  }
+
   fn add_file(&mut self, path: &Path, data: Vec<u8>) -> Result<(), AnyError> {
     log::debug!("Adding file '{}'", path.display());
     let checksum = util::checksum::gen(&[&data]);

cli/tests/integration/compile_tests.rs

@@ -798,15 +798,36 @@ testing[WILDCARD]this
     r#"{ "dependencies": { "@denotest/esm-basic": "1" } }"#,
   );
 
-  let output = context
+  context
     .new_command()
     .args("compile --output binary main.ts")
-    .run();
+    .run()
-  output.assert_exit_code(0);
+    .assert_exit_code(0)
-  output.skip_output_check();
+    .skip_output_check();
 
-  let output = context.new_command().name(binary_path).run();
+  context
-  output.assert_matches_text("2\n");
+    .new_command()
+    .name(&binary_path)
+    .run()
+    .assert_matches_text("2\n");
+
+  // now try with byonm
+  temp_dir.remove_dir_all("node_modules");
+  temp_dir.write("deno.json", r#"{"unstable":["byonm"]}"#);
+  context.run_npm("install");
+
+  context
+    .new_command()
+    .args("compile --output binary main.ts")
+    .run()
+    .assert_exit_code(0)
+    .assert_matches_text("Check file:///[WILDCARD]/main.ts\nCompile file:///[WILDCARD]/main.ts to binary[WILDCARD]\n");
+
+  context
+    .new_command()
+    .name(&binary_path)
+    .run()
+    .assert_matches_text("2\n");
 }
 
 #[test]

cli/tools/task.rs

@@ -122,7 +122,7 @@ pub async fn execute_script(
           None => Default::default(),
         };
         let env_vars = match npm_resolver.root_node_modules_path() {
-          Some(dir_path) => collect_env_vars_with_node_modules_dir(&dir_path),
+          Some(dir_path) => collect_env_vars_with_node_modules_dir(dir_path),
           None => collect_env_vars(),
         };
         let local = LocalSet::new();

cli/util/fs.rs

@@ -6,6 +6,7 @@ pub use deno_core::normalize_path;
 use deno_core::unsync::spawn_blocking;
 use deno_core::ModuleSpecifier;
 use deno_runtime::deno_crypto::rand;
+use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node::PathClean;
 use std::borrow::Cow;
 use std::env::current_dir;
@@ -187,10 +188,19 @@ pub fn canonicalize_path(path: &Path) -> Result<PathBuf, Error> {
 pub fn canonicalize_path_maybe_not_exists(
   path: &Path,
 ) -> Result<PathBuf, Error> {
-  canonicalize_path_maybe_not_exists_with_fs(path, canonicalize_path)
+  canonicalize_path_maybe_not_exists_with_custom_fn(path, canonicalize_path)
 }
 
 pub fn canonicalize_path_maybe_not_exists_with_fs(
+  path: &Path,
+  fs: &dyn FileSystem,
+) -> Result<PathBuf, Error> {
+  canonicalize_path_maybe_not_exists_with_custom_fn(path, |path| {
+    fs.realpath_sync(path).map_err(|err| err.into_io_error())
+  })
+}
+
+fn canonicalize_path_maybe_not_exists_with_custom_fn(
   path: &Path,
   canonicalize: impl Fn(&Path) -> Result<PathBuf, Error>,
 ) -> Result<PathBuf, Error> {