fix(vendor): ignore import map in output directory instead of erroring (#14998)

cli/args/mod.rs

@@ -44,6 +44,14 @@ use crate::file_fetcher::CacheSetting;
 use crate::lockfile::Lockfile;
 use crate::version;
 
+/// Overrides for the options below that when set will
+/// use these values over the values derived from the
+/// CLI flags or config file.
+#[derive(Default)]
+struct CliOptionOverrides {
+  import_map_specifier: Option<Option<ModuleSpecifier>>,
+}
+
 /// Holds the common options used by many sub commands
 /// and provides some helper function for creating common objects.
 pub struct CliOptions {
@@ -51,6 +59,7 @@ pub struct CliOptions {
   // application need not concern itself with, so keep these private
   flags: Flags,
   maybe_config_file: Option<ConfigFile>,
+  overrides: CliOptionOverrides,
 }
 
 impl CliOptions {
@@ -72,6 +81,7 @@ impl CliOptions {
     Ok(Self {
       maybe_config_file,
       flags,
+      overrides: Default::default(),
     })
   }
 
@@ -113,13 +123,21 @@ impl CliOptions {
 
   /// Based on an optional command line import map path and an optional
   /// configuration file, return a resolved module specifier to an import map.
-  pub fn resolve_import_map_path(
+  pub fn resolve_import_map_specifier(
     &self,
   ) -> Result<Option<ModuleSpecifier>, AnyError> {
-    resolve_import_map_specifier(
+    match self.overrides.import_map_specifier.clone() {
+      Some(path) => Ok(path),
+      None => resolve_import_map_specifier(
         self.flags.import_map_path.as_deref(),
         self.maybe_config_file.as_ref(),
-    )
+      ),
+    }
+  }
+
+  /// Overrides the import map specifier to use.
+  pub fn set_import_map_specifier(&mut self, path: Option<ModuleSpecifier>) {
+    self.overrides.import_map_specifier = Some(path);
   }
 
   pub fn resolve_root_cert_store(&self) -> Result<RootCertStore, AnyError> {

cli/main.rs

@@ -769,7 +769,7 @@ async fn bundle_command(
 
       if let Ok(Some(import_map_path)) = ps
         .options
-        .resolve_import_map_path()
+        .resolve_import_map_specifier()
         .map(|ms| ms.and_then(|ref s| s.to_file_path().ok()))
       {
         paths_to_watch.push(import_map_path);
@@ -1237,8 +1237,7 @@ async fn vendor_command(
   flags: Flags,
   vendor_flags: VendorFlags,
 ) -> Result<i32, AnyError> {
-  let ps = ProcState::build(flags).await?;
+  tools::vendor::vendor(flags, vendor_flags).await?;
-  tools::vendor::vendor(ps, vendor_flags).await?;
   Ok(0)
 }
 

cli/proc_state.rs

@@ -117,7 +117,7 @@ impl ProcState {
 
     if let Ok(Some(import_map_path)) = ps
       .options
-      .resolve_import_map_path()
+      .resolve_import_map_specifier()
       .map(|ms| ms.and_then(|ref s| s.to_file_path().ok()))
     {
       files_to_watch_sender.send(vec![import_map_path]).unwrap();
@@ -153,7 +153,8 @@ impl ProcState {
     let lockfile = cli_options
       .resolve_lock_file()?
       .map(|f| Arc::new(Mutex::new(f)));
-    let maybe_import_map_specifier = cli_options.resolve_import_map_path()?;
+    let maybe_import_map_specifier =
+      cli_options.resolve_import_map_specifier()?;
 
     let maybe_import_map =
       if let Some(import_map_specifier) = maybe_import_map_specifier {

cli/tests/integration/vendor_tests.rs

@@ -72,43 +72,6 @@ fn output_dir_exists() {
   assert!(status.success());
 }
 
-#[test]
-fn import_map_output_dir() {
-  let t = TempDir::new();
-  t.write("mod.ts", "");
-  t.create_dir_all("vendor");
-  t.write(
-    "vendor/import_map.json",
-    "{ \"imports\": { \"https://localhost/\": \"./localhost/\" }}",
-  );
-
-  let deno = util::deno_cmd()
-    .current_dir(t.path())
-    .env("NO_COLOR", "1")
-    .arg("vendor")
-    .arg("--force")
-    .arg("--import-map")
-    .arg("vendor/import_map.json")
-    .arg("mod.ts")
-    .stdout(Stdio::piped())
-    .stderr(Stdio::piped())
-    .spawn()
-    .unwrap();
-  let output = deno.wait_with_output().unwrap();
-  assert_eq!(
-    String::from_utf8_lossy(&output.stderr).trim(),
-    format!(
-      concat!(
-        "error: Specifying an import map file ({}) in the deno vendor ",
-        "output directory is not supported. Please specify no import ",
-        "map or one located outside this directory.",
-      ),
-      PathBuf::from("vendor").join("import_map.json").display(),
-    ),
-  );
-  assert!(!output.status.success());
-}
-
 #[test]
 fn standard_test() {
   let _server = http_server();
@@ -185,6 +148,57 @@ fn standard_test() {
   assert!(output.status.success());
 }
 
+#[test]
+fn import_map_output_dir() {
+  let _server = http_server();
+  let t = TempDir::new();
+  t.write("mod.ts", "");
+  t.create_dir_all("vendor");
+  t.write(
+    "vendor/import_map.json",
+    // will be ignored
+    "{ \"imports\": { \"https://localhost:4545/\": \"./localhost/\" }}",
+  );
+  t.write(
+    "deno.json",
+    "{ \"import_map\": \"./vendor/import_map.json\" }",
+  );
+  t.write(
+    "my_app.ts",
+    "import {Logger} from 'http://localhost:4545/vendor/logger.ts'; new Logger().log('outputted');",
+  );
+
+  let deno = util::deno_cmd()
+    .current_dir(t.path())
+    .env("NO_COLOR", "1")
+    .arg("vendor")
+    .arg("--force")
+    .arg("--import-map")
+    .arg("vendor/import_map.json")
+    .arg("my_app.ts")
+    .stdout(Stdio::piped())
+    .stderr(Stdio::piped())
+    .spawn()
+    .unwrap();
+  let output = deno.wait_with_output().unwrap();
+  assert_eq!(
+    String::from_utf8_lossy(&output.stderr).trim(),
+    format!(
+      concat!(
+        "Ignoring import map. Specifying an import map file ({}) in the deno ",
+        "vendor output directory is not supported. If you wish to use an ",
+        "import map while vendoring, please specify one located outside this ",
+        "directory.\n",
+        "Download http://localhost:4545/vendor/logger.ts\n",
+        "{}",
+      ),
+      PathBuf::from("vendor").join("import_map.json").display(),
+      success_text_updated_deno_json("1 module", "vendor/"),
+    )
+  );
+  assert!(output.status.success());
+}
+
 #[test]
 fn remote_module_test() {
   let _server = http_server();
@@ -487,16 +501,8 @@ fn update_existing_config_test() {
   assert_eq!(
     String::from_utf8_lossy(&output.stderr).trim(),
     format!(
-      concat!(
+      "Download http://localhost:4545/vendor/logger.ts\n{}",
-        "Download http://localhost:4545/vendor/logger.ts\n",
+      success_text_updated_deno_json("1 module", "vendor2",)
-        "Vendored 1 module into vendor2 directory.\n\n",
-        "Updated your local Deno configuration file with a reference to the ",
-        "new vendored import map at {}. Invoking Deno subcommands will ",
-        "now automatically resolve using the vendored modules. You may override ",
-        "this by providing the `--import-map <other-import-map>` flag or by ",
-        "manually editing your Deno configuration file."
-      ),
-      PathBuf::from("vendor2").join("import_map.json").display(),
     )
   );
   assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "");
@@ -541,3 +547,27 @@ fn success_text(module_count: &str, dir: &str, has_import_map: bool) -> String {
   }
   text
 }
+
+fn success_text_updated_deno_json(module_count: &str, dir: &str) -> String {
+  format!(
+    concat!(
+      "Vendored {} into {} directory.\n\n",
+      "Updated your local Deno configuration file with a reference to the ",
+      "new vendored import map at {}import_map.json. Invoking Deno subcommands will ",
+      "now automatically resolve using the vendored modules. You may override ",
+      "this by providing the `--import-map <other-import-map>` flag or by ",
+      "manually editing your Deno configuration file.",
+    ),
+    module_count,
+    dir,
+    if dir != "vendor/" {
+      format!(
+        "{}{}",
+        dir.trim_end_matches('/'),
+        if cfg!(windows) { '\\' } else { '/' }
+      )
+    } else {
+      dir.to_string()
+    }
+  )
+}

cli/tools/vendor/mod.rs

@@ -2,6 +2,7 @@
 
 use std::path::Path;
 use std::path::PathBuf;
+use std::sync::Arc;
 
 use deno_ast::ModuleSpecifier;
 use deno_core::anyhow::bail;
@@ -11,6 +12,8 @@ use deno_core::resolve_url_or_path;
 use deno_runtime::permissions::Permissions;
 use log::warn;
 
+use crate::args::CliOptions;
+use crate::args::Flags;
 use crate::args::FmtOptionsConfig;
 use crate::args::VendorFlags;
 use crate::fs_util;
@@ -30,14 +33,20 @@ mod specifiers;
 #[cfg(test)]
 mod test;
 
-pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> {
+pub async fn vendor(
-  let raw_output_dir = match &flags.output_path {
+  flags: Flags,
+  vendor_flags: VendorFlags,
+) -> Result<(), AnyError> {
+  let mut cli_options = CliOptions::from_flags(flags)?;
+  let raw_output_dir = match &vendor_flags.output_path {
     Some(output_path) => output_path.to_owned(),
     None => PathBuf::from("vendor/"),
   };
   let output_dir = fs_util::resolve_from_cwd(&raw_output_dir)?;
-  validate_output_dir(&output_dir, &flags, &ps)?;
+  validate_output_dir(&output_dir, &vendor_flags)?;
-  let graph = create_graph(&ps, &flags).await?;
+  validate_options(&mut cli_options, &output_dir)?;
+  let ps = ProcState::from_options(Arc::new(cli_options)).await?;
+  let graph = create_graph(&ps, &vendor_flags).await?;
   let vendored_count = build::build(
     graph,
     &output_dir,
@@ -86,7 +95,6 @@ pub async fn vendor(ps: ProcState, flags: VendorFlags) -> Result<(), AnyError> {
 fn validate_output_dir(
   output_dir: &Path,
   flags: &VendorFlags,
-  ps: &ProcState,
 ) -> Result<(), AnyError> {
   if !flags.force && !is_dir_empty(output_dir)? {
     bail!(concat!(
@@ -94,12 +102,17 @@ fn validate_output_dir(
       "--force to ignore this error and potentially overwrite its contents.",
     ));
   }
+  Ok(())
+}
 
+fn validate_options(
+  options: &mut CliOptions,
+  output_dir: &Path,
+) -> Result<(), AnyError> {
   // check the import map
-  if let Some(import_map_path) = ps
+  if let Some(import_map_path) = options
-    .maybe_import_map
+    .resolve_import_map_specifier()?
-    .as_ref()
+    .and_then(|p| specifier_to_file_path(&p).ok())
-    .and_then(|m| specifier_to_file_path(m.base_url()).ok())
     .and_then(|p| fs_util::canonicalize_path(&p).ok())
   {
     // make the output directory in order to canonicalize it for the check below
@@ -114,11 +127,12 @@ fn validate_output_dir(
       let cwd = fs_util::canonicalize_path(&std::env::current_dir()?)?;
       // We don't allow using the output directory to help generate the
       // new state because this may lead to cryptic error messages.
-      bail!(
+      log::warn!(
         concat!(
-          "Specifying an import map file ({}) in the deno vendor output ",
+          "Ignoring import map. Specifying an import map file ({}) in the ",
-          "directory is not supported. Please specify no import map or one ",
+          "deno vendor output directory is not supported. If you wish to use ",
-          "located outside this directory."
+          "an import map while vendoring, please specify one located outside ",
+          "this directory."
         ),
         import_map_path
           .strip_prefix(&cwd)
@@ -126,6 +140,9 @@ fn validate_output_dir(
           .display()
           .to_string(),
       );
+
+      // don't use an import map in the config
+      options.set_import_map_specifier(None);
     }
   }