docs: correct Bun.semver.satisfies invalid range text and the escapeHTML hardware name - #41052
Conversation
|
Updated 7:08 AM PT - Aug 31st, 2026
✅ @robobun, your commit 0f399a7dc87061cf7f72e71e59bb51e3c80f4990 passed in 🧪 To try this PR locally: bunx bun-pr 41052That installs a local version of the PR into your bun-41052 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. WalkthroughThe runtime documentation clarifies ChangesDocumentation updates
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This PR corrects documented semver behavior and a benchmark hardware name without changing runtime code or product behavior. No actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the problem, the fixes, verification steps, scope, and linked issue. It uses Problem and Fix headings instead of the template headings, but it provides the required information.
Comment |
…TML hardware name Bun.semver.satisfies does not return false for an unparseable range. It drops the parts it cannot parse. A range with nothing left behaves like "*". Only an invalid version returns false. The Bun.escapeHTML benchmark text named an "M1X" processor, which does not exist. Use "M1 Max". Fixes #13342
8985f0e to
17cddd2
Compare
|
On the out-of-scope check: the The PR is now scoped to |
|
Took the optional note on the non-ASCII case. |
There was a problem hiding this comment.
LGTM — thanks for adding the non-ASCII carve-out; the satisfies sentence now matches the short-circuit in SemverObject.rs.
What was reviewed:
- Verified each new
satisfiesclaim againstsrc/semver_jsc/SemverObject.rs(non-ASCII guards at 81-86,query::parseonly failing on OOM, wildcard-in-version →false). - Checked prose against the docs voice rules — short sentences, active voice, present tense, "Bun" as the actor.
- The "M1X" → "M1 Max" fix is a straight hardware-name correction.
Extended reasoning...
Overview
This PR touches two documentation files only. docs/runtime/semver.mdx rewrites one sentence describing Bun.semver.satisfies error handling to match actual runtime behavior: invalid version or any non-ASCII byte in either argument returns false; unparseable parts of range are dropped; a fully-unparseable range acts like *. docs/runtime/utils.mdx corrects "M1X" to "M1 Max" in the Bun.escapeHTML benchmark blurb. No source, type, or test files change.
Security risks
None. Prose-only edits to published docs; no code, config, or build changes.
Level of scrutiny
Low, but per the "Docs, types, and comments" rule every published claim must be verified against the implementation. I read src/semver_jsc/SemverObject.rs end-to-end: lines 81-86 return JSValue::FALSE when either argument contains a non-ASCII byte; lines 88-90 return false for wildcard-bearing versions; lines 96-102 confirm query::parse on the range can only fail with OOM, so garbage tokens are silently dropped and an empty group falls through to Group::satisfies, which matches non-prerelease versions like *. Every claim in the new sentence checks out. The prose also passes the voice rules in docs/project/contributing.mdx.
Other factors
My prior inline comment on this PR flagged that the earlier draft did not account for the non-ASCII short-circuit on range. Commit 0f399a7 addressed it by adding "or if either argument contains a non-ASCII character", which correctly scopes the remaining claims to ASCII input. No CODEOWNERS entry covers docs/. No outstanding third-party CHANGES_REQUESTED reviews. The change is small, self-contained, and now accurate.
|
Thanks for the review and merge. |
Problem
docs/runtime/semver.mdxsaysBun.semver.satisfiesreturnsfalsewhenrangeis invalid. It does not.Bun.semver.satisfies("1.0.0", "!!not-a-range!!")returnstrueon bun 1.4.1. Only an invalidversionreturnsfalse.docs/runtime/utils.mdxnames an "M1X" processor in theBun.escapeHTMLbenchmark text. That chip does not exist. The benchmark ran on an M1 Max (Docs:Bun.escapeHTML()references non-existent processor #13342).Fix
satisfiessentence. Bun skips the parts ofrangeit cannot parse. Arangewith no parseable part behaves like*. An invalidversion, or a non-ASCII character in either argument, returnsfalse.Fixes #13342
Background
Bun.semver.satisfiesissrc/semver_jsc/SemverObject.rs. It parsesrangewithbun_semver::query::parse. That parser drops tokens it does not recognize and never fails on bad input.Group::is_emptydocuments this: "the input was only unrecognised words".*range in node-semver.Bun.escapeHTMLJSDoc inpackages/bun-types/bun.d.ts(line 2397) has the same "M1X" text. This PR is scoped todocs/. That copy is for a types change.Notes
Probe on bun 1.4.1:
The
versionhalf of the sentence is unchanged from main. It is not exact for every input:satisfies("", "*")andsatisfies("1.0.0.0", "^1.0.0")returntruebecausesatisfiesdoes not check the parser'svalidflag, whileorderdoes. Those are degenerate inputs and the sentence describes the common case, as before.Not included: #33709 also removed the "Export condition macro" section from
docs/bundler/macros.mdx. On bun 1.4.1,import { macro } from "my-package" with { type: "macro" }resolves through the normal conditions, not the"macro"condition, so that section documents a behavior the bundler does not have. Whether the docs or the resolver changes is a maintainer decision. See the closing comment on #33709.no test proof · iteration 1 · docs-only change; test-proof not applicable