fix(bundle): rework how patterns for externals are handled (#29680)

Now aligns with esbuild's behavior

cli/tools/bundle/esbuild.rs

@@ -61,8 +61,15 @@ pub async fn ensure_esbuild(
   let nv =
     PackageNv::from_str(&format!("{}@{}", pkg_name, ESBUILD_VERSION)).unwrap();
   let api = npm_registry_info.as_npm_registry_api();
-  let info = api.package_info(&pkg_name).await?;
+  let mut info = api.package_info(&pkg_name).await?;
-  let version_info = info.version_info(&nv, &workspace_patch_packages.0)?;
+  let version_info = match info.version_info(&nv, &workspace_patch_packages.0) {
+    Ok(version_info) => version_info,
+    Err(_) => {
+      api.mark_force_reload();
+      info = api.package_info(&pkg_name).await?;
+      info.version_info(&nv, &workspace_patch_packages.0)?
+    }
+  };
   if let Some(dist) = &version_info.dist {
     let registry_url = npmrc.get_registry_url(&nv.name);
     let package_folder =

cli/tools/bundle/externals.rs

@@ -0,0 +1,257 @@
+// Copyright 2018-2025 the Deno authors. MIT license.
+
+use std::collections::HashSet;
+use std::path::Path;
+
+#[derive(Debug)]
+struct Pattern {
+  prefix: String,
+  suffix: String,
+}
+impl Pattern {
+  fn new_with_wildcard(pattern: &str, wildcard_index: usize) -> Self {
+    let prefix = pattern[..wildcard_index].to_string();
+    let suffix = pattern[wildcard_index + 1..].to_string();
+    Self { prefix, suffix }
+  }
+
+  fn new_prefix(pattern: String) -> Self {
+    Self {
+      prefix: pattern,
+      suffix: String::new(),
+    }
+  }
+}
+#[derive(Debug)]
+struct Patterns {
+  patterns: Vec<Pattern>,
+  exact: HashSet<String>,
+}
+
+impl Patterns {
+  fn is_match(&self, path: &str) -> bool {
+    if self.exact.contains(path) {
+      return true;
+    }
+    for pattern in &self.patterns {
+      if path.starts_with(&pattern.prefix) && path.ends_with(&pattern.suffix) {
+        return true;
+      }
+    }
+    false
+  }
+}
+#[derive(Debug)]
+pub struct ExternalsMatcher {
+  pre_resolve: Patterns,
+  post_resolve: Patterns,
+}
+
+fn is_package_path(path: &str) -> bool {
+  !path.starts_with('/')
+    && !path.starts_with("./")
+    && !path.starts_with("../")
+    && path != "."
+    && path != ".."
+}
+
+fn to_absolute_path(path: &str, cwd: &Path) -> String {
+  if path.starts_with('/') {
+    path.to_string()
+  } else {
+    let path = cwd.join(path);
+    deno_path_util::normalize_path(&path)
+      .to_string_lossy()
+      .to_string()
+  }
+}
+
+impl ExternalsMatcher {
+  /// A set of patterns indicating files to mark as external.
+  ///
+  /// For instance given, `--external="*.node" --external="*.wasm"`, the matcher will match
+  /// any path that ends with `.node` or `.wasm`.
+  pub fn new(externals: &[String], cwd: &Path) -> Self {
+    let mut pre_resolve = Patterns {
+      patterns: vec![],
+      exact: HashSet::new(),
+    };
+    let mut post_resolve = Patterns {
+      patterns: vec![],
+      exact: HashSet::new(),
+    };
+    for external in externals {
+      let wildcard = external.find("*");
+      if let Some(wildcard_index) = wildcard {
+        if external[wildcard_index + 1..].contains('*') {
+          log::error!("Externals must not contain multiple wildcards");
+          continue;
+        }
+        pre_resolve
+          .patterns
+          .push(Pattern::new_with_wildcard(external, wildcard_index));
+        if !is_package_path(external) {
+          let normalized = to_absolute_path(external, cwd);
+          if let Some(index) = normalized.find('*') {
+            post_resolve
+              .patterns
+              .push(Pattern::new_with_wildcard(&normalized, index));
+          }
+        }
+      } else {
+        pre_resolve.exact.insert(external.to_string());
+        if is_package_path(external) {
+          pre_resolve
+            .patterns
+            .push(Pattern::new_prefix([external, "/"].join("")));
+        } else {
+          let normalized = to_absolute_path(external, cwd);
+          post_resolve.exact.insert(normalized);
+        }
+      }
+    }
+    Self {
+      pre_resolve,
+      post_resolve,
+    }
+  }
+
+  pub fn is_pre_resolve_match(&self, path: &str) -> bool {
+    self.pre_resolve.is_match(path)
+  }
+
+  pub fn is_post_resolve_match(&self, path: &str) -> bool {
+    self.post_resolve.is_match(path)
+  }
+}
+
+#[cfg(test)]
+mod tests {
+  #![allow(clippy::print_stderr)]
+  use std::path::Path;
+
+  use super::ExternalsMatcher;
+
+  struct Matches {
+    pre_resolve: Vec<String>,
+    post_resolve: Vec<String>,
+  }
+
+  fn matches_all<'a, S: AsRef<str>>(
+    patterns: impl IntoIterator<Item = S>,
+    matches: Matches,
+    no_match: impl IntoIterator<Item = &'a str>,
+  ) -> bool {
+    let patterns = patterns
+      .into_iter()
+      .map(|p| p.as_ref().to_string())
+      .collect::<Vec<_>>();
+    let cwd = std::env::current_dir().unwrap();
+    let matcher = ExternalsMatcher::new(&patterns, &cwd);
+    for path in matches.pre_resolve {
+      if !matcher.is_pre_resolve_match(&path) {
+        eprintln!("failed to match pre resolve: {}", path);
+        return false;
+      }
+    }
+    for path in matches.post_resolve {
+      if !matcher.is_post_resolve_match(&path) {
+        eprintln!("failed to match post resolve: {}", path);
+        return false;
+      }
+    }
+    for path in no_match {
+      if matcher.is_pre_resolve_match(path) {
+        eprintln!("matched pre resolve when it should not: {}", path);
+        return false;
+      }
+      if matcher.is_post_resolve_match(path) {
+        eprintln!("matched post resolve when it should not: {}", path);
+        return false;
+      }
+    }
+    true
+  }
+
+  fn s<S: AsRef<str>>(s: impl IntoIterator<Item = S>) -> Vec<String> {
+    s.into_iter().map(|p| p.as_ref().to_string()).collect()
+  }
+
+  fn path_str(path: impl AsRef<Path>) -> String {
+    path.as_ref().to_string_lossy().to_string()
+  }
+
+  #[test]
+  fn matches_package_path() {
+    assert!(matches_all(
+      ["chalk"],
+      Matches {
+        pre_resolve: s(["chalk", "chalk/foo"]),
+        post_resolve: vec![],
+      },
+      ["other/chalk", "./chalk/foo.ts", "./chalk"]
+    ));
+    assert!(matches_all(
+      ["@std/fs"],
+      Matches {
+        pre_resolve: s(["@std/fs", "@std/fs/foo"]),
+        post_resolve: vec![],
+      },
+      ["other/@std/fs", "./@std/fs/foo.ts", "./@std/fs"]
+    ));
+  }
+
+  #[test]
+  fn matches_path() {
+    assert!(matches_all(
+      ["/node_modules/fo"],
+      Matches {
+        pre_resolve: s(["/node_modules/fo"]),
+        post_resolve: s(["/node_modules/fo"]),
+      },
+      ["/node_modules/foo"]
+    ));
+
+    let cwd = std::env::current_dir().unwrap();
+    assert!(matches_all(
+      ["./foo"],
+      Matches {
+        pre_resolve: s(["./foo"]),
+        post_resolve: s([path_str(cwd.join("foo"))]),
+      },
+      ["other/foo", "./foo.ts", "./foo/bar", "thing/./foo"]
+    ));
+  }
+
+  #[test]
+  fn matches_wildcard() {
+    assert!(matches_all(
+      ["*.node"],
+      Matches {
+        pre_resolve: s(["foo.node", "foo/bar.node"]),
+        post_resolve: vec![],
+      },
+      ["foo.ts", "./foo.node.ts", "./foo/bar.node.ts"]
+    ));
+    assert!(matches_all(
+      ["@std/*"],
+      Matches {
+        pre_resolve: s(["@std/fs", "@std/fs/foo"]),
+        post_resolve: vec![],
+      },
+      ["other/@std/fs", "./@std/fs/foo.ts", "./@std/fs"]
+    ));
+    let cwd = std::env::current_dir().unwrap();
+    assert!(matches_all(
+      ["./foo/*"],
+      Matches {
+        pre_resolve: s(["./foo/bar", "./foo/baz"]),
+        post_resolve: vec![
+          path_str(cwd.join("foo").join("bar")),
+          path_str(cwd.join("foo").join("baz")),
+        ],
+      },
+      ["other/foo/bar", "./bar/foo", "./bar/./foo/bar"]
+    ));
+  }
+}

cli/tools/bundle/mod.rs

@@ -1,6 +1,7 @@
 // Copyright 2018-2025 the Deno authors. MIT license.
 
 mod esbuild;
+mod externals;
 
 use std::path::Path;
 use std::rc::Rc;
@@ -26,7 +27,6 @@ use indexmap::IndexMap;
 use node_resolver::errors::PackageSubpathResolveError;
 use node_resolver::NodeResolutionKind;
 use node_resolver::ResolutionMode;
-use regex::Regex;
 use sys_traits::EnvCurrentDir;
 
 use crate::args::BundleFlags;
@@ -42,27 +42,7 @@ use crate::module_loader::PrepareModuleLoadOptions;
 use crate::node::CliNodeResolver;
 use crate::npm::CliNpmResolver;
 use crate::resolver::CliResolver;
-
+use crate::tools::bundle::externals::ExternalsMatcher;
-/// Given a set of pattern indicating files to mark as external,
-/// return a regex that matches any of those patterns.
-///
-/// For instance given, `--external="*.node" --external="*.wasm"`, the regex will match
-/// any path that ends with `.node` or `.wasm`.
-pub fn externals_regex(external: &[String]) -> Regex {
-  let mut regex_str = String::new();
-  for (i, e) in external.iter().enumerate() {
-    if i > 0 {
-      regex_str.push('|');
-    }
-    regex_str.push_str("(^");
-    if e.starts_with("/") {
-      regex_str.push_str(".*");
-    }
-    regex_str.push_str(&regex::escape(e).replace("\\*", ".*"));
-    regex_str.push(')');
-  }
-  regex::Regex::new(&regex_str).unwrap()
-}
 
 pub async fn bundle(
   flags: Arc<Flags>,
@@ -97,7 +77,7 @@ pub async fn bundle(
     .create_for_main(root_permissions.clone())
     .module_loader;
   let sys = factory.sys();
-  let init_cwd = cli_options.initial_cwd().canonicalize()?;
+  let init_cwd = cli_options.initial_cwd().to_path_buf();
 
   #[allow(clippy::arc_with_non_send_sync)]
   let plugin_handler = Arc::new(DenoPluginHandler {
@@ -111,11 +91,10 @@ pub async fn bundle(
     npm_resolver: npm_resolver.clone(),
     node_resolver: node_resolver.clone(),
     module_loader: module_loader.clone(),
-    // TODO(nathanwhit): look at the external patterns to give diagnostics for probably incorrect patterns
+    externals_matcher: if bundle_flags.external.is_empty() {
-    externals_regex: if bundle_flags.external.is_empty() {
       None
     } else {
-      Some(externals_regex(&bundle_flags.external))
+      Some(ExternalsMatcher::new(&bundle_flags.external, &init_cwd))
     },
   });
   let start = std::time::Instant::now();
@@ -367,7 +346,7 @@ struct DenoPluginHandler {
   npm_resolver: CliNpmResolver,
   node_resolver: Arc<CliNodeResolver>,
   module_loader: Rc<dyn ModuleLoader>,
-  externals_regex: Option<Regex>,
+  externals_matcher: Option<ExternalsMatcher>,
 }
 
 #[async_trait::async_trait(?Send)]
@@ -377,8 +356,8 @@ impl esbuild_client::PluginHandler for DenoPluginHandler {
     args: esbuild_client::OnResolveArgs,
   ) -> Result<Option<esbuild_client::OnResolveResult>, AnyError> {
     log::debug!("{}: {args:?}", deno_terminal::colors::cyan("on_resolve"));
-    if let Some(reg) = &self.externals_regex {
+    if let Some(matcher) = &self.externals_matcher {
-      if reg.is_match(&args.path) {
+      if matcher.is_pre_resolve_match(&args.path) {
         return Ok(Some(esbuild_client::OnResolveResult {
           external: Some(true),
           path: Some(args.path),
@@ -397,6 +376,16 @@ impl esbuild_client::PluginHandler for DenoPluginHandler {
     )?;
 
     Ok(result.map(|r| {
+      // TODO(nathanwhit): remap the resolved path to be relative
+      // to the output file. It will be tricky to figure out which
+      // output file this import will end up in. We may have to use the metafile and rewrite at the end
+      let is_external = r.starts_with("node:")
+        || self
+          .externals_matcher
+          .as_ref()
+          .map(|matcher| matcher.is_post_resolve_match(&r))
+          .unwrap_or(false);
+
       esbuild_client::OnResolveResult {
         namespace: if r.starts_with("jsr:")
           || r.starts_with("https:")
@@ -407,14 +396,7 @@ impl esbuild_client::PluginHandler for DenoPluginHandler {
         } else {
           None
         },
-        external: Some(
+        external: Some(is_external),
-          r.starts_with("node:")
-            || self
-              .externals_regex
-              .as_ref()
-              .map(|reg| reg.is_match(&r))
-              .unwrap_or(false),
-        ),
         path: Some(r),
         plugin_name: Some("deno".to_string()),
         plugin_data: None,

tests/specs/bundle/externals/__test__.jsonc

@@ -0,0 +1,17 @@
+{
+  "tempDir": true,
+  "tests": {
+    "requires_external": {
+      "steps": [{
+        "args": "bundle --external=foo --external=bar require_external.cjs",
+        "output": "[WILDCARD]require(\"foo\");[WILDCARD]require(\"bar/baz\");[WILDCARD]"
+      }]
+    },
+    "imports_external": {
+      "steps": [{
+        "args": "bundle --external=./ext/* --external=jsr:@std/* imports_ext.ts",
+        "output": "import.out"
+      }]
+    }
+  }
+}

tests/specs/bundle/externals/deno.jsonc

@@ -0,0 +1,5 @@
+{
+  "imports": {
+    "mappedfoo": "./ext/foo.ts"
+  }
+}

tests/specs/bundle/externals/ext/bar.ts

@@ -0,0 +1 @@
+export default "bar";

tests/specs/bundle/externals/ext/foo.ts

@@ -0,0 +1 @@
+export default "foo";

tests/specs/bundle/externals/import.out

@@ -0,0 +1,4 @@
+[WILDCARD]import foo from "[WILDCARD]foo.ts";
+[WILDCARD]import bar from "[WILDCARD]ext/bar.ts";
+import * as fs from "jsr:@std/fs";
+[WILDCARD]console.log(fs.CRLF);[WILDCARD]

tests/specs/bundle/externals/imports_ext.ts

@@ -0,0 +1,7 @@
+import foo from "mappedfoo";
+import bar from "./ext/bar.ts";
+import * as fs from "jsr:@std/fs";
+
+console.log(foo);
+console.log(bar);
+console.log(fs.CRLF);

tests/specs/bundle/externals/require_external.cjs

@@ -0,0 +1,4 @@
+const foo = require("foo");
+const bar = require("bar/baz");
+console.log(foo);
+console.log(bar);