fix(bundle): only replace require shim in js files, spruce up output (#29892)

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)
This commit is contained in:
Nathan Whitaker 2025-06-26 11:35:19 -07:00 committed by GitHub
parent 0a4f946fd9
commit 8cefe97b09
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 168 additions and 53 deletions

View file

@ -4,12 +4,14 @@ mod esbuild;
mod externals; mod externals;
mod transform; mod transform;
use std::borrow::Cow;
use std::cell::RefCell; use std::cell::RefCell;
use std::path::Path; use std::path::Path;
use std::path::PathBuf; use std::path::PathBuf;
use std::rc::Rc; use std::rc::Rc;
use std::sync::Arc; use std::sync::Arc;
use std::sync::LazyLock; use std::sync::LazyLock;
use std::time::Duration;
use deno_ast::EmitOptions; use deno_ast::EmitOptions;
use deno_ast::MediaType; use deno_ast::MediaType;
@ -158,16 +160,11 @@ pub async fn bundle(
handle_esbuild_errors_and_warnings(&response, &init_cwd); handle_esbuild_errors_and_warnings(&response, &init_cwd);
if response.errors.is_empty() { 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() { if bundle_flags.output_dir.is_some() || bundle_flags.output_path.is_some() {
log::info!( print_finished_message(&metafile, &output_infos, start.elapsed())?;
"{}",
deno_terminal::colors::green(format!(
"bundled in {}",
crate::display::human_elapsed(start.elapsed().as_millis()),
))
);
} }
} }
@ -178,6 +175,16 @@ pub async fn bundle(
Ok(()) Ok(())
} }
fn metafile_from_response(
response: &BuildResponse,
) -> Result<esbuild_client::Metafile, AnyError> {
Ok(serde_json::from_str::<esbuild_client::Metafile>(
response.metafile.as_deref().ok_or_else(|| {
deno_core::anyhow::anyhow!("expected a metafile to be present")
})?,
)?)
}
async fn bundle_watch( async fn bundle_watch(
flags: Arc<Flags>, flags: Arc<Flags>,
bundler: EsbuildBundler, bundler: EsbuildBundler,
@ -217,14 +224,10 @@ async fn bundle_watch(
let response = bundler.rebuild().await?; let response = bundler.rebuild().await?;
handle_esbuild_errors_and_warnings(&response, &bundler.cwd); handle_esbuild_errors_and_warnings(&response, &bundler.cwd);
if response.errors.is_empty() { if response.errors.is_empty() {
process_result(&response, *DISABLE_HACK)?; let metafile = metafile_from_response(&response)?;
log::info!( let output_infos =
"{}", process_result(&response, &bundler.cwd, *DISABLE_HACK)?;
deno_terminal::colors::green(format!( print_finished_message(&metafile, &output_infos, start.elapsed())?;
"bundled in {}",
crate::display::human_elapsed(start.elapsed().as_millis()),
))
);
let new_watched = get_input_paths_for_watch(&response); let new_watched = get_input_paths_for_watch(&response);
*current_roots.borrow_mut() = new_watched.clone(); *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() { } else if let Some(output_path) = bundle_flags.output_path.clone() {
builder.outfile(output_path); builder.outfile(output_path);
} }
if bundle_flags.watch { builder.metafile(true);
builder.metafile(true);
}
match bundle_flags.platform { match bundle_flags.platform {
crate::args::BundlePlatform::Browser => { 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( fn process_result(
response: &BuildResponse, response: &BuildResponse,
// init_cwd: &Path, cwd: &Path,
should_replace_require_shim: bool, should_replace_require_shim: bool,
) -> Result<(), AnyError> { ) -> Result<Vec<OutputFileInfo>, AnyError> {
if let Some(output_files) = response.output_files.as_ref() { let mut exists_cache = std::collections::HashSet::new();
let mut exists_cache = std::collections::HashSet::new(); let output_files = response
for file in output_files.iter() { .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("<stdout>") {
let string = String::from_utf8(file.contents.clone())?; let string = String::from_utf8(file.contents.clone())?;
let string = if should_replace_require_shim { let string = if should_replace_require_shim {
replace_require_shim(&string) replace_require_shim(&string)
} else { } else {
string string
}; };
Cow::Owned(string.into_bytes())
} else {
Cow::Borrowed(&file.contents)
};
if file.path == "<stdout>" { if file.path.ends_with("<stdout>") {
crate::display::write_to_stdout_ignore_sigpipe(string.as_bytes())?; crate::display::write_to_stdout_ignore_sigpipe(bytes.as_slice())?;
continue; 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 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!(
"{:<longest$}",
info.relative_path.display()
))
} else {
deno_terminal::colors::magenta(format!(
"{:<longest$}",
info.relative_path.display()
))
},
deno_terminal::colors::gray(
crate::display::human_size(info.size as f64,)
)
));
}
output.push('\n');
log::info!("{}", output);
Ok(()) Ok(())
} }

View file

@ -2037,7 +2037,7 @@ async fn bundle_watch() {
.spawn() .spawn()
.unwrap(); .unwrap();
let (_, mut stderr_lines) = child_lines(&mut child); let (_, mut stderr_lines) = child_lines(&mut child);
wait_contains("bundled in", &mut stderr_lines).await; wait_contains("Bundled 1 module in", &mut stderr_lines).await;
let contents = t.path().join("output.js").read_to_string(); let contents = t.path().join("output.js").read_to_string();
assert_contains!(contents, "console.log(\"hello\");"); assert_contains!(contents, "console.log(\"hello\");");
@ -2049,7 +2049,7 @@ async fn bundle_watch() {
"#, "#,
); );
wait_contains("File change detected", &mut stderr_lines).await; wait_contains("File change detected", &mut stderr_lines).await;
wait_contains("bundled in", &mut stderr_lines).await; wait_contains("Bundled 1 module in", &mut stderr_lines).await;
let contents = t.path().join("output.js").read_to_string(); let contents = t.path().join("output.js").read_to_string();
assert_contains!(contents, "console.log(\"hello world\");"); assert_contains!(contents, "console.log(\"hello world\");");

View file

@ -5,7 +5,7 @@
"steps": [ "steps": [
{ {
"args": "bundle --output=out.js main.ts", "args": "bundle --output=out.js main.ts",
"output": "[WILDCARD]\nbundled in [WILDCARD]s\n" "output": "import_meta_main.out"
}, },
{ {
"args": "run out.js", "args": "run out.js",

View file

@ -0,0 +1,3 @@
[WILDCARD]
Bundled [WILDCARD] modules in [WILDCARD]s
out.js [WILDCARD]

View file

@ -13,7 +13,7 @@
}, },
{ {
"args": "bundle --output=out.js main.ts", "args": "bundle --output=out.js main.ts",
"output": "[WILDCARD]\nbundled in [WILDCARD]s\n" "output": "npm_specifier.out"
}, },
{ {
"args": "clean", "args": "clean",
@ -37,7 +37,7 @@
}, },
{ {
"args": "bundle --output=out.js main2.ts", "args": "bundle --output=out.js main2.ts",
"output": "[WILDCARD]\nbundled in [WILDCARD]s\n" "output": "npm_specifier_with_import_map.out"
}, },
{ {
"args": "clean", "args": "clean",
@ -57,7 +57,7 @@
}, },
{ {
"args": "bundle --output=out.js main_jsr.ts", "args": "bundle --output=out.js main_jsr.ts",
"output": "[WILDCARD]\nbundled in [WILDCARD]s\n" "output": "jsr_specifier.out"
}, },
{ {
"args": "clean", "args": "clean",
@ -73,7 +73,7 @@
"steps": [ "steps": [
{ {
"args": "bundle --output=out.js uses_node_builtin.cjs", "args": "bundle --output=out.js uses_node_builtin.cjs",
"output": "[WILDCARD]\nbundled in [WILDCARD]s\n" "output": "requires_node_builtin.out"
}, },
{ {
"args": "run --no-lock --cached-only --no-config -A out.js", "args": "run --no-lock --cached-only --no-config -A out.js",
@ -85,7 +85,7 @@
"steps": [ "steps": [
{ {
"args": "bundle --output=out.js imports_json.ts", "args": "bundle --output=out.js imports_json.ts",
"output": "[WILDCARD]\nbundled in [WILDCARD]s\n" "output": "json_import.out"
}, },
{ {
"args": ["eval", "console.log(Deno.readTextFileSync('./out.js'))"], "args": ["eval", "console.log(Deno.readTextFileSync('./out.js'))"],
@ -101,7 +101,7 @@
"steps": [ "steps": [
{ {
"args": "bundle --unstable-sloppy-imports --output=out.js sloppy.ts", "args": "bundle --unstable-sloppy-imports --output=out.js sloppy.ts",
"output": "[WILDCARD]\nbundled in [WILDCARD]s\n" "output": "sloppy_imports.out"
}, },
{ {
"args": "run --no-lock --cached-only --no-config -A out.js", "args": "run --no-lock --cached-only --no-config -A out.js",

View file

@ -0,0 +1,3 @@
[WILDCARD]
Bundled [WILDCARD] modules in [WILDCARD]s
out.js [WILDCARD]

View file

@ -0,0 +1,3 @@
[WILDCARD]
Bundled [WILDCARD] modules in [WILDCARD]s
out.js [WILDCARD]

View file

@ -0,0 +1,3 @@
[WILDCARD]
Bundled [WILDCARD] modules in [WILDCARD]s
out.js [WILDCARD]

View file

@ -0,0 +1,3 @@
[WILDCARD]
Bundled [WILDCARD] modules in [WILDCARD]s
out.js [WILDCARD]

View file

@ -0,0 +1,3 @@
[WILDCARD]
Bundled [WILDCARD] module in [WILDCARD]s
out.js [WILDCARD]

View file

@ -0,0 +1,3 @@
[WILDCARD]
Bundled [WILDCARD] modules in [WILDCARD]s
out.js [WILDCARD]

View file

@ -3,7 +3,7 @@
"steps": [ "steps": [
{ {
"args": "bundle --outdir=out src/foo/main.ts src/bar/main.ts", "args": "bundle --outdir=out src/foo/main.ts src/bar/main.ts",
"output": "[WILDCARD]\nbundled in [WILDCARD]s\n" "output": "multiple_entries.out"
}, },
{ {
"args": "run -A out/foo/main.js", "args": "run -A out/foo/main.js",

View file

@ -0,0 +1,4 @@
[WILDCARD]
Bundled [WILDCARD] modules in [WILDCARD]s
out[WILDCHAR]foo[WILDCHAR]main.js [WILDCARD]
out[WILDCHAR]bar[WILDCHAR]main.js [WILDCARD]

View file

@ -5,7 +5,7 @@
"steps": [ "steps": [
{ {
"args": "bundle --sourcemap=linked --output=out2.js main.ts", "args": "bundle --sourcemap=linked --output=out2.js main.ts",
"output": "[WILDCARD]\nbundled in [WILDCARD]s\n" "output": "sourcemap_linked.out"
}, },
{ {
"args": [ "args": [
@ -27,7 +27,7 @@
"steps": [ "steps": [
{ {
"args": "bundle --sourcemap=external --output=out3.js main.ts", "args": "bundle --sourcemap=external --output=out3.js main.ts",
"output": "[WILDCARD]\nbundled in [WILDCARD]s\n" "output": "sourcemap_external.out"
}, },
{ {
"args": [ "args": [
@ -49,7 +49,7 @@
"steps": [ "steps": [
{ {
"args": "bundle --sourcemap=inline --output=out4.js main.ts", "args": "bundle --sourcemap=inline --output=out4.js main.ts",
"output": "[WILDCARD]\nbundled in [WILDCARD]s\n" "output": "sourcemap_inline.out"
}, },
{ {
"args": [ "args": [

View file

@ -0,0 +1,4 @@
[WILDCARD]
Bundled [WILDCARD] module in [WILDCARD]s
out3.js.map [WILDCARD]
out3.js [WILDCARD]

View file

@ -0,0 +1,3 @@
[WILDCARD]
Bundled [WILDCARD] module in [WILDCARD]s
out4.js [WILDCARD]

View file

@ -0,0 +1,4 @@
[WILDCARD]
Bundled [WILDCARD] module in [WILDCARD]s
out2.js.map [WILDCARD]
out2.js [WILDCARD]