Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 36 additions & 12 deletions src/bundler/bundle_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2099,6 +2099,19 @@ pub mod bv2_impl {
);
}

/// Callers require an entry point, so none after parsing means one was dropped without an error.
fn fail_if_no_entry_points(&self) -> Result<(), Error> {
if !self.graph.entry_points.is_empty() {
return Ok(());
}
self.transpiler.log_mut().add_error(
None,
bun_ast::Loc::EMPTY,
"None of the entry points could be bundled",
);
Err(crate::Error::BuildFailed)
}

/// `BUN_THREADPOOL_STATS=1` instrumentation hook — dump aggregate worker
/// idle/busy time since the previous call. No-op when env var unset.
#[inline]
Expand Down Expand Up @@ -2629,10 +2642,9 @@ pub mod bv2_impl {
let result = &mut *resolve;
// borrowck: clone the active path out so we don't hold a `&mut`
// into `result` across the `&mut self` calls below.
let mut path: Fs::Path<'static> = match result.path() {
Some(p) => *p,
None => return Ok(None),
};
let mut path: Fs::Path<'static> = *result
.path()
.expect("resolve_entry_point rejects disabled results and FileMap results have a path");

path.assert_file_path_is_absolute();
// borrowck: get-then-put instead of a single get-or-put.
Expand Down Expand Up @@ -3843,10 +3855,7 @@ pub mod bv2_impl {
// sidestep for the `&mut self` overlap.
this.enqueue_entry_points_normal(unsafe { &*entry_points })?;

if this.transpiler.log().has_errors() {
return Err(crate::Error::BuildFailed);
}

// Like `run_from_js_in_new_thread`: drain the pool, then report entry point errors.
this.wait_for_parse();
this.dump_pool_stats("parse");

Expand All @@ -3858,6 +3867,7 @@ pub mod bv2_impl {
if this.transpiler.log().has_errors() {
return Err(crate::Error::BuildFailed);
}
this.fail_if_no_entry_points()?;

this.scan_for_secondary_paths();

Expand Down Expand Up @@ -4017,10 +4027,7 @@ pub mod bv2_impl {

this.enqueue_entry_points_bake_production(entry_points)?;

if this.transpiler.log().has_errors() {
return Err(crate::Error::BuildFailed);
}

// Drain the pool, then report entry point errors (as `generate_from_cli` does).
Comment thread
coderabbitai[bot] marked this conversation as resolved.
this.wait_for_parse();

if this.transpiler.log().has_errors() {
Expand Down Expand Up @@ -4792,6 +4799,22 @@ pub mod bv2_impl {
drop(result.path);
}
} else {
// An external import is left as is in the importer; an external
// entry point has nothing to emit, so it is a build error (as in esbuild).
Comment thread
robobun marked this conversation as resolved.
Outdated
if resolve.import_record.kind == ImportKind::EntryPointBuild {
let log = this.log_for_resolution_failures(
&resolve.import_record.source_file,
resolve.import_record.original_target.bake_graph(),
);
log.add_error_fmt(
None,
bun_ast::Loc::EMPTY,
format_args!(
"The entry point {} cannot be marked as external",
bun_core::fmt::quote(&resolve.import_record.specifier),
),
);
}
drop(result.namespace);
drop(result.path);
}
Expand Down Expand Up @@ -4986,6 +5009,7 @@ pub mod bv2_impl {
if self.transpiler.log().errors > 0 {
return Err(crate::Error::BuildFailed);
}
self.fail_if_no_entry_points()?;

self.scan_for_secondary_paths();

Expand Down
53 changes: 43 additions & 10 deletions src/bundler/transpiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,15 +458,7 @@ impl<'a> Transpiler<'a> {
/// retrying once on failure before reporting the error to the log.
pub fn resolve_entry_point(&mut self, entry_point: &[u8]) -> crate::Result<resolver::Result> {
match self._resolve_entry_point(entry_point) {
Ok(r) => Ok(r),
// Nothing that long names a directory whose cache could be stale
// (and the join below has a PathBuffer to fit `top_level_dir/entry/..` in).
Err(err)
if self.fs().top_level_dir.len() + entry_point.len() + 4
> bun_paths::MAX_PATH_BYTES =>
{
Err(err)
}
Ok(r) => self.reject_disabled_entry_point(r, entry_point),
Err(err) => {
let mut cache_bust_buf = bun_paths::PathBuffer::uninit();

Expand All @@ -477,6 +469,14 @@ impl<'a> Transpiler<'a> {
// disjoint mutable borrows of `cache_bust_buf` across `break`,
// so compute `busted` directly instead.
let busted: bool = 'name: {
// Nothing that long names a directory whose cache could be
// stale (and neither buster name below would fit
// `cache_bust_buf`).
Comment thread
robobun marked this conversation as resolved.
Outdated
if self.fs().top_level_dir.len() + entry_point.len() + 4
> bun_paths::MAX_PATH_BYTES
{
break 'name false;
}
if bun_paths::is_absolute(entry_point) {
let dir = bun_paths::resolve_path::dirname::<bun_paths::platform::Auto>(
entry_point,
Expand Down Expand Up @@ -515,7 +515,7 @@ impl<'a> Transpiler<'a> {
// Only re-query if we previously had something cached.
if busted {
if let Ok(result) = self._resolve_entry_point(entry_point) {
return Ok(result);
return self.reject_disabled_entry_point(result, entry_point);
}
// ignore this error, we will print the original error
}
Expand All @@ -534,6 +534,39 @@ impl<'a> Transpiler<'a> {
}
}

/// A disabled module (no usable path) imports as `{}`, but an entry point has nothing to emit.
fn reject_disabled_entry_point(
&self,
resolved: resolver::Result,
entry_point: &[u8],
) -> crate::Result<resolver::Result> {
if resolved.path_const().is_some() {
return Ok(resolved);
}
Comment thread
robobun marked this conversation as resolved.

// Stubbed builtins carry the "node" namespace; anything else came from a "browser" map.
if resolved.path_pair.primary.namespace == b"node" {
self.log_mut().add_error_fmt(
None,
bun_ast::Loc::EMPTY,
format_args!(
"Cannot use Node.js builtin \"{}\" as an entry point",
bstr::BStr::new(entry_point)
),
);
} else {
self.log_mut().add_error_fmt(
None,
bun_ast::Loc::EMPTY,
format_args!(
"\"{}\" is disabled due to \"browser\" field in package.json (entry point)",
bstr::BStr::new(entry_point)
),
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Err(crate::Error::ResolveMessage)
}

/// Load env files and build `options.define`. Idempotent — a no-op once
/// `options.defines_loaded` is set.
pub fn configure_defines(&mut self) -> crate::Result<()> {
Expand Down
13 changes: 6 additions & 7 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1418,11 +1418,10 @@ unsafe fn resolve_entry_point_specifier<'s>(
// `Path::text` borrows the resolver's process-lifetime `dirname_store` /
// `filename_store` (`Path<'static>`), NOT `resolved_entry_point` itself —
// copy the slice out and let `resolved_entry_point` drop on the stack.
match resolved_entry_point.path_const() {
Some(entry_path) => Some(entry_path.text),
None => {
*error_message = BunString::static_(b"Worker entry point is missing");
None
}
}
Some(
resolved_entry_point
.path_const()
.expect("resolve_entry_point rejects disabled results")
.text,
)
}
88 changes: 88 additions & 0 deletions test/bundler/bun-build-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,94 @@ describe("Bun.build", () => {
}
});

// Runs in a child because the unfixed behavior was a process abort: the
// disabled entry point was dropped without an error and the linker ran with
// zero entry points.
test.concurrent("an entry point disabled by the package.json browser field is a build error", async () => {
using dir = tempDir("build-entry-point-disabled-by-browser-field", {
"package.json": JSON.stringify({ name: "app", browser: { "./entry.js": false } }),
"entry.js": `console.log("entry");`,
"build.mjs": `
const returned = await Bun.build({ entrypoints: ["./entry.js"], target: "browser", throw: false });
let thrown;
try {
await Bun.build({ entrypoints: ["./entry.js"], target: "browser" });
} catch (e) {
thrown = {
isAggregateError: e instanceof AggregateError,
errors: e.errors.map(error => ({ name: error.name, level: error.level, position: error.position, message: error.message })),
};
}
console.log(JSON.stringify({
returned: {
success: returned.success,
outputs: returned.outputs.length,
logs: returned.logs.map(log => ({ name: log.name, level: log.level, position: log.position, message: log.message })),
},
thrown,
}));
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "build.mjs"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

const message = {
name: "BuildMessage",
level: "error",
position: null,
message: '"./entry.js" is disabled due to "browser" field in package.json (entry point)',
};
expect(stderr).toBe("");
expect(JSON.parse(stdout)).toEqual({
returned: { success: false, outputs: 0, logs: [message] },
thrown: { isAggregateError: true, errors: [message] },
});
expect(exitCode).toBe(0);
});

test.concurrent("an entry point too long for a path buffer is reported like any other missing one", async () => {
// Resolving it failed without logging anything, so the build went on
// with the entry point silently dropped: a successful build when another
// entry point was given, a crash in the linker when it was the only one.
// Runs in a child so the crash shows up as a failed assertion.
using dir = tempDir("build-api-long-entrypoint", { "valid.js": "console.log(1);" });
const fixture = /* ts */ `
// Longer than the path buffer on every platform, Windows included.
const long = Buffer.alloc(100_000, "a").toString();
const report = async (entrypoints: string[]) => {
const { success, outputs, logs } = await Bun.build({ entrypoints, throw: false });
return { success, outputs: outputs.length, logs: logs.map(log => [log.name, log.message]) };
};
console.log(JSON.stringify({
alone: await report([long]),
withValidEntryPoint: await report(["./valid.js", long]),
}));
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", fixture],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
const notFound = {
success: false,
outputs: 0,
logs: [["BuildMessage", `ModuleNotFound resolving "${Buffer.alloc(100_000, "a").toString()}" (entry point)`]],
};
expect(JSON.parse(stdout)).toEqual({ alone: notFound, withValidEntryPoint: notFound });
expect(exitCode).toBe(0);
});

test("returns output files", async () => {
Bun.gc(true);
const build = await Bun.build({
Expand Down
81 changes: 81 additions & 0 deletions test/bundler/bundler_browser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,87 @@ describe("bundler", () => {
},
});

// An entry point the "browser" field maps to false has nothing to bundle.
// This used to reach the linker with zero entry points and crash
// ("index out of bounds" in generateChunksInParallel); with a second, live
// entry point it silently built only that one.
const browserFieldDisabledEntryPointFiles = {
"/package.json": /* json */ `
{ "name": "app", "browser": { "./entry.js": false } }
`,
"/entry.js": /* js */ `
console.log("entry");
`,
"/other.js": /* js */ `
console.log("other");
`,
};
itBundled("browser/EntryPointDisabledByBrowserField", {
skipOnEsbuild: true,
backend: "cli",
files: browserFieldDisabledEntryPointFiles,
entryPointsRaw: ["./entry.js"],
target: "browser",
bundleErrors: {
"<bun>": ['"./entry.js" is disabled due to "browser" field in package.json (entry point)'],
},
});
itBundled("browser/EntryPointDisabledByBrowserFieldNextToLiveEntryPoint", {
skipOnEsbuild: true,
backend: "cli",
files: browserFieldDisabledEntryPointFiles,
entryPointsRaw: ["./entry.js", "./other.js"],
target: "browser",
bundleErrors: {
"<bun>": ['"./entry.js" is disabled due to "browser" field in package.json (entry point)'],
},
});
itBundled("browser/EntryPointDisabledByBrowserFieldOnlyAppliesToBrowserTarget", {
skipOnEsbuild: true,
backend: "cli",
files: browserFieldDisabledEntryPointFiles,
entryPointsRaw: ["./entry.js"],
target: "bun",
run: {
file: "/out/entry.js",
stdout: "entry",
},
});
itBundled("browser/EntryPointDisabledByPackageMainBrowserField", {
// The disabled module is reached through a package's "main", so the entry
// point specifier and the disabled file differ.
skipOnEsbuild: true,
backend: "cli",
files: {
"/node_modules/pkg/package.json": /* json */ `
{ "name": "pkg", "main": "./node.js", "browser": { "./node.js": false } }
`,
"/node_modules/pkg/node.js": /* js */ `
console.log("node only");
`,
},
entryPointsRaw: ["pkg"],
target: "browser",
bundleErrors: {
"<bun>": ['"pkg" is disabled due to "browser" field in package.json (entry point)'],
},
});
itBundled("browser/EntryPointIsNodeBuiltinStubbedForBrowser", {
// Browser builds replace "fs" (and node:* builtins without a polyfill) with
// an empty module, so as entry points they have nothing to bundle either.
skipOnEsbuild: true,
backend: "cli",
files: {},
entryPointsRaw: ["fs", "node:fs"],
target: "browser",
bundleErrors: {
"<bun>": [
`Cannot use Node.js builtin "fs" as an entry point`,
`Cannot use Node.js builtin "node:fs" as an entry point`,
],
},
});

// unsure: do we want polyfills or no-op stuff like node:* has
// right now all error except bun:wrap which errors at resolve time, but is included if external
const bunModules: Record<string, "no-op" | "polyfill" | "error"> = {
Expand Down
Loading
Loading