From 8cefe97b09ef76727509ffad088ec04fd61cba7c Mon Sep 17 00:00:00 2001 From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> Date: Thu, 26 Jun 2025 11:35:19 -0700 Subject: [PATCH] fix(bundle): only replace require shim in js files, spruce up output (#29892) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We were trying to do the hack of replacing the esbuild require shim on every output file, regardless of whether it was js/ts. This fixes that. In addition, this spruces up the output of deno bundle a bit. before: ![Screenshot 2025-06-25 at 6 33 55 PM](https://github.com/user-attachments/assets/9f2a4bfb-7f00-422e-8b45-84141b8447b7) after: ![Screenshot 2025-06-25 at 6 33 32 PM](https://github.com/user-attachments/assets/567cee6b-8f72-4722-878a-849704ecbe79) --- cli/tools/bundle/mod.rs | 159 +++++++++++++----- tests/integration/watcher_tests.rs | 4 +- .../bundle/import_meta_main/__test__.jsonc | 2 +- .../import_meta_main/import_meta_main.out | 3 + tests/specs/bundle/main/__test__.jsonc | 12 +- tests/specs/bundle/main/json_import.out | 3 + tests/specs/bundle/main/jsr_specifier.out | 3 + tests/specs/bundle/main/npm_specifier.out | 3 + .../main/npm_specifier_with_import_map.out | 3 + .../bundle/main/requires_node_builtin.out | 3 + tests/specs/bundle/main/sloppy_imports.out | 3 + .../bundle/multiple_entries/__test__.jsonc | 2 +- .../multiple_entries/multiple_entries.out | 4 + tests/specs/bundle/sourcemap/__test__.jsonc | 6 +- .../bundle/sourcemap/sourcemap_external.out | 4 + .../bundle/sourcemap/sourcemap_inline.out | 3 + .../bundle/sourcemap/sourcemap_linked.out | 4 + 17 files changed, 168 insertions(+), 53 deletions(-) create mode 100644 tests/specs/bundle/import_meta_main/import_meta_main.out create mode 100644 tests/specs/bundle/main/json_import.out create mode 100644 tests/specs/bundle/main/jsr_specifier.out create mode 100644 tests/specs/bundle/main/npm_specifier.out create mode 100644 tests/specs/bundle/main/npm_specifier_with_import_map.out create mode 100644 tests/specs/bundle/main/requires_node_builtin.out create mode 100644 tests/specs/bundle/main/sloppy_imports.out create mode 100644 tests/specs/bundle/multiple_entries/multiple_entries.out create mode 100644 tests/specs/bundle/sourcemap/sourcemap_external.out create mode 100644 tests/specs/bundle/sourcemap/sourcemap_inline.out create mode 100644 tests/specs/bundle/sourcemap/sourcemap_linked.out diff --git a/cli/tools/bundle/mod.rs b/cli/tools/bundle/mod.rs index 8ce8932e59..82f0dfca6a 100644 --- a/cli/tools/bundle/mod.rs +++ b/cli/tools/bundle/mod.rs @@ -4,12 +4,14 @@ mod esbuild; mod externals; mod transform; +use std::borrow::Cow; use std::cell::RefCell; use std::path::Path; use std::path::PathBuf; use std::rc::Rc; use std::sync::Arc; use std::sync::LazyLock; +use std::time::Duration; use deno_ast::EmitOptions; use deno_ast::MediaType; @@ -158,16 +160,11 @@ pub async fn bundle( handle_esbuild_errors_and_warnings(&response, &init_cwd); if response.errors.is_empty() { - process_result(&response, *DISABLE_HACK)?; + let metafile = metafile_from_response(&response)?; + let output_infos = process_result(&response, &init_cwd, *DISABLE_HACK)?; if bundle_flags.output_dir.is_some() || bundle_flags.output_path.is_some() { - log::info!( - "{}", - deno_terminal::colors::green(format!( - "bundled in {}", - crate::display::human_elapsed(start.elapsed().as_millis()), - )) - ); + print_finished_message(&metafile, &output_infos, start.elapsed())?; } } @@ -178,6 +175,16 @@ pub async fn bundle( Ok(()) } +fn metafile_from_response( + response: &BuildResponse, +) -> Result { + Ok(serde_json::from_str::( + response.metafile.as_deref().ok_or_else(|| { + deno_core::anyhow::anyhow!("expected a metafile to be present") + })?, + )?) +} + async fn bundle_watch( flags: Arc, bundler: EsbuildBundler, @@ -217,14 +224,10 @@ async fn bundle_watch( let response = bundler.rebuild().await?; handle_esbuild_errors_and_warnings(&response, &bundler.cwd); if response.errors.is_empty() { - process_result(&response, *DISABLE_HACK)?; - log::info!( - "{}", - deno_terminal::colors::green(format!( - "bundled in {}", - crate::display::human_elapsed(start.elapsed().as_millis()), - )) - ); + let metafile = metafile_from_response(&response)?; + let output_infos = + process_result(&response, &bundler.cwd, *DISABLE_HACK)?; + print_finished_message(&metafile, &output_infos, start.elapsed())?; let new_watched = get_input_paths_for_watch(&response); *current_roots.borrow_mut() = new_watched.clone(); @@ -1100,9 +1103,7 @@ fn configure_esbuild_flags(bundle_flags: &BundleFlags) -> EsbuildFlags { } else if let Some(output_path) = bundle_flags.output_path.clone() { builder.outfile(output_path); } - if bundle_flags.watch { - builder.metafile(true); - } + builder.metafile(true); match bundle_flags.platform { crate::args::BundlePlatform::Browser => { @@ -1135,38 +1136,116 @@ fn handle_esbuild_errors_and_warnings( } } +fn is_js(path: &Path) -> bool { + if let Some(ext) = path.extension() { + matches!( + ext.to_string_lossy().as_ref(), + "js" | "mjs" | "cjs" | "jsx" | "ts" | "tsx" | "mts" | "cts" | "dts" + ) + } else { + false + } +} + +struct OutputFileInfo { + relative_path: PathBuf, + size: usize, + is_js: bool, +} fn process_result( response: &BuildResponse, - // init_cwd: &Path, + cwd: &Path, should_replace_require_shim: bool, -) -> Result<(), AnyError> { - if let Some(output_files) = response.output_files.as_ref() { - let mut exists_cache = std::collections::HashSet::new(); - for file in output_files.iter() { +) -> Result, AnyError> { + let mut exists_cache = std::collections::HashSet::new(); + let output_files = response + .output_files + .as_ref() + .map(Cow::Borrowed) + .unwrap_or_default(); + let mut output_infos = Vec::new(); + for file in output_files.iter() { + let path = Path::new(&file.path); + let relative_path = + pathdiff::diff_paths(path, cwd).unwrap_or_else(|| path.to_path_buf()); + let is_js = is_js(path); + let bytes = if is_js || file.path.ends_with("") { let string = String::from_utf8(file.contents.clone())?; let string = if should_replace_require_shim { replace_require_shim(&string) } else { string }; + Cow::Owned(string.into_bytes()) + } else { + Cow::Borrowed(&file.contents) + }; - if file.path == "" { - crate::display::write_to_stdout_ignore_sigpipe(string.as_bytes())?; - continue; - } - let path = PathBuf::from(&file.path); - - if let Some(parent) = path.parent() { - if !exists_cache.contains(parent) { - if !parent.exists() { - std::fs::create_dir_all(parent)?; - } - exists_cache.insert(parent.to_path_buf()); - } - } - - std::fs::write(&file.path, string)?; + if file.path.ends_with("") { + crate::display::write_to_stdout_ignore_sigpipe(bytes.as_slice())?; + continue; } + + if let Some(parent) = path.parent() { + if !exists_cache.contains(parent) { + if !parent.exists() { + std::fs::create_dir_all(parent)?; + } + exists_cache.insert(parent.to_path_buf()); + } + } + + output_infos.push(OutputFileInfo { + relative_path, + size: bytes.len(), + is_js, + }); + + std::fs::write(path, bytes.as_ref())?; } + Ok(output_infos) +} + +fn print_finished_message( + metafile: &esbuild_client::Metafile, + output_infos: &[OutputFileInfo], + duration: Duration, +) -> Result<(), AnyError> { + let mut output = String::new(); + output.push_str(&format!( + "{} {} module{} in {}", + deno_terminal::colors::green("Bundled"), + metafile.inputs.len(), + if metafile.inputs.len() == 1 { "" } else { "s" }, + crate::display::human_elapsed(duration.as_millis()), + )); + + let longest = output_infos + .iter() + .map(|info| info.relative_path.to_string_lossy().len()) + .max() + .unwrap_or(0); + for info in output_infos { + output.push_str(&format!( + "\n {} {}", + if info.is_js { + deno_terminal::colors::cyan(format!( + "{: