Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions packages/bun-types/bun.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3047,6 +3047,16 @@ declare module "bun" {
inline?: boolean;

/**
* The lowest level of diagnostic that the transpiler keeps.
*
* A warning never makes the constructor, `transform`, `transformSync`,
* `scan` or `scanImports` throw. Only an error does. When a call throws,
* the thrown value also holds the warnings at this level or above, so it
* can be an `AggregateError`. At `"error"` it holds errors only.
*
* When the option is not set, the level is the log level of the process:
* `logLevel` in bunfig.toml, or `"warn"`.
*
* @default "warn"
*/
logLevel?: "verbose" | "debug" | "info" | "warn" | "error";
Expand Down
9 changes: 9 additions & 0 deletions src/ast/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1616,6 +1616,15 @@ impl Log {
Log::init()
}

/// An empty log at the `level` of `self`, for messages that are appended to `self` later.
#[inline]
pub fn child(&self) -> Log {
Log {
level: self.level,
..Default::default()
}
}

#[inline]
pub fn add_debug_fmt(&mut self, source: Option<&Source>, l: Loc, args: fmt::Arguments<'_>) {
if !Kind::Debug.should_print(self.level) {
Expand Down
5 changes: 2 additions & 3 deletions src/bundler/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ impl JavaScript {
log: &mut bun_ast::Log,
source: &'a bun_ast::Source,
) -> Result<Option<js_parser::Result<'a>>, crate::Error> {
let mut temp_log = bun_ast::Log::init();
temp_log.level = log.level;
let mut temp_log = log.child();
let parser = match js_parser::Parser::init(opts, &mut temp_log, source, defines, bump) {
Ok(p) => p,
Err(_) => {
Expand Down Expand Up @@ -125,7 +124,7 @@ impl JavaScript {
return Ok(());
}

let mut temp_log = bun_ast::Log::init();
let mut temp_log = log.child();
// scopeguard cannot capture &mut temp_log while it's used below;
// explicit `append_to_maybe_recycled` calls at each exit.

Expand Down
17 changes: 17 additions & 0 deletions src/jsc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1365,6 +1365,12 @@ pub trait LogJsc {
global: &JSGlobalObject,
message: core::fmt::Arguments<'_>,
) -> JsResult<JSValue>;
/// The value to throw for this log: `to_js` if its `errors` counter is above zero, else `None`.
fn errors_to_js(
&self,
global: &JSGlobalObject,
message: core::fmt::Arguments<'_>,
) -> JsResult<Option<JSValue>>;
fn to_js_array(&self, global: &JSGlobalObject) -> JsResult<JSValue>;
/// Unlike `to_js`, always produces an `AggregateError`.
fn to_js_aggregate_error(
Expand Down Expand Up @@ -1408,6 +1414,17 @@ impl LogJsc for bun_ast::Log {
}
}
}
#[inline]
fn errors_to_js(
&self,
global: &JSGlobalObject,
message: core::fmt::Arguments<'_>,
) -> JsResult<Option<JSValue>> {
if !self.has_errors() {
return Ok(None);
}
self.to_js(global, message).map(Some)
}
fn to_js_array(&self, global: &JSGlobalObject) -> JsResult<JSValue> {
JSValue::create_array_from_iter(global, self.msgs.iter(), |msg| msg_to_js(msg, global))
}
Expand Down
11 changes: 7 additions & 4 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1116,13 +1116,16 @@ impl WebWorker {
return;
}
let global = vm.global();
let result: jsc::JsResult<(JSValue, BunString)> = (|| {
let err = vm_log.to_js(global, format_args!("Error in worker"))?;
let result: jsc::JsResult<Option<(JSValue, BunString)>> = (|| {
let Some(err) = vm_log.errors_to_js(global, format_args!("Error in worker"))? else {
return Ok(None);
};
let str = err.to_bun_string(global)?;
Ok((err, str))
Ok(Some((err, str)))
})();
let (err, str) = match result {
Ok(pair) => pair,
Ok(Some(pair)) => pair,
Ok(None) => return,
Err(JsError::OutOfMemory) => bun_core::out_of_memory(),
Err(err) => {
// The worker's start sequence is its outermost frame: building the error from the
Expand Down
4 changes: 2 additions & 2 deletions src/resolver/tsconfig_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl JsonCache {
&bun_alloc::Arena,
) -> Result<bun_ast::Expr, bun_parsers::Error>,
) -> Result<Option<bun_ast::Expr>, crate::Error> {
let mut temp_log = bun_ast::Log::init();
let mut temp_log = log.child();
Comment thread
robobun marked this conversation as resolved.
let bump = self.bump.get_or_insert_with(bun_alloc::Arena::new);
let result = func(source, &mut temp_log, bump).ok();
let _ = temp_log.append_to_maybe_recycled(log, source);
Expand All @@ -65,7 +65,7 @@ impl JsonCache {
&mut bun_ast::Log,
) -> Result<json_parser::ParsedJson, bun_parsers::Error>,
) -> Result<Option<json_parser::ParsedJson>, crate::Error> {
let mut temp_log = bun_ast::Log::init();
let mut temp_log = log.child();
let result = func(source, &mut temp_log).ok();
let _ = temp_log.append_to_maybe_recycled(log, source);
Ok(result)
Expand Down
166 changes: 73 additions & 93 deletions src/runtime/api/JSTranspiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,17 +280,6 @@ impl Config {
break 'tsconfig;
}
self.tsconfig_buf = out.to_owned_slice().into();

// TODO: JSC -> Ast conversion
// SAFETY: VirtualMachine::get() returns the live singleton on the JS thread.
let vm = VirtualMachine::get().as_mut();
if let Ok(Some(parsed_tsconfig)) = TSConfigJSON::parse(
&mut self.log,
&bun_ast::Source::init_path_string(b"tsconfig.json", &self.tsconfig_buf[..]),
&mut vm.transpiler.resolver.caches.json,
) {
self.tsconfig = Some(parsed_tsconfig);
}
}
}

Expand Down Expand Up @@ -325,18 +314,6 @@ impl Config {
break 'macros;
}
self.macros_buf = out.to_owned_slice().into();
let source =
bun_ast::Source::init_path_string(b"macros.json", &self.macros_buf[..]);
// SAFETY: VirtualMachine::get() returns the live singleton on the JS thread.
let vm = VirtualMachine::get().as_mut();
let Ok(Some(json)) = vm.transpiler.resolver.caches.json.parse_json(
&mut self.log,
&source,
bun_resolver::tsconfig_json::JsonMode::Json,
) else {
break 'macros;
};
self.macro_map = PackageJSON::parse_macros_json(json, &mut self.log, &source);
}
}

Expand Down Expand Up @@ -582,6 +559,31 @@ impl Config {
}
}

// `tsconfig` and `macro` are parsed after `logLevel` so that it covers their diagnostics.
if !self.tsconfig_buf.is_empty() {
// SAFETY: VirtualMachine::get() returns the live singleton on the JS thread.
let vm = VirtualMachine::get().as_mut();
if let Ok(Some(parsed_tsconfig)) = TSConfigJSON::parse(
&mut self.log,
&bun_ast::Source::init_path_string(b"tsconfig.json", &self.tsconfig_buf[..]),
&mut vm.transpiler.resolver.caches.json,
) {
self.tsconfig = Some(parsed_tsconfig);
}
}
if !self.macros_buf.is_empty() {
let source = bun_ast::Source::init_path_string(b"macros.json", &self.macros_buf[..]);
// SAFETY: VirtualMachine::get() returns the live singleton on the JS thread.
let vm = VirtualMachine::get().as_mut();
if let Ok(Some(json)) = vm.transpiler.resolver.caches.json.parse_json(
&mut self.log,
&source,
bun_resolver::tsconfig_json::JsonMode::Json,
) {
self.macro_map = PackageJSON::parse_macros_json(json, &mut self.log, &source);
}
}

self.tree_shaking = tree_shaking.unwrap_or(false);
self.trim_unused_imports = Some(trim_unused_imports.unwrap_or(self.tree_shaking));
Ok(())
Expand Down Expand Up @@ -642,8 +644,7 @@ impl TransformTask {
loader: Loader,
) -> JSValue {
let config = transpiler.config.get();
let mut log = bun_ast::Log::init();
log.level = config.log.level;
let log = config.log.child();

// SAFETY: `ManuallyDrop` keeps this bitwise copy from freeing what the
// wrapper owns; `run` re-aims its log and arena pointers.
Expand Down Expand Up @@ -801,31 +802,27 @@ impl TransformTask {
fn then(&mut self, promise: &mut JSPromise, global: &JSGlobalObject) -> JsResult<()> {
// The job drops this `TransformTask` (running its `Drop`: transpiler
// deref etc.) right after `then` returns.
if self.log.has_any() || self.err.is_some() {
let error_value: JsResult<JSValue> = 'brk: {
if let Some(err) = &self.err {
if !self.log.has_any() {
break 'brk bun_jsc::BuildMessage::create(
global,
bun_ast::Msg {
data: bun_ast::Data {
text: err.name().as_bytes().to_vec().into(),
..Default::default()
},
..Default::default()
},
);
}
}

break 'brk self.log.to_js(global, format_args!("Transform failed"));
};

promise.reject_with_async_stack(global, error_value)?;
return Ok(());
}
let log_error = self
.log
.errors_to_js(global, format_args!("Transform failed"))
.transpose();
let error_value: JsResult<JSValue> = match (log_error, &self.err) {
(Some(value), _) => value,
(None, Some(err)) => bun_jsc::BuildMessage::create(
global,
bun_ast::Msg {
data: bun_ast::Data {
text: err.name().as_bytes().to_vec().into(),
..Default::default()
},
..Default::default()
},
),
(None, None) => return self.finish(promise, global),
};

self.finish(promise, global)
promise.reject_with_async_stack(global, error_value)?;
Ok(())
}

fn finish(&mut self, promise: &mut JSPromise, global: &JSGlobalObject) -> JsResult<()> {
Expand Down Expand Up @@ -923,12 +920,11 @@ impl JSTranspiler {

config.from_js(global, config_arg, arena_ref)?;

if (config.log.warnings + config.log.errors) > 0 {
return Err(global.throw_value(
config
.log
.to_js(global, format_args!("Failed to create transpiler"))?,
));
if let Some(value) = config
.log
.errors_to_js(global, format_args!("Failed to create transpiler"))?
{
return Err(global.throw_value(value));
}
Comment on lines +923 to 928

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) Users who pass a tsconfig or macro option that is only partly valid now get a Transpiler that silently ignores that part, with no signal anywhere. JSTranspiler.rs:963-968 continues when config.log holds only warnings, and nothing later prints or exposes them. So jsxFactory: "1abc", an invalid paths pattern or a non-package macro key are dropped together with their warning; the base threw it. Fix: give a surviving warning an outlet at the configured level, for the constructor and the four methods alike, e.g. print config.log warnings to stderr as bun build does or return them. The PR calls this an accepted downside; sized, it turns a loud misconfiguration into silent wrong output.

Why this was flagged

A caller constructs new Bun.Transpiler({ tsconfig: { compilerOptions: { jsxFactory: "1abc" } } }) or { macro: { "./rel": { x: "y" } } }. In Config::from_js the parse at JSTranspiler.rs:604-610 / 616-622 logs only a warning and the option is left unapplied (factory stays the default, the remap is skipped). The gate at JSTranspiler.rs:963-968 calls errors_to_js, which returns None because errors == 0 (src/jsc/lib.rs:1416), so construction continues. The warnings sit in config.log until Drop runs clear_and_free at JSTranspiler.rs:1074; no code path prints them and no API returns them. The new test at test/bundler/transpiler/transpiler.test.js:2884-2916 asserts stderr is "" for all twelve warning-only configs. On the base branch the same constructor threw Failed to create transpiler carrying the warning, so the user learned the option was ignored. After this change Bun.Transpiler is the only transpile API where a warning has no outlet at any logLevel.

Verification: Triggering condition: any new Bun.Transpiler({...}) whose tsconfig or macro option logs only a warning. The constructor gate at JSTranspiler.rs:963-968 calls errors_to_js, which returns Ok(None) when !self.has_errors(), so construction proceeds and nothing reads, prints or exposes config.log afterwards. On the base, these inputs threw a BuildMessage carrying the warning.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is decision 2 in the Notes and it is open for a maintainer. An earlier version of this change printed the warnings of the tsconfig and macro options to stderr at construction (one write, none at logLevel: "error"). I removed it for three reasons. No other JS API of bun prints a log. It would be the only stderr promise in bun-types. A later change that returns warnings as data would have to keep the print or break it. If a maintainer wants an outlet now, the print is about ten lines in the constructor. I leave this thread open for that decision.


// SAFETY: VirtualMachine::get() returns the live singleton on the JS thread.
Expand All @@ -941,11 +937,11 @@ impl JSTranspiler {
) {
Ok(t) => t,
Err(err) => {
let log = &mut config.log;
if (log.warnings + log.errors) > 0 {
return Err(global.throw_value(
log.to_js(global, format_args!("Failed to create transpiler"))?,
));
if let Some(value) = config
.log
.errors_to_js(global, format_args!("Failed to create transpiler"))?
{
return Err(global.throw_value(value));
}
return Err(global.throw_error(err, "Error creating transpiler"));
}
Expand Down Expand Up @@ -979,11 +975,11 @@ impl JSTranspiler {
transpiler.configure_linker_with_auto_jsx(false);
transpiler.options.env.behavior = options::EnvBehavior::disable;
if let Err(err) = transpiler.configure_defines() {
let log = &mut config.log;
if (log.warnings + log.errors) > 0 {
return Err(
global.throw_value(log.to_js(global, format_args!("Failed to load define"))?)
);
if let Some(value) = config
.log
.errors_to_js(global, format_args!("Failed to load define"))?
{
return Err(global.throw_value(value));
}
return Err(global.throw_error(err, "Failed to load define"));
}
Expand Down Expand Up @@ -1237,7 +1233,7 @@ impl JSTranspiler {
let loader: Option<Loader> = loader_from_js(global, loader_arg.to_js())?;

let arena = Arena::new();
let mut log = bun_ast::Log::init();
let mut log = self.config.get().log.child();
// SAFETY: `arena` outlives every use through `self.transpiler` in this fn body;
// `_restore` (declared after `arena`/`log`, so dropped first) restores
// `prev_arena` and `&self.config.log` before either local drops.
Expand All @@ -1261,17 +1257,13 @@ impl JSTranspiler {

let parse_result = self.get_parse_result(arena_ref, code, loader, MacroJSCtx::ZERO);
let log_ref = self.transpiler.get().log_mut();
if let Some(value) = log_ref.errors_to_js(global, format_args!("Parse error"))? {
return Err(global.throw_value(value));
}
let Some(mut parse_result) = parse_result else {
if (log_ref.warnings + log_ref.errors) > 0 {
return Err(global.throw_value(log_ref.to_js(global, format_args!("Parse error"))?));
}
return Err(global.throw(format_args!("Failed to parse")));
};

if (log_ref.warnings + log_ref.errors) > 0 {
return Err(global.throw_value(log_ref.to_js(global, format_args!("Parse error"))?));
}

let exports_label = EncodedSlice::latin1(b"exports");
let imports_label = EncodedSlice::latin1(b"imports");
let named_imports_value = named_imports_to_js(
Expand Down Expand Up @@ -1380,8 +1372,7 @@ impl JSTranspiler {
// (`prev_bundler = this.transpiler`) and restores it on exit. `Transpiler` is not
// bitwise-copyable in Rust, so explicitly snapshot the fields the body mutates
// (`allocator`, `log`, `macro_context`) and restore them via RAII guard.
let mut log = bun_ast::Log::init();
log.level = self.config.get().log.level;
let mut log = self.config.get().log.child();
// SAFETY: `arena` outlives every use through `self.transpiler` in this fn body;
// `_restore` (declared after `arena`/`log`, so dropped first) restores
// `prev_arena`, `&self.config.log`, and `prev_macro_context` before either drops.
Expand All @@ -1406,17 +1397,13 @@ impl JSTranspiler {
let macro_js_ctx: MacroJSCtx = MacroJSCtx(js_ctx_value.0 as i64);
let parse_result = self.get_parse_result(arena_ref, code, loader, macro_js_ctx);
let log_ref = self.transpiler.get().log_mut();
if let Some(value) = log_ref.errors_to_js(global, format_args!("Parse error"))? {
return Err(global.throw_value(value));
}
let Some(parse_result) = parse_result else {
if (log_ref.warnings + log_ref.errors) > 0 {
return Err(global.throw_value(log_ref.to_js(global, format_args!("Parse error"))?));
}
return Err(global.throw(format_args!("Failed to parse code")));
};

if (log_ref.warnings + log_ref.errors) > 0 {
return Err(global.throw_value(log_ref.to_js(global, format_args!("Parse error"))?));
}

let mut buffer_writer = self.buffer_writer.replace(None).unwrap_or_else(|| {
let mut writer = JSPrinter::BufferWriter::init();
bun_core::handle_oom(writer.buffer.grow_if_needed(code.len()));
Expand Down Expand Up @@ -1531,7 +1518,7 @@ impl JSTranspiler {
}

let arena = Arena::new();
let mut log = bun_ast::Log::init();
let mut log = self.config.get().log.child();
// SAFETY: `arena` outlives every use through `self.transpiler` in this fn body;
// `_restore` (declared after `arena`/`log`, so dropped first) restores
// `prev_arena` and `&self.config.log` before either local drops.
Expand Down Expand Up @@ -1594,20 +1581,13 @@ impl JSTranspiler {
// (including the error paths). Compute the result, then reset
// unconditionally before returning.
let result = (|| -> JsResult<JSValue> {
if let Some(value) = log.errors_to_js(global, format_args!("Failed to scan imports"))? {
return Err(global.throw_value(value));
}
if let Err(err) = scan_result {
if (log.warnings + log.errors) > 0 {
return Err(global
.throw_value(log.to_js(global, format_args!("Failed to scan imports"))?));
}
return Err(global.throw_error(err, "Failed to scan imports"));
}

if (log.warnings + log.errors) > 0 {
return Err(
global.throw_value(log.to_js(global, format_args!("Failed to scan imports"))?)
);
}

named_imports_to_js(
global,
self.scan_pass_result.get().import_records.as_slice(),
Expand Down
Loading
Loading