Conversation
The @example on Bun.stringWidth said a family ZWJ sequence measures 1. The runtime (and the string-width package the API is documented to match) measure it as one 2-column grapheme cluster, and the perCodePoint option documented a few lines above already describes it that way. Correct the value and show the perCodePoint: true reading (8) next to it. Add a test that runs the example block from bun.d.ts and compares what it prints against the values in its comments, so the example cannot drift from the implementation again.
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Status: fix and test pushed, waiting on CI. Reproduced with |
There was a problem hiding this comment.
LGTM — doc-only correction of a stale JSDoc example value, with a regression test.
What was reviewed:
- Confirmed
Bun.stringWidth("👩👩👧👦")is 2 (already pinned elsewhere in this test file) and 8 withperCodePoint: true, matching the option's own docs a few lines above. - Traced the new test's extraction:
lastIndexOf("/**")→ code-fence regex → strip*prefixes → run viabunRun;bunRuntrims stdout so the 6 printed lines line up with the 6console.logclaims. - Checked
join(tempDir(...), ...)— Bun'spath.joincoerces theDisposableString, so no TypeError.
Extended reasoning...
Overview
Two files: packages/bun-types/bun.d.ts changes one JSDoc @example comment value (// 1 → // 2) and adds one adjacent example line demonstrating { perCodePoint: true } returning 8. test/js/bun/util/stringWidth.test.ts gains one test that reads the JSDoc block from bun.d.ts, extracts the fenced code, runs it in a temp dir with bunRun, and asserts each printed value matches its trailing // N comment. No runtime or type-signature changes.
Security risks
None. The .d.ts edit is inside a comment; the test only reads a repo-local file, writes to a tempDir, and spawns the debug bun via bunRun/bunExe. No network, no untrusted input.
Level of scrutiny
Low. This is a documentation typo fix. The corrected values are already asserted elsewhere in the same test file ("👩👩👧👦" → 2 in the "emoji widths" block) and are described identically in the perCodePoint option docs directly above the changed example, so the file was internally inconsistent before this PR and is now self-consistent.
Other factors
- Verified the test mechanics:
bunRunreturnsstdout.trim(), so splitting on\nyields exactly one entry perconsole.logline, and the length assertion holds. tempDirreturns aDisposableString(String subclass with bothSymbol.disposeandSymbol.asyncDispose);await usingis valid, and Bun'spath.joinaccepts String objects (confirmed empirically), sojoin(dir, "example.ts")works without explicit coercion.- The
expect(claims).not.toBeEmpty()guard prevents the test from vacuously passing if the JSDoc extraction regex ever stops matching. - PR description states 174/174 pass on debug build and the new test fails against main, satisfying the fails-on-main / passes-with-fix requirement. No prior human review comments to address; only a CodeRabbit rate-limit notice.
|
Updated 4:05 AM PT - Aug 14th, 2026
❌ @robobun, your commit 2555b86 has some failures in 🧪 To try this PR locally: bunx bun-pr 38388That installs a local version of the PR into your bun-38388 --bun |
Problem
@exampleonBun.stringWidthinpackages/bun-types/bun.d.ts(line 588 on main) readsconsole.log(stringWidth("👩👩👧👦")); // 1.2, on the 1.4.0 release and on a debug build of main.stringWidthcommit (5147c0b, 2024) and was never updated when the implementation moved to grapheme clusters. TheperCodePointdocs a few lines above it already say a family sequence measures 2 by default, so the file contradicted itself.Fix
// 2, and add the same string measured with{ perCodePoint: true }(// 8) on the next line, so the reader sees why a four-emoji sequence is 2 columns and how to get the per-code-point figure.2is the correct value to document: the runtime returns it,test/js/bun/util/stringWidth.test.tsalready pins it ("👩👩👧👦"is 2, line 1251 on main), thestring-widthnpm package the JSDoc says this API matches returns 2 as well (checked againststring-width@7.0.0), and theperCodePointdocs in the same file describe it that way.8is what the runtime returns withperCodePoint: true(four emoji at 2 columns each, ZWJs at 0), matching that option's own docs.test/js/bun/util/stringWidth.test.ts("bun.d.ts @example for stringWidth prints the widths its comments claim"): extracts the example block frombun.d.ts, runs it, and compares what it prints with the// Ncomments. Fails on main on the// 1line, passes with this change, and covers every value in the example rather than just the family emoji, so a future width change has to update the example too.bun bd test test/js/bun/util/stringWidth.test.ts: 174 pass (173 pass, 1 fail withpackages/stashed).bun test test/integration/bun-types/bun-types.test.ts: 15 pass.Background
Bun.stringWidthreturns the number of terminal columns a string occupies. By default it measures grapheme clusters: a user-perceived character such as an emoji joined out of several code points with U+200D ZERO WIDTH JOINER counts once, and emoji are 2 columns wide, so the family sequence is 2.perCodePoint: trueswitches to the algorithm Node uses forutil.inspectandconsole.tablealignment, which measures each code point separately: the four emoji in the family sequence are 2 columns each and the three joiners are 0, hence 8.docs/runtime/utils.mdxdoes not contain this example; itsstringWidthvalues were checked and are correct, so only the.d.tschanges here.Runtime check
Failure output of the new test on main: