Conversation
3c56114 to
c1c99b6
Compare
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
docs/upgrade-to-1.4.mdx:68— The two links here —[nested rules](/pm/overrides#nested-overrides)and[version-scoped keys](/pm/overrides#version-scoped-overrides)— point at anchors that don't exist:docs/pm/overrides.mdxhas only## "overrides"and## "resolutions". Worse, that page still says "Bun only supports top-level "overrides", not nested" (line 49) and "nested resolutions are not supported" (line 70), directly contradicting this section. Either updatedocs/pm/overrides.mdxin this PR (add the sections and drop the "not supported" notes) and link to the real headings, or drop the anchors and link to/pm/overridesuntil those sections exist.Extended reasoning...
What the bug is
docs/upgrade-to-1.4.mdx:68introduces two internal links:Bun 1.4 also applies [nested rules](/pm/overrides#nested-overrides) (npm's object form, ...) and [version-scoped keys](/pm/overrides#version-scoped-overrides), ...
Neither anchor exists. Reading
docs/pm/overrides.mdxon this branch, the file has exactly two headings —## "overrides"(line 44) and## "resolutions"(line 66). There is no#nested-overridesheading and no#version-scoped-overridesheading.git log -- docs/pm/overrides.mdxshows the file is untouched by this PR and its base branch, so nothing upstream is adding those sections either.The second half of the problem is worse than a dead anchor: the target page still documents the pre-1.4 behavior. Lines 48-51:
Bun only supports top-level
"overrides", not nested overrides.and line 70:
As with
"overrides", nested resolutions are not supported.So the upgrade guide tells the reader "Bun 1.4 now applies nested and version-scoped overrides", links them to
/pm/overridesfor the details, and that page tells them the opposite.Why existing checks didn't catch it
The PR description states "Internal links resolve to pages and headings that exist on main", but that verification evidently checked page paths, not fragment anchors —
/pm/overridesexists,#nested-overrideson it does not. Mintlify does not fail the build on a fragment that resolves to no heading; the link simply scrolls nowhere.prettier --check(the only check the description mentions running) does not validate anchors.Step-by-step proof
- Open
docs/upgrade-to-1.4.mdxat line 68 on this branch: the two links target/pm/overrides#nested-overridesand/pm/overrides#version-scoped-overrides. grep -n '^##' docs/pm/overrides.mdxon this branch:Neither target heading exists, so both fragments are dead.44:## "overrides" 66:## "resolutions"sed -n '48,51p;70p' docs/pm/overrides.mdxshows the two "not supported" statements quoted above — the linked page contradicts the upgrade guide.git log --oneline -- docs/pm/overrides.mdxshows no commit on this branch or its base touching the file, so merging this PR ships the contradiction.
Impact
A reader following either link from the 1.4 upgrade guide lands at the top of
/pm/overrides(the fragment matches nothing, so the browser doesn't scroll), reads down, and is told nested overrides are not supported — the exact opposite of what the guide just said. For a page whose purpose is to explain what changed in 1.4, that's actively misleading, not just cosmetic.The repo's own docs guidance (
.claude/docs/landing-prs.md, "Docs, types, and comments") says: "Sweep the same PR for everything describing the old state ... A comment contradicting the code is a correctness bug, not a nit" and "fetch every URL". This PR's whole purpose is that sweep, andoverrides.mdxis squarely inside it.How to fix
Preferred: update
docs/pm/overrides.mdxin this PR — remove the two "not supported" notes and add### Nested overridesand### Version-scoped overridessections (or whatever headings you want the anchors to be), then point the two links here at those real slugs.Minimal: drop the fragments and link both to plain
/pm/overrides, and delete the two "not supported" statements from that page so it at least stops contradicting the guide. The detailed sections can follow later. - Open
|
The two anchors flagged here were in the first revision of this PR ( Checking this did turn up a related problem on the base branch: it also added a second |
|
Right, the "unchanged" clause was wrong. Checked on the current build: for both One correction to the review: the |
There was a problem hiding this comment.
LGTM — the module-not-found bullet was rewritten in b8dc487d to cover both require() and import/import() message changes, and now matches ResolveMessage::node_message() (including the note that the code values did not change, which corrects my earlier remark). docs/pm/overrides.mdx has each heading once, so the #nested-overrides / #version-scoped-overrides anchors resolve unambiguously.
Extended reasoning...
Overview
Docs-only change to two files: docs/upgrade-to-1.4.mdx gains ~20 new behavior-change entries (MySQL public-key retrieval, TLS enforcement, .npmrc / registry-credential precedence, module.enableCompileCache(), module-not-found messages, AbortError wording, GCM IV length, mkdtemp(""), vm options, ICU 78, compression-stream chunking, SQLite bindings, server.reload, bundler splitting/enum/export-order/minify-$, and CLI outdated/dedupe/up/workspace:/isolated-store items), plus a summary-table row and a wording tweak to the TOML integer bullet. docs/pm/overrides.mdx drops the duplicate "Nested overrides" section the base branch added and folds the upgrade-guide pointer into the existing lockfileVersion 3 limitation.
Follow-up on prior review
My earlier inline finding (the "messages for import and import() are unchanged" clause) was addressed in b8dc487d. I re-checked the rewritten bullet against src/jsc/ResolveMessage.rs:392-413: the require() form (Cannot find module 'x' + Require stack:) and both import forms (Cannot find package 'x' imported from ... / Cannot find module './x' imported from ...) match, and the author is correct that the code values were already MODULE_NOT_FOUND / ERR_MODULE_NOT_FOUND in 1.3, so "unchanged" is accurate there. I also confirmed docs/pm/overrides.mdx now lists each ## heading exactly once.
Security risks
None. Pure documentation; no executable code, config, or CI changes.
Level of scrutiny
Medium — factual accuracy matters for an upgrade guide, but the author documented per-claim verification against 1.4.0-canary.1+8326d1bd3 and the bun-v1.3.14 tag, cited the source PR for each item, and responded precisely to the one factual error found in the prior pass (including correcting a detail I had wrong). This run's bug hunt found nothing further.
Other factors
Stacked on #36463, so this merges into that branch rather than main. The overrides dedup is defensive against a rebase artifact and leaves the page with three added sentences relative to main. prettier --check reportedly passes.
MySQL public key retrieval now refused by default, tls/ssl options requiring TLS, ~/.npmrc fallback and registry URL credentials, module.enableCompileCache() being implemented, the require() not-found and AbortError messages, GCM IV validation, mkdtemp and vm validation, server.reload, ICU 78, Compression stream chunking, the Bun.SQL sqlite binding change, splitting with cjs/iife being an error, transpiler output differences, bun outdated exit codes, the dedupe/up commands, workspace: ranges in downloaded packages, and isolated store entry names.
import and import() failures now read "imported from" in error.message as well; only the codes and Bun's printed line are unchanged.
b8dc487 to
de55df7
Compare
|
Rebased onto the base branch's current head ( |
There was a problem hiding this comment.
The prior finding on the module-not-found bullet is addressed — it now covers the import / import() wording change and correctly states the codes are unchanged. No new issues found on this revision. Given this adds ~20 distinct factual claims about 1.3→1.4 behavior across install, SQL, runtime, and bundler, a maintainer familiar with the 1.4 release scope should still give it a read for coverage and phrasing.
Checked: the rewritten not-found bullet against ResolveMessage::node_message() and missing-module.test.js; the /upgrade-to-1.4 link from overrides.mdx and the #mysql anchor; that overrides.mdx has no duplicate headings after the rebase; the summary-table row lines up with its section.
Extended reasoning...
Overview
Docs-only change to two files: docs/upgrade-to-1.4.mdx gains a MySQL public-key-retrieval section (plus summary-table row), a module.enableCompileCache() section, an .npmrc/credentials addendum, a TLS-required note under the Postgres section, and roughly fifteen new bullets across the Node-compat / Bun-APIs / bundler / CLI lists. docs/pm/overrides.mdx gains one sentence pointing the lockfileVersion 3 limitation at the upgrade guide. Stacked on #36463.
Security risks
None — documentation only, no code paths touched.
Level of scrutiny
Medium. There is no runtime risk, but this is user-facing release migration documentation making many specific, verifiable claims (exact error messages, error codes, option names, version numbers). The prior revision contained a factual error in exactly this class ("the messages for import are unchanged"), which was caught and fixed. The author has documented empirical verification for each runtime claim against a 1.4 canary and source-checked the install/bundler claims, which raises confidence, but the sheer breadth (MySQL auth, ICU versions, GCM IV limits, isolated-linker store names, enum lowering, etc.) is more than an automated pass can fully fact-check.
Other factors
- My prior inline finding was addressed in
b8dc487de1; the author also correctly pushed back on the sub-claim about theimporterrorcodechanging (it did not — 1.3 already usedERR_MODULE_NOT_FOUND), and the current text reflects that. - The bug-hunting pass on this revision found nothing.
- The
overrides.mdxduplicate-heading issue was resolved on the base branch and this PR was rebased over it; the current file lists each heading once. - Because this merges into #36463 rather than
main, a human will review the combined guide there regardless; deferring here keeps that loop intact without blocking.
|
Nothing further to change from this pass. Current state for whoever reads the combined guide on #36463: head |
…pgrade guide
The guide covers migration steps; narrow validation changes (GCM IV
length, mkdtemp(""), vm options, server.reload, isolated store entry
names, registry URL credentials) are tracked in #28792 instead.
|
Agreed. Since this PR was merged into the #36463 branch, the trimming happened there: f40d4dd removed the compile cache section, and f24047a drops the other items from this batch that were validation changes rather than migration steps (GCM IV length, |
#39445) Stacked on #36463 (the base branch is that PR's branch, so the diff here is only the additions). Merging this into #36463 adds the behavior changes listed below; #36463 itself now covers the #38333 install batch, the optional-peer correction, and the TOML / `bun init` fixes, so this PR no longer touches those. ### Problem - These 1.3 to 1.4 behavior changes are not in the guide at `701b3e2a0`: - MySQL: the first `caching_sha2_password` connection over plain TCP is refused unless `allowPublicKeyRetrieval: true` (#31129; 1.3.14 requested the key automatically, `MySQLConnection.zig` in the 1.3.14 tag). SQL `tls` / `ssl` options now require TLS instead of falling back to plaintext, and `?ssl=` / `?ssl-mode=` are read (`shared.ts` 1.3.14 only read `?sslmode=`; #37669). - Install: `~/.npmrc` fallback when `XDG_CONFIG_HOME` is set (#36289), credentials in `--registry` / env / bunfig object URLs are sent and outrank same-host `.npmrc` tokens (#38796, #38824), `bun outdated` exits 1 on fetch failures (#38809), new `dedupe` / `up` commands shadow scripts of those names and `bun feedback` is removed (#38333, #38444), `workspace:` ranges inside registry packages (#37669), isolated store entry names (#39014). - Runtime: `module.enableCompileCache()` / `NODE_COMPILE_CACHE` implemented (#34660), `require()` / `import` not-found messages (#34660), `AbortError` message without the period (#39277; 1.3.14's `BunCommonStrings.h` has the period), GCM IV length (#34092), `mkdtemp("")` (#34908), vm options (#38381), `server.reload` (#38697), ICU 75/73 to 78 (#38013), Compression stream chunking (#38695), `Bun.SQL` sqlite bindings (#35950). - Bundler: `splitting` with `cjs` / `iife` is an error (#32685), block-scoped `enum` lowers to `let` (#34249), exports emitted ascending instead of descending (#35957; `doStep5.zig` in 1.3.14 used `sortDesc`), minified `$` (#35668). - The TOML integer bullet did not say what the limit or the fix is. ### Fix - Adds a MySQL public key section (plus a summary table row), a TLS note under the `PGSSLMODE` section, an `.npmrc` / credentials addendum to the `bunfig.toml` section, a `module.enableCompileCache()` section, and the rest as bullets in the existing lists. - `docs/pm/overrides.mdx`: one-line change adding a pointer to this guide in the existing `lockfileVersion` 3 limitation. (The base branch briefly had a duplicate "Nested overrides" section; it removed that itself in `8257d01acb`, and this PR was rebased over it.) - Verification: each runtime claim was run against `1.4.0-canary.1+8326d1bd3` (22 commits behind main; contains every change referenced), and each install or bundler claim was checked against the source on main, with the 1.3 side taken from the `bun-v1.3.14` tag where the PR body did not state it. The `/runtime/sql#mysql` and `/upgrade-to-1.4` links resolve. `prettier --check` passes. ### Not included on purpose - Lifecycle scripts no longer receiving `npm_package_name` / `npm_package_version` / `npm_package_json` / `npm_config_local_prefix` during `bun install`, and transitive `"*"` ranges no longer deduplicating onto the root's version: regressions with open fixes (#36690, #38110, #38770). They need either the fixes or a guide line before release. - Postgres `sslmode=prefer` / `allow` (including `PGSSLMODE=prefer`, which 1.4 newly reads) hangs until the connection timeout against a server without SSL because nothing sends the startup message after the `N` reply. Same code in 1.3.14; filed as a bug instead of documented. <details> <summary>Commands used to verify the runtime claims</summary> ``` timers/promises setTimeout with an aborted signal # "The operation was aborted" bun req.cjs # Cannot find module ... Require stack: bun b.mjs (import() of a missing package / relative file) # Cannot find package 'x' imported from /path, ERR_MODULE_NOT_FOUND bun a_static.mjs (unhandled static import) # printed line still: Cannot find package 'x' from '/path' process.versions.icu # 78.3 createCipheriv("aes-128-gcm", key, Buffer.alloc(129)) # ERR_CRYPTO_INVALID_IV DecompressionStream of a 1 MiB gzip member # 16 chunks of 65536 bytes new SQL("sqlite://:memory:") with ${[1,2]} / ${new Date()} # Binding expected ... fs.mkdtempSync("") # EINVAL vm.runInThisContext("1", []) # ERR_INVALID_ARG_TYPE NODE_COMPILE_CACHE=/tmp/cc bun cc.cjs # creates /tmp/cc/v1.4.0-x86_64-<sha>-<uid> NODE_DISABLE_COMPILE_CACHE=1 + enableCompileCache() # status 3 (DISABLED) bun dedupe / bun up with package.json scripts of those names # built-in command runs bun feedback # Script not found "feedback" Bun.build({ splitting: true, format: "cjs" }) # Code splitting is currently only supported ... bun build of a function-scoped enum and import * as ns # let Color; exports a, m, z new SQL({ url: "postgres://...", tls: true }) on a non-TLS server # ERR_POSTGRES_TLS_NOT_AVAILABLE Bun.TOML.parse("a = 9007199254740993") # Integer cannot be losslessly represented ... ``` </details> <details> <summary>Previous revision</summary> The first revision of this PR (`3c5611454a`) also rewrote the package manager section for #38333 / #38853 (nested overrides and `lockfileVersion: 3`, the optional-peer correction, `bun update`, `bunfig.toml` over `.npmrc`, `--filter`) and fixed the TOML date and `bun init` lines. #36463 picked those up in its own commits the same day, so this PR was rebased onto its new head and reduced to the items above. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · docs-only change; test-proof not applicable <!-- robobun:evidence:end -->
…pgrade guide
The guide covers migration steps; narrow validation changes (GCM IV
length, mkdtemp(""), vm options, server.reload, isolated store entry
names, registry URL credentials) are tracked in #28792 instead.
Stacked on #36463 (the base branch is that PR's branch, so the diff here is only the additions). Merging this into #36463 adds the behavior changes listed below; #36463 itself now covers the #38333 install batch, the optional-peer correction, and the TOML /
bun initfixes, so this PR no longer touches those.Problem
701b3e2a0:caching_sha2_passwordconnection over plain TCP is refused unlessallowPublicKeyRetrieval: true(Hardening: input validation and bounds tightening across 26 subsystems #31129; 1.3.14 requested the key automatically,MySQLConnection.zigin the 1.3.14 tag). SQLtls/ssloptions now require TLS instead of falling back to plaintext, and?ssl=/?ssl-mode=are read (shared.ts1.3.14 only read?sslmode=; Robustness and input-handling pass across install, shell, TLS/QUIC, HTTP/3, SQL and crypto #37669).~/.npmrcfallback whenXDG_CONFIG_HOMEis set (install: fall back to $HOME/.npmrc when $XDG_CONFIG_HOME is set #36289), credentials in--registry/ env / bunfig object URLs are sent and outrank same-host.npmrctokens (install: send credentials embedded in --registry and registry env var URLs #38796, bunfig: send credentials written into the url of a registry object #38824),bun outdatedexits 1 on fetch failures (install: failbun outdatedandbun update -iwhen a dependency's manifest cannot be fetched #38809), newdedupe/upcommands shadow scripts of those names andbun feedbackis removed (install: pnpm parity — dedupe, prune, pm licenses, audit fix, add --filter/--catalog, nested overrides, transitive update, and workspace fixes #38333, Remove the bun feedback command #38444),workspace:ranges inside registry packages (Robustness and input-handling pass across install, shell, TLS/QUIC, HTTP/3, SQL and crypto #37669), isolated store entry names (install: leave URL credentials out of isolated store entry names #39014).module.enableCompileCache()/NODE_COMPILE_CACHEimplemented (node compat batch: callback-throw dispatch, Assert class + native deep-equality parity, Intl gate + URL/buffer fallout, compile cache, watch kill-signal, profilers (+98 tests) #34660),require()/importnot-found messages (node compat batch: callback-throw dispatch, Assert class + native deep-equality parity, Intl gate + URL/buffer fallout, compile cache, watch kill-signal, profilers (+98 tests) #34660),AbortErrormessage without the period (Drop the trailing period from the node-shaped AbortError message #39277; 1.3.14'sBunCommonStrings.hhas the period), GCM IV length (crypto: cap GCM IV length at 128 bytes in createCipheriv #34092),mkdtemp("")(fs: reject empty mkdtemp prefix with EINVAL #34908), vm options (node:vm: reject array and function options like Node's validateObject #38381),server.reload(serve: reload() refuses a config that would leave the server with no handler #38697), ICU 75/73 to 78 (url: drop the Unicode 16 IDNA pre-pass; bump bundled ICU to 78 #38013), Compression stream chunking (CompressionStream/DecompressionStream: emit each chunk's output in bounded steps #38695),Bun.SQLsqlite bindings (sql(sqlite): pass positional bindings to bun:sqlite as one array, not spread #35950).splittingwithcjs/iifeis an error (bundler: error instead of panic when splitting is used with non-ESM format #32685), block-scopedenumlowers tolet(js_parser: lower block-scoped TypeScript enums with let instead of var #34249), exports emitted ascending instead of descending (bundler: sort module namespace exports ascending to match spec #35957;doStep5.zigin 1.3.14 usedsortDesc), minified$(bundler: never pick bare$as a minified identifier #35668).Fix
PGSSLMODEsection, an.npmrc/ credentials addendum to thebunfig.tomlsection, amodule.enableCompileCache()section, and the rest as bullets in the existing lists.docs/pm/overrides.mdx: one-line change adding a pointer to this guide in the existinglockfileVersion3 limitation. (The base branch briefly had a duplicate "Nested overrides" section; it removed that itself in8257d01acb, and this PR was rebased over it.)1.4.0-canary.1+8326d1bd3(22 commits behind main; contains every change referenced), and each install or bundler claim was checked against the source on main, with the 1.3 side taken from thebun-v1.3.14tag where the PR body did not state it. The/runtime/sql#mysqland/upgrade-to-1.4links resolve.prettier --checkpasses.Not included on purpose
npm_package_name/npm_package_version/npm_package_json/npm_config_local_prefixduringbun install, and transitive"*"ranges no longer deduplicating onto the root's version: regressions with open fixes (install: set npm_package_name/version/json for dependency lifecycle scripts #36690, install: set npm_config_local_prefix and npm_package_config_* for lifecycle scripts #38110, install: keep one copy of a package when one version satisfies every range #38770). They need either the fixes or a guide line before release.sslmode=prefer/allow(includingPGSSLMODE=prefer, which 1.4 newly reads) hangs until the connection timeout against a server without SSL because nothing sends the startup message after theNreply. Same code in 1.3.14; filed as a bug instead of documented.Commands used to verify the runtime claims
Previous revision
The first revision of this PR (
3c5611454a) also rewrote the package manager section for #38333 / #38853 (nested overrides andlockfileVersion: 3, the optional-peer correction,bun update,bunfig.tomlover.npmrc,--filter) and fixed the TOML date andbun initlines. #36463 picked those up in its own commits the same day, so this PR was rebased onto its new head and reduced to the items above.no test proof · iteration 0 · docs-only change; test-proof not applicable