Skip to content

console.table: ignore properties for a Map - #47014

Open
robobun wants to merge 2 commits into
mainfrom
robobun/d0899f9b/console-table-properties-columns
Open

robobun wants to merge 2 commits into
mainfrom
robobun/d0899f9b/console-table-properties-columns

Conversation

@robobun

@robobun robobun commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • console.table(map, properties) and Bun.inspect.table(map, properties) print one empty column for each properties entry. console.table(new Map([["k", 1]]), ["a"]) prints Key | a | Values with a blank.
  • An entry whose conversion throws (a throwing toString, a Symbol) stops the table, although no row uses it.
  • Cause: TablePrinter::print_table (src/jsc/ConsoleObject.rs:894) adds a column for each entry. The Map branch of collect_row (:731-745) fills only Key and Values.

Fix

  • print_table adds the properties columns only in the else of the JSType::is_map() test that adds Key. A Map now prints the same table with and without properties.
  • is_map() is also true for a WeakMap, so a WeakMap changes in the same way (see Notes).
  • Verified: test/js/bun/console/bun-inspect-table.test.ts (9 new tests, 8 fail without the fix). Also the three console test directories.
  • Self-reviewed: 13 concerns raised, 13 addressed.

Background

  • TablePrinter is the native printer behind console.table (also the node:console export) and Bun.inspect.table. collect_row reads one row and fills its cells.
  • For a Map it prints Key, and Values once the Map has an entry.
  • Considered the other properties rules of Node.js, which Notes lists. They change valid output, and no user reported them.
Notes

Origin

There is no user report. A comparison with Node.js v26.3.0 found this. Node prints Key and Values for a Map with properties. The loop dates from the first console.table (#7866).

Repro

console.table(new Map([["k", { a: 1 }]]), ["a", "b"]);

main (449877f) and bun 1.4.2:

┌───┬─────┬───┬───┬──────────┐
│   │ Key │ a │ b │ Values   │
├───┼─────┼───┼───┼──────────┤
│ 0 │ k   │   │   │ { a: 1 } │
└───┴─────┴───┴───┴──────────┘

This PR:

┌───┬─────┬──────────┐
│   │ Key │ Values   │
├───┼─────┼──────────┤
│ 0 │ k   │ { a: 1 } │
└───┴─────┴──────────┘
  • ["Key", "Values"] printed Key | Key | Values | Values.
  • [{ toString() { throw new Error("x") } }] threw Error: x. [Symbol()] threw TypeError: Cannot convert a symbol to a string.

Record

A review thread on #46986 (comment) asked for three changes to this loop: no conversion for zero rows, no properties column for a Map, and none for a Set. The reply left all three out as a change of the column model. This PR reverses that reply for a Map only.

  • A Map row can fill no properties column, so each removed cell is blank.
  • A Set row fills its properties columns: new Set([{ a: 1 }]) with ["a"] prints a with the cell 1. That stays.
  • A table with zero rows prints the names as its header. That stays.

The body of #46986 is not edited. A reply on that thread links this PR.

WeakMap

JSType::is_map() (src/jsc/JSType.rs:782) matches a Map and a WeakMap. The guard uses it on purpose. It is the test that adds Key, and collect_row (:731) uses the same test before it writes columns[1] (:740). The three uses must stay one predicate.

Bun.inspect.table(data, properties) main this PR Node v26.3.0 console.table
new WeakMap(), ["a"] Key, a Key (index)
Object.assign(new WeakMap(), { x: { a: 1 } }), ["a"] Key, a, Values. Row x: undefined, blank, undefined Key, Values. Row x: undefined, undefined (index), a. Row x: 1
same data, an entry whose toString throws throws prints, no conversion throws
same data, a Symbol entry TypeError prints (index) only

Not in this PR

  • A WeakMap prints as a Map. console.table(new WeakMap()) prints a Key header (Node: (index) only). Object.assign(new WeakMap(), { x: 5 }) prints the row x | undefined | undefined (Node: Values with 5). With { x: null } it throws TypeError: null is not an object (Node: Values with null). These are the same on main and with this PR. The cause is JSType::is_map() itself, which is_object_empty (src/jsc/JSValue.rs:2788) also uses. A correction changes that predicate and does not edit this guard. No issue tracks this.
  • An empty Map prints Key alone, with or without properties. Node prints Key and Values. new Map() with ["a"] goes from Key | a to Key.
  • The other properties rules of Node.js: one column for a repeated entry, integer-like names first, no column for zero rows, Set entries under Values. This PR has only the part where every removed cell is blank.
  • The JS Console class (new console.Console(), node:worker_threads workers) is a different printer. This PR does not touch it.

Open PRs on the same code

Measurements (linux x64, release builds of 449877f with and without the change)

  • .text: 82,164,617 B in both. Stripped bun: 107,730,656 B in both.
  • print_table::<false>: 3,629 B to 3,640 B. print_table::<true>: 3,709 B to 3,704 B. collect_row: 2,981 B in both.
  • Conversions of one properties entry for a Map: 1 to 0.
  • Data that is not a Map or a WeakMap: the same two tests run in the same order.
  • Existing snapshots changed: 0 of 48. test/js/bun/console/, test/js/web/console/ and test/js/node/console/: 122 pass.

Tests

  • "still names the columns of a Map that is a row" is a control. It passes without the fix.
  • The last test runs the global console.table in a child process.
  • The WeakMap test and the empty-Map test compare two calls and have no literal table. They hold when the WeakMap typing or the empty-Map header changes.

Downsides

  • Behaviour change: a call with a Map or a WeakMap and properties prints fewer columns. An entry that throws no longer throws there.
  • None found for other data. Checked: the diff adds no branch, call or allocation, and release .text keeps its size (82,164,617 B).

A Map row fills only the Key and Values columns. `TablePrinter::print_table`
pushed one column per `properties` entry all the same, so each of those
columns was empty in every row, and an entry whose conversion throws stopped
a table that never uses it.

The `properties` columns are now created only when `JSType::is_map()` is
false for the data. That test is the one that adds the Key column and that
sends a row to the Map branch of `collect_row`. It matches a WeakMap as well
as a Map, so a WeakMap with `properties` changes in the same way.

This applies to `console.table` and to `Bun.inspect.table`.
@robobun

robobun commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator Author

Reproduced on main (449877f) and on bun 1.4.2:

console.table(new Map([["k", 1]]), ["a"]);
┌───┬─────┬───┬────────┐
│   │ Key │ a │ Values │
├───┼─────┼───┼────────┤
│ 0 │ k   │   │ 1      │
└───┴─────┴───┴────────┘

Column a is empty in every row. With this PR (#47014) the call prints Key and Values only.

CI status (c3bec93, build #133279)

The diff is green. 179 of 181 jobs passed, and no test is red.

  • The 2 jobs that remain are darwin any x64 - test-bun. They had no free agent for 85 minutes, expired, and are in the queue again. They did not run a test yet.
  • The first build (#133252) had one red test: test/js/first_party/ws/ws-proxy.test.ts on Windows 2019 x64. This PR does not touch it, and it passed in build #133279.
  • test/js/bun/console/bun-inspect-table.test.ts has no failure on any lane in the two builds.

Ready for review.

@robobun

robobun commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 4:57 PM PT - Oct 26th, 2026

✅ @robobun, your commit c3bec93ff33170d42b7ce6c12fdd6622e06389e0 passed in Build #133279! 🎉


🧪   To try this PR locally:

bunx bun-pr 47014

That installs a local version of the PR into your bun-47014 executable, so you can run:

bun-47014 --bun

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 3396aea and 68d1d26.

Files selected for processing: 2
  • src/jsc/ConsoleObject.rs
  • test/js/bun/console/bun-inspect-table.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Walkthrough

print_table now skips property-column initialization for Map tables. Tests cover Map output with properties, Map edge cases, and console.table behavior in a subprocess.

Changes

Map table handling

Layer / File(s) Summary
Map table columns and output tests
src/jsc/ConsoleObject.rs, test/js/bun/console/bun-inspect-table.test.ts
Map tables no longer add columns from the properties argument. Tests cover Map subclasses, duplicate column names, non-string and Symbol entries, empty Maps and WeakMaps, Maps used as rows, and subprocess output.

Suggested reviewers: jarred-sumner

Priority: Low

Merge Risk: Minimal · up to c3bec

Merge Risk: ⚪ Minimal · up to c3bec

No merge-blocking issue is identified for the Map table column change.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: console.table ignores properties for Maps.
Description check Passed The description explains the problem, implementation, scope, behavior changes, limitations, and verification results. It does not use the template headings exactly, but it provides the required inform…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the problem, implementation, scope, behavior changes, limitations, and verification results. It does not use the template headings exactly, but it provides the required information and is substantially complete.

Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

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.

Code review found no issues

No high-confidence issues detected in this change.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant