Skip to content

FileSystemRouter: take each route's path from Resolver::real_path_of, not from Entry.abs_path - #44612

Draft
robobun wants to merge 1 commit into
robobun/a21c995d/watch-dir-event-lazy-abs-pathfrom
robobun/0a857bc4/fsr-route-path
Draft

robobun wants to merge 1 commit into
robobun/a21c995d/watch-dir-event-lazy-abs-pathfrom
robobun/0a857bc4/fsr-route-path

Conversation

@robobun

@robobun robobun commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • A route's filePath depends on earlier resolver calls. Through a symlinked dir: <root>/real/pages/b.tsx alone, <root>/link/pages/b.tsx after Bun.build of that file. reload() changes it again.
  • A router built first makes Bun.build read tsconfig.json from the real directory's parents.
  • Entry.abs_path (src/resolver/fs.rs) is one slot of the shared directory cache. The resolver stores the first import's spelling. Route::parse (src/router/lib.rs) stored the kernel realpath and reused what it found.

Fix

Background

Downsides

  • A file deleted after the resolver cached the directory listing is a route of a new router. The constructor threw ENOENT opening route.
  • On Linux, a route file imported in another letter case loaded after a router scanned its directory. Now it fails.
  • On a case-insensitive file system filePath keeps the case of dir, not of the disk.
Notes

Decisions for a maintainer

  1. This goes one step past Remove get_fd_path: derive paths from cwd and what was opened, not from fds #38365, which applies the same path rule but still reads and writes Entry.abs_path.
  2. A mis-spelled dir on a case-insensitive file system: filePath follows dir, as Bun.resolveSync does. The old value had the spelling on disk. If the same file is also imported under the spelling on disk, import(filePath) loads a second copy of the module.
  3. A new router reads the directory listing that the resolver cached and can be older than the disk. This was so before, but a deleted route file then made the constructor throw. reload() reads the disk.

Settled on main: a dangling symlink

Main skips a dangling symlink in the router and in the resolver since #44968 (EntryKind::Dangling, from #38756), and the base of this PR includes that commit. The router skips the entry before Route::parse runs, so this PR does not change what happens to the link. One new test pins that the router lists the link only if the resolver resolves it. It passes on the base too.

#34614 has a case that expects ENOENT opening route for a dangling symlink. Main no longer reports that error for a dangling link, and this PR removes it for every file.

What still depends on order

filePath is now independent of Entry.abs_path. It is the resolver's real path, so it follows the resolver in two cases:

A directory symlink that is created or retargeted after its parent was listed gives the old answer until the cache is busted. The router stays a reader of the resolver's cache here.

Reproductions on released 1.4.3 (Linux x64)

// <root>/link -> <root>/real, pages = <root>/link/pages
Bun.resolveSync(pages + "/b.tsx", root); // or Bun.build({ entrypoints: [pages + "/b.tsx"] })
const router = new Bun.FileSystemRouter({ dir: pages, style: "nextjs" });
router.match("/a").filePath; // <root>/real/pages/a.tsx
router.match("/b").filePath; // <root>/link/pages/b.tsx, src "../../link/pages/b.tsx"
  • Plain dir with pages/c.tsx linked to ../shared/e.tsx: after a resolve the router reports <root>/pages/c.tsx, and reload() changes it to <root>/shared/e.tsx.
  • No symlink: Bun.resolveSync(pages + "/B", root) (wrong letter case) makes the router report <root>/pages/B.tsx, a file that does not exist on Linux.
  • Reverse: proj/tsconfig.json sets jsxImportSource: "custom-src" and proj/link points outside proj. Bun.build of proj/link/pages/b.tsx imports custom-src/jsx-dev-runtime. With a router over proj/link/pages built first it imports react/jsx-dev-runtime.
  • --preserve-symlinks: the router reports the real path with src ../../real/pages/a.tsx, and Bun.resolveSync of the symlinked path returns the real path after the router ran.

No user has reported these. The bug was found by a read of the code.

Measurements (debug builds of the base and of this PR, 100 and 1100 route files, per-route difference)

base this PR
syscalls per route, constructor (ptrace count) openat 1, readlink 1, close 1 0
syscalls per route, constructor + reload() 2 each 0
DirnameStore appends per route, constructor (gdb breakpoint count) 2 1
appends, two routers over one cached directory 3 2
appends, constructor + reload() 4 2
appends, resolver resolved every route first 2 2

The three syscalls are a few microseconds per route. They are not the reason for the design. DirnameStore is never freed and has a fixed number of slots, so one slot per route per load doubles the number of reloads before it is full. A route below a symlinked subdirectory, and a symlinked route file, still take a slot for the name.

strace, perf, valgrind and bloaty are not installed here. The syscall count comes from a small ptrace counter and the append count from gdb breakpoints on BSSStringList<4096, 129>::do_append. Binary size is not measured: the change removes code and adds no host function.

Other behavior changes

  • Under --preserve-symlinks, filePath is the path the resolver returns in that mode. src is then a.tsx instead of ../../real/pages/a.tsx.
  • A file name with a backslash on POSIX made the constructor throw ENOENT opening route: <root>/pages/a/b.tsx, because the old join treated the backslash as a separator. It is now listed as <root>/pages/a\b.tsx.
  • The router no longer needs /proc to load routes that are not symlinks.

Related open PRs

Not changed

bun test's scanner still uses Entry.abs_path as its "already seen" mark, and the six resolver sites still fill it. Entry::symlink() still reads the cache without the mutex while finalize_result stores cache.symlink under it. The router now reads through the same accessor as the resolver, so a fix there covers both. scopeguard stays in src/router/Cargo.toml although the crate no longer uses it, because dropping it also changes Cargo.lock.

Not run locally

Windows and macOS. cargo check passes for x86_64-pc-windows-msvc and aarch64-apple-darwin on the touched crates. One new test needs a file system that ignores letter case and is skipped on Linux.


no test proof · iteration 2 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/util/filesystem_router.test.ts

@github-actions github-actions Bot added the claude label Oct 6, 2026
@robobun

robobun commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on Bun 1.4.3 (Linux x64) and on a debug build of the base branch:

  • Put the routes directory under a symlink. Call Bun.resolveSync or Bun.build on a route file through the symlinked path. Then create the router. filePath is the symlinked path for that file and the real path for the others.
  • In a plain routes directory with a symlinked route file, do the same. filePath is the link's path, and reload() changes it to the target.
  • Create the router first. Bun.build then reads tsconfig.json from the parents of the real directory.

The new cases in test/js/bun/util/filesystem_router.test.ts cover these: 10 of 15 fail on the base and all pass with this change (one is skipped on a file system that keeps letter case apart).

This PR is stacked on #41617 and stays a draft until that PR merges and a maintainer answers the three decisions in the notes of the description.

@robobun

robobun commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 3:08 PM PT - Oct 18th, 2026

✅ @robobun, your commit e55f201a988174d7bd2673b0ee6726ed61754906 passed in Build #128470! 🎉


🧪   To try this PR locally:

bunx bun-pr 44612

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

bun-44612 --bun

@robobun robobun changed the title Stop using the resolver's Entry.abs_path in FileSystemRouter and the hot reloader FileSystemRouter: take each route's path from Resolver::real_path_of, not from Entry.abs_path Oct 6, 2026
@robobun
robobun changed the base branch from main to robobun/a21c995d/watch-dir-event-lazy-abs-path October 6, 2026 11:51
@robobun

robobun commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

cc @dylan-conway: this uses the path rule of #38365 for Route::parse and goes one step further, so it needs a maintainer's answer on three points before it leaves draft. They are under "Decisions for a maintainer" in the notes of the description. In short:

  1. The router no longer reads or writes Entry.abs_path at all (Remove get_fd_path: derive paths from cwd and what was opened, not from fds #38365 keeps both).
  2. On a case-insensitive file system filePath keeps the spelling of dir, not the spelling on disk.
  3. A new router can list a route file that was deleted after the resolver cached the directory listing. Before, the constructor threw in that case.

An earlier version of this comment had a fourth point, a dangling symlink in the routes directory. #44968 settled it on main: the router and the resolver skip the link.

The PR is stacked on #41617, because --hot found a file saved by rename through the same field.

… not from Entry.abs_path

Entry.abs_path is a slot of the shared directory-entry cache. The
resolver fills it with the spelling of the first import that reaches a
file. The router filled it with the kernel realpath and reused any
value it found, so a route's filePath depended on whether the resolver
saw the file first, and a router built first changed the path the
resolver returned for later imports.

Resolver::real_path_of is the rule that finalize_result and
dir_info_uncached applied inline: the target of a symlinked entry, else
the real path of its directory. The router builds each route's path
from it. It no longer reads or writes Entry.abs_path, and it no longer
opens each route file. The path and the route name share one interned
string when the name is the tail of the path.

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