bundler: give files imports the same display path as disk files

[robobun]

Problem

• A module supplied through Bun.build({ files }) and reached through an import is displayed under three different names depending on how the import was resolved. The display name is what ends up in the // path comment above the module, in the metafile input key and imports[].path, and in the chunk's isolated hash, so the hashed output filename changes with it:
  • no plugin involved: the absolute files key (// /tmp/proj/lib.js, metafile key /tmp/proj/lib.js)
  • an onResolve plugin matched the import and returned undefined: comment still absolute, but the metafile key is relativized and the chunk hash differs from the no-plugin build. Declining is supposed to be a no-op.
  • an onResolve plugin returned the key itself, the same file used as an entry point, or the same file read from disk: relative to the working directory (proj/lib.js), like every other file.
• So a fully in-memory build lists ../app/entry.js and /app/lib.js side by side in one metafile, and overriding a disk file through files changes the output bytes and the hashed filename even when the contents are identical, because the absolute path of the build machine is printed into the bundle.
• Cause: the two FileMap import branches in src/bundler/bundle_v2.rs set up pretty by hand instead of calling path_with_pretty_initialized like the disk, entry point and plugin paths do.
  • resolve_import_records (no plugin) copies the absolute key into pretty.
  • run_resolver (plugin declined) does the same on a local copy of the path and never writes it back into the resolver::Result, and ParseTask::init reads the path from that result. The parsed source therefore has pretty aliasing text, which LinkerContext::generate_isolated_hash treats as "not initialized" and relativizes in place, after the filename comment was already printed. That is where the hash and metafile key diverge from the no-plugin build.

Fix

• Both branches now call path_with_pretty_initialized, and run_resolver writes the result back into the resolver::Result that ParseTask::init reads. A files module gets the same display path as a disk file at the same location, whichever way the import reached it, and no absolute path of the build machine is written into the output.
• This is the convention everything else already follows: Path::pretty is documented as the cwd-relative display path, files entry points (enqueue_entry_item), plugin-resolved paths (on_resolve) and disk imports all go through the same function, and generate_isolated_hash hashes pretty precisely because it is supposed to be location independent. The files option keeps working as an override of disk files only if the override is invisible in the output. Builds that did not use a plugin will see the metafile key and filename comment of files imports change from the absolute key to the relative form their entry points already used.
• generate_isolated_hash asserted pretty.ptr != text.ptr after initializing the path. That is not an invariant: a relative files key such as "bare-key.js" relativizes to itself and dupe_alloc re-slices pretty out of text, so once relative keys take this path too, debug builds tripped the assertion (release builds were unaffected; the recomputation is idempotent). The assertion is replaced with assert_pretty_is_valid(), which is the check this code had before the port. bundler: allow relative FileMap keys without tripping absolute-path debug asserts #32716 relaxes the same assertion for the entry point version of this situation; its enqueue_entry_item change is independent of this PR.
• Tests, in test/bundler/bundler_files.test.ts:
  • the same files import built with no plugin, with a declining onResolve plugin, and with a plugin returning the key produces identical output text, hashed filenames and metafile
  • the same comparison for a bare key and a ./ key (this one also exercises the assertion in debug builds)
  • overriding a disk file through files with identical contents produces byte-identical output, filename and metafile to the plain disk build
  • in a fully in-memory build, the import's metafile key sits next to its entry point's key and imports[].path matches it
• All four fail on the released build (see below) and pass with this change. metafile.test.ts, bun-build-api.test.ts and bundler-plugin-onresolve-entrypoint.test.ts still pass.

Background

• Path in the bundler carries two strings: text, the canonical location used as the identity of the module (for files, the map key), and pretty, the display form. pretty is what is printed in filename comments, used as metafile keys, and hashed into chunk names; for file-namespace paths it is made relative to the working directory with forward slashes by path_with_pretty_initialized.
• A freshly constructed Path has pretty pointing at the same bytes as text. Several places use that pointer equality as "display path not computed yet" and compute it on the spot, which is why the plugin-declined build ended up relativized late instead of never.
• FileMap is the files option. FileMap::resolve returns a resolver::Result for a key, and ParseTask::init takes its path from that result, so any display path set up for the module has to be stored back into it.

Probe on the released build (1.4.0), same inputs, three ways to reach the import

=== JS entry on disk, lib.js from files ===
no plugin        -> [ "./entry-tzeagbbb.js" ]  inputs: [ "p-FDpfUu/entry.js", "/tmp/p-FDpfUu/lib.js" ]
   comments: // /tmp/p-FDpfUu/lib.js | // p-FDpfUu/entry.js
declining plugin -> [ "./entry-sgda3ws2.js" ]  inputs: [ "p-FDpfUu/entry.js", "p-FDpfUu/lib.js" ]
   comments: // /tmp/p-FDpfUu/lib.js | // p-FDpfUu/entry.js
resolving plugin -> [ "./entry-eyf9vzsd.js" ]  inputs: [ "p-FDpfUu/entry.js", "p-FDpfUu/lib.js" ]
   comments: // p-FDpfUu/lib.js | // p-FDpfUu/entry.js

=== entry and lib both from files ===
no plugin        -> inputs: [ "../app/entry.js", "/app/lib.js" ]
declining plugin -> inputs: [ "../app/entry.js", "../app/lib.js" ]

With this change all of the above print // p-FDpfUu/lib.js, key the module as p-FDpfUu/lib.js, and produce the same hashed filename.

New tests on the released build

(fail) bundler files option > in-memory import is displayed the same way no matter how it was resolved
(fail) bundler files option > in-memory imports with relative keys are displayed the same way with and without a declining plugin
(fail) bundler files option > overriding a disk file in memory produces the same output as the disk file
(fail) bundler files option > in-memory import uses the same display path convention as its in-memory entry point
 23 pass
 4 fail

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 28 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e0393f93-08f3-45bc-ac1f-305079f79b6c

📥 Commits

Reviewing files that changed from the base of the PR and between 2f5c180 and 5086995.

📒 Files selected for processing (3)

• src/bundler/LinkerContext.rs
• src/bundler/bundle_v2.rs
• test/bundler/bundler_files.test.ts

---

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

[robobun]

Status: reproduced on the released 1.4.0 build with the probe in the description (same files import, three different display names depending on whether an onResolve plugin declined it). The four new tests in test/bundler/bundler_files.test.ts fail on that build and pass with this branch under a debug build; metafile.test.ts, bun-build-api.test.ts and bundler-plugin-onresolve-entrypoint.test.ts also pass. Waiting on CI.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes user-visible bundler output (metafile keys, filename comments, and chunk hashes for files imports switch from absolute to cwd-relative) and touches the core resolve loop in bundle_v2.rs, a human sign-off on the behavior change would still be worthwhile.

What was reviewed:

• Both FileMap branches now route through path_with_pretty_initialized like disk/entry-point/plugin paths already do; run_resolver writes the result back into file_map_result so ParseTask::init reads the relativized path — matches the existing pattern at enqueue_entry_item and on_resolve.
• Confirmed assert_pretty_is_valid() is defined on bun_paths::fs::Path (the type Source::path uses), so the stale comment about it being resolver-only was wrong; the relaxed assertion is sound because dupe_alloc re-slices pretty at offset 0 of text when they're byte-equal.
• The dropped unsafe { detach_lifetime(...) } is subsumed by the arena-lifetime erasure already inside path_with_pretty_initialized.
• Four new tests cover no-plugin / declining-plugin / resolving-plugin, bare and ./ relative keys, disk-override byte-equality, and metafile key consistency.

Extended reasoning...

Overview

This PR fixes an inconsistency in how Bun.build({ files }) modules are named for display purposes. Two FileMap import branches in src/bundler/bundle_v2.rs (run_resolver for the plugin-declined path, resolve_import_records for the no-plugin path) were setting Path::pretty by hand — copying the absolute key verbatim — instead of calling path_with_pretty_initialized, which every other code path (disk imports, entry points, plugin-resolved paths) already uses to produce a cwd-relative display path. The fix routes both branches through the shared helper and, in run_resolver, writes the result back into the resolver::Result that ParseTask::init reads. A related over-strict debug assertion in generate_isolated_hash (pretty.ptr != text.ptr) is replaced with assert_pretty_is_valid(), since dupe_alloc can legitimately re-slice pretty at offset 0 of the interned text when a relative key relativizes to itself.

Security risks

None. This is display-path normalization for bundler output — filename comments, metafile input keys, and the isolated chunk hash. No untrusted input parsing, auth, crypto, or filesystem-write logic is touched.

Level of scrutiny

Medium-high. The change is small (~20 lines of production code) and is a convergence onto an existing, well-exercised helper rather than new logic. But it sits in bundle_v2.rs's core resolve loop, changes user-visible bundler output (metafile keys and // path comments for files imports switch from absolute to relative, and chunk hashes change accordingly), and interacts with FilenameStore interning via dupe_alloc. The PR description is exceptionally thorough — it traces the mechanism end-to-end, shows before/after probe output, references the sibling assertion relaxation in #32716, and confirms the four new tests fail on the released build and that metafile.test.ts, bun-build-api.test.ts, and bundler-plugin-onresolve-entrypoint.test.ts still pass.

Other factors

I verified: assert_pretty_is_valid is defined on bun_paths::fs::Path (src/paths/lib.rs:913), which is the type Source::path uses via re-export, so the removed comment claiming it was unavailable there was stale. The dropped unsafe { bun_ptr::detach_lifetime(...) } in resolve_import_records is subsumed by the arena-lifetime erasure already inside path_with_pretty_initialized (bundle_v2.rs:5776-5777). The get_or_put map key in run_resolver is captured before path_primary is shadowed by the re-interned path, so the map entry stays keyed on the original bytes — unchanged from before. The write-back to file_map_result.path_pair.primary in resolve_import_records was already present; only run_resolver gains it, mirroring the pattern at bundle_v2.rs:2641. The new tests use tempDir, assert deep equality of { outputs: [[path, text]], inputs } across resolution variants (covering hash, text, and metafile in one comparison), and include a positive content check so a matching-but-wrong pair would still fail.

Deferring because the user-visible behavior change to metafile keys and chunk hashes — though clearly a bugfix and clearly the right convention — is the kind of thing a maintainer should sign off on rather than an automated review.

[robobun]

On the behavior-change question raised in the review, to make the sign-off concrete:

• The narrower fix would be to keep the absolute key as the display path and only add the missing write-back in run_resolver. That also makes the declining plugin a no-op, but it keeps files imports as the one kind of file-namespace module displayed by its absolute path: entry points from the same files map, plugin-resolved paths and disk files are all cwd-relative, so a fully in-memory build would still list ../app/entry.js next to /app/lib.js, and overriding a disk file through files would still print the build machine's absolute path into the bundle and change the chunk hash.
• This PR takes the other option: files imports use the same display path as every other module. The only observable difference for builds that did not use a plugin is that metafile keys and filename comments of files imports change from the absolute key to the same relative form their entry points already had. Nothing else (sourcemaps, output naming, resolution) keys off this path.

Unrelated to this diff: the red cargo clippy and mordant checks are main failing to compile at 2f5c180 (PostgresSQLConnection.rs calls report_active_exception_as_unhandled, which 5a34f8d removed). bun_bundler itself passes clippy in the same job.

[robobun]

Heads up: #38635 removes the files: branch of resolve_import_records that this PR edits, routing a file map hit through the disk tail; it keeps the display path as the map key behind an is_in_memory check at the path_with_pretty_initialized call. If that lands first, this change in resolve_import_records becomes deleting that check and calling path_with_pretty_initialized unconditionally; the run_resolver and LinkerContext parts are unaffected.