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
85 changes: 79 additions & 6 deletions src/jsc/ConsoleObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2951,15 +2951,29 @@ pub mod formatter {
}
}

/// The opening bracket of an array while its named properties print. `Pending`: not written.
#[derive(Clone, Copy, PartialEq, Eq)]
pub(crate) enum ArrayOpen {
/// An object, or an array whose first element wrote the bracket.
No,
Pending,
Inline,
Multiline,
}

pub(crate) struct PropertyIteratorCtx<'a, 'b, const C: bool> {
pub(crate) formatter: &'a mut Formatter<'b>,
pub(crate) writer: &'a mut dyn bun_io::Write,
pub(crate) i: usize,
pub(crate) single_line: bool,
pub(crate) always_newline: bool,
pub(crate) array_open: ArrayOpen,
pub(crate) parent: JSValue,
}

// Lives in the frame of every nested `print_object` and `print_array`.
const _: () = assert!(core::mem::size_of::<PropertyIteratorCtx<'_, '_, true>>() == 48);

impl<'a, 'b, const C: bool> PropertyIteratorCtx<'a, 'b, C> {
pub(crate) fn handle_first_property(
&mut self,
Expand Down Expand Up @@ -2993,6 +3007,34 @@ pub mod formatter {
Ok(())
}

/// Writes the bracket that `print_array` held back for an array with no elements.
#[cold]
#[inline(never)]
fn open_array(&mut self) {
let indent = self.formatter.indent;
let mut writer = WrappedWriter {
ctx: &mut *self.writer,
failed: false,
estimated_line_length: &mut self.formatter.estimated_line_length,
};
let was_good_time =
self.formatter.always_newline_scope || writer.good_time_for_a_new_line(indent);
if !self.single_line && (self.formatter.ordered_properties || was_good_time) {
writer.reset_line(indent);
writer.write_all(b"[\n");
writer.write_indent(indent);
writer.add_for_new_line(1);
self.array_open = ArrayOpen::Multiline;
} else {
writer.write_all(b"[ ");
writer.add_for_new_line(2);
self.array_open = ArrayOpen::Inline;
}
if writer.failed {
self.formatter.failed = true;
}
}

/// Prints `key:` (with quoting/symbol decoration) and the optional
/// string-value colour prefix. Hoisted out of `for_each` so its
/// `format_args!` temporaries are popped before the recursive
Expand Down Expand Up @@ -3119,7 +3161,9 @@ pub mod formatter {
}

if ctx.i == 0 {
if ctx.handle_first_property(global_this, ctx.parent).is_err() {
if ctx.array_open == ArrayOpen::Pending {
ctx.open_array();
} else if ctx.handle_first_property(global_this, ctx.parent).is_err() {
return None;
}
}
Expand Down Expand Up @@ -4285,8 +4329,12 @@ pub mod formatter {
value: JSValue,
js_type: jsc::JSType,
) -> JsResult<()> {
let len = value.get_length(self.global_this)?;
if len != 0 && self.depth > self.max_depth {
// `named`: no elements, and an enumerable named property to print.
let (len, named) = match value.array_length_and_named_keys() {
Some((len, named)) => (u64::from(len), named),
None => (value.get_length(self.global_this)?, false),
};
if (len != 0 || named) && self.depth > self.max_depth {
Comment thread
robobun marked this conversation as resolved.
Resolved
return self.print_depth_exceeded_marker::<C>(writer_, "Array");
}
// Cache once: `disable_inspect_custom` does not change inside this
Expand All @@ -4307,7 +4355,7 @@ pub mod formatter {
// TODO: DerivedArray does not get passed along in JSType, and it's
// not clear why.

if len == 0 {
if len == 0 && !named {
writer.write_all(b"[]");
writer.add_for_new_line(2);
return Ok(());
Expand All @@ -4329,6 +4377,10 @@ pub mod formatter {
let _qs = defer_restore!(self.quote_strings, prev_quote_strings);
let mut empty_start: Option<u32> = None;
'first: {
if len == 0 {
// `open_array` writes the bracket with the first property.
break 'first;
}
let element = value.get_direct_index(self.global_this, 0)?;

let tag = Tag::get_advanced(element, self.global_this, tag_opts)?;
Expand Down Expand Up @@ -4365,7 +4417,7 @@ pub mod formatter {
}
}

let mut i: u32 = 1;
let mut i = u32::from(len != 0);
let mut nonempty_count: u32 = 1;

while (i as u64) < len {
Expand Down Expand Up @@ -4512,7 +4564,9 @@ pub mod formatter {
// Hoist field reads before `formatter: self` reborrows the
// whole `*self` (struct-literal field order is not eval
// order in the borrow checker's eyes once `self` is moved).
let always_newline = !self.single_line
// `open_array` reads the line length of an empty array: do not reset it.
let always_newline = len != 0
&& !self.single_line
&& (self.always_newline_scope || self.good_time_for_a_new_line());
let single_line = self.single_line;
let global_this = self.global_this;
Expand All @@ -4521,6 +4575,11 @@ pub mod formatter {
writer: writer_,
always_newline,
single_line,
array_open: if len == 0 {
ArrayOpen::Pending
} else {
ArrayOpen::No
},
parent: value,
i: i as usize,
};
Expand All @@ -4529,6 +4588,7 @@ pub mod formatter {
(&raw mut iter).cast::<c_void>(),
PropertyIteratorCtx::<C>::for_each,
)?;
let array_open = iter.array_open;
if self.failed {
return Ok(());
}
Expand All @@ -4537,6 +4597,18 @@ pub mod formatter {
failed: false,
estimated_line_length: &mut self.estimated_line_length,
};
match array_open {
// No property printed.
ArrayOpen::Pending => {
writer.write_all(b"[]");
if writer.failed {
self.failed = true;
}
return Ok(());
}
ArrayOpen::Multiline => was_good_time = true,
ArrayOpen::Inline | ArrayOpen::No => {}
}
}
}

Expand Down Expand Up @@ -5432,6 +5504,7 @@ pub mod formatter {
writer: writer_,
always_newline,
single_line,
array_open: ArrayOpen::No,
parent: value,
i: 0,
};
Expand Down
10 changes: 10 additions & 0 deletions src/jsc/JSValue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2575,6 +2575,16 @@ impl JSValue {
index => Some(index as u32),
}
}
/// For an `Array`: its length, and whether it is empty and has an enumerable named property.
pub(crate) fn array_length_and_named_keys(self) -> Option<(u32, bool)> {
unsafe extern "C" {
safe fn Bun__JSArray__lengthAndNamedKeys(this: JSValue) -> u64;
}
match Bun__JSArray__lengthAndNamedKeys(self) {
u64::MAX => None,
bits => Some((bits as u32, bits >> 32 != 0)),
}
}
/// `JSValue.getNameProperty` — the value's `.name` (function/class
/// name). Empty for empty/`undefined`/`null`.
pub fn get_name_property(self, global: &JSGlobalObject) -> JsResult<bun_core::String> {
Expand Down
28 changes: 28 additions & 0 deletions src/jsc/bindings/UtilInspect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,34 @@ extern "C" JSC::EncodedJSValue JSC__JSValue__callCustomInspectFunction(
RELEASE_AND_RETURN(scope, JSValue::encode(inspectRet));
}

// The length, plus bit 32 when the array is empty and has an enumerable named key. UINT64_MAX: not an ArrayType cell.
extern "C" uint64_t Bun__JSArray__lengthAndNamedKeys(JSC::EncodedJSValue encodedValue)
{
static constexpr uint64_t namedKeys = 1ULL << 32;

JSValue value = JSValue::decode(encodedValue);
if (!value.isCell() || value.asCell()->type() != ArrayType)
return std::numeric_limits<uint64_t>::max();

JSArray* array = uncheckedDowncast<JSArray>(value.asCell());
uint64_t length = array->length();
Structure* structure = array->structure();
if (length || (!structure->outOfLineSize() && !structure->inlineSize()))
return length;

VM& vm = array->vm();
bool found = false;
structure->forEachProperty(vm, [&](const PropertyTableEntry& entry) -> bool {
if ((entry.attributes() & PropertyAttribute::DontEnum) || PropertyName(entry.key()).isPrivateName())
return true;
Comment thread
robobun marked this conversation as resolved.
Resolved
if (WTF::equal(entry.key(), "constructor"_s)) // the property walk prints no key with this text, a symbol too
return true;
found = true;
return false;
});
return found ? namedKeys : 0;
}

// Port of V8's `internalBinding('util').getOwnNonIndexProperties(object, filter)` used by
// util.inspect and the REPL completer: own keys minus array indices, without materializing
// a name (or descriptor) per element the way Object.getOwnPropertyNames() on an array would.
Expand Down
14 changes: 8 additions & 6 deletions src/runtime/bake/client/inspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,6 @@ function formatObject(ctx: InspectContext, value: any, recurseTimes: number) {
* @returns {string} Formatted array
*/
function formatArray(ctx: InspectContext, value: any[], recurseTimes: number) {
// Special case for empty arrays
if (value.length === 0) return "[]";

const maxLength = Math.min(ctx.maxArrayLength!, value.length);
const output: string[] = [];

Expand All @@ -279,12 +276,17 @@ function formatArray(ctx: InspectContext, value: any[], recurseTimes: number) {
return !(Number(key) >= 0 && Number(key) < value.length && Number(key) === +key);
});

if (keys.length > 0) {
for (const key of keys) {
output.push(`${key}: ${formatValue(ctx, value[key], recurseTimes)}`);
for (const key of keys) {
try {
const desc = Object.getOwnPropertyDescriptor(value, key);
const accessor = desc?.get ? (desc.set ? "[Getter/Setter]" : "[Getter]") : desc?.set ? "[Setter]" : null;
output.push(`${key}: ${accessor ?? formatValue(ctx, value[key], recurseTimes)}`);
Comment thread
robobun marked this conversation as resolved.
Resolved
} catch {
output.push(`${key}: undefined`);
}
}

if (output.length === 0) return "[]";
return `[ ${output.join(", ")} ]`;
}

Expand Down
41 changes: 41 additions & 0 deletions test/bake/dev/esm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,47 @@ devTest("browser console forwarding strips terminal control bytes", {
},
});

// https://github.com/oven-sh/bun/issues/22569
devTest("browser console forwarding prints the named properties of an array with no elements", {
files: {
"index.html": emptyHtmlFile({
scripts: ["index.ts"],
}),
"index.ts": `
console.log("array-probe", Object.assign([], { total: 7001 }), Object.assign([5], { total: 7001 }), []);
let calls = 0;
const lazy = { get: () => ++calls, enumerable: true };
console.log("getter-probe", Object.defineProperty([], "lazy", lazy), Object.defineProperty([5], "lazy", lazy));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
console.log("getter-calls", calls);
`,
"bun.app.ts": `
import html from "./index.html";
export default {
static: {
"/": html,
},
development: {
console: true,
},
fetch(req) {
return new Response("Not Found", { status: 404 });
},
};
`,
},
htmlFiles: [],
async test(dev) {
await using c = await dev.client("/");
await c.expectMessage("array-probe", "getter-probe", "getter-calls");
const line = await dev.output.waitForLine(/\[browser\].*array-probe/);
expect(line.input).toContain("array-probe [ total: 7001 ] [ 5, total: 7001 ] []");
const getters = await dev.output.waitForLine(/\[browser\].*getter-probe/);
expect(getters.input).toContain("getter-probe [ lazy: [Getter] ] [ 5, lazy: [Getter] ]");
const calls = await dev.output.waitForLine(/\[browser\].*getter-calls/);
expect(calls.input).toContain("getter-calls 0");
},
});

devTest("error report endpoint tolerates a browser url whose normalized origin is longer than the input", {
framework: minimalFramework,
files: {
Expand Down
Loading
Loading