Repository navigation
dev server: give the page of an HTML route one file of a package with "main" and "module" - #46994
Conversation
… "main" and "module" An import of a package with "main" and "module" (or "jsnext:main") and no "exports" goes to the "module" file. A `require()` of the package goes to the "main" file. A build that has both files binds every import to the "main" file. The dev server prints each file alone, with the id that the import resolved to, so a page ran two copies of the package. The route trace now decides this when it builds the page of an HTML route. A route forwards the "module" file to the "main" file when its walk of the client graph reaches both files and a file that imports the package by name. The page chunk then has no module for the "module" file, and the chunk config names the forward. The module loader sends an id from the code of a module one step through that map. No import record moves, and no file is bundled again.
|
Status of #46994. How the problem reproduces on the base (d79de18):
|
|
Updated 11:24 AM PT - Oct 26th, 2026
✅ @robobun, your commit ba13605fc4795fbdb3890c58114a6f6f6451c4e6 passed in 🧪 To try this PR locally: bunx bun-pr 46994That installs a local version of the PR into your bun-46994 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the &'static [u8] paths kept in SecondaryPaths (src/bundler/secondary_paths.rs): the only consumers, mark_pair_edge and refresh_pairs in src/runtime/bake/dev_server/incremental_graph.rs, copy them into Pair.main with Box::from while the BundleV2 that owns the bytes is still alive in finalize_bundle, so no borrowed path outlives the bundle. Also examined and ruled out: the hot-update path at finalize.rs:1071 for a page that newly requires a "main" file another route already bundled, and the hmr-module.ts:701 edit path for a workspace package's "module" file.
Extended reasoning...
The change adds per-route forwarding of a package's "module" file to its "main" file in the Bake dev server: a new bundler-side SecondaryPaths map and HAS_SECONDARY_PATH import-record flag, pair/edge refcount bookkeeping and a second trace walk in IncrementalGraph, a forwards table in the page chunk config, and id rewriting at three load sites in hmr-module.ts, with ~1,100 lines of tests. It touches no auth, crypto or injection surface. Four confirmed findings about hot-update staleness and pair refresh are posted inline, and the change is a large, design-heavy modification of the dev server graph, so approval is not appropriate; this note only records what else was checked.
Findings marked 🟡 are optional suggestions and need no follow-up push.
| if goal != TraceImportGoal::FindErrors && !graph.pairs.is_empty() { | ||
| graph.find_forwards(gts); | ||
| if !gts.forwards.is_empty() { | ||
| graph.rewind_trace(gts, mark); | ||
| graph.trace_imports(html.bundled_file, gts, goal)?; |
There was a problem hiding this comment.
🟡 (optional) A viewer of an HTML page that forwards a package's "module" file loses a hot update and gets a full page reload when an edit newly imports a file that only that "module" file reached. The second walk at trace.rs:97 drops the "module" file's exclusive subtree from the page chunk, but the hot update chunk at finalize.rs:1071 ships only re-parsed files, so the page has no record for the cached file. Fix: a hot update must give the page every file its route newly reaches that the page's chunk lacks (or the server must force a reload instead of sending the update), for every file left out by a forward, not only the cited case.
Why this was flagged
Page P imports "pkg" and require()s it, so trace_all_route_imports forwards pkg/es/index.js to pkg/lib/index.js (trace.rs:94-97) and the second trace_imports walk never visits pkg/es/util.js, which only pkg/es/index.js imports; the page chunk from take_js_bundle (render.rs:708) has no record for it. The user then edits a file B of the page to add import "pkg/es/util.js". In that bundle pkg/es/util.js is not stale, so is_file_cached (bundle.rs:87) returns Some and it is not re-parsed; the HMR chunk at finalize.rs:1071 contains only B. In the browser replaceModules reloads B; parseEsmDependencies (hmr-module.ts:596) calls loadModuleAsync for pkg/es/util.js, forwards has no entry for it, registry and unloadedModuleRegistry have none, so throwNotFound (hmr-module.ts:918) throws; the rejection reaches hmr-runtime-client.ts:107 which logs it and calls fullReload(). On the base branch the single walk includes pkg/es/util.js in the page chunk, so the same edit applies in place and page state survives.
Verification: The second walk maps the "module" file to the "main" file (incremental_graph.rs:1429-1437), so the page chunk has no record for pkg/es/util.js. finalize.rs:1071-1079 ships only re-parsed files, so the update carries B alone. On the base branch the same edit hot-updates in place. The new tests only exercise files new to the graph (bundle.test.ts:2050-2053, 2087-2098), so this case is untested.
There was a problem hiding this comment.
Not changed in this PR. A hot update has only the files of its bundle, also on the base. An edit that imports a cached file that the page does not have ends in "Failed to load bundled module" and one page load. A file that only the left-out "module" file imported is now one more such file.
The repair is the same for every cached file: the server must know the files of the page. So it is not part of this change. The PR notes list the case.
| ) { | ||
| for (module, main) in secondary_paths.iter() { | ||
| let Some(module) = self.bundled_files.get_index(module) else { | ||
| continue; | ||
| }; | ||
| let Some(pair) = self.pairs.get_mut(&FileIndex::init(module as u32)) else { | ||
| continue; | ||
| }; | ||
| if *pair.main != *main { | ||
| pair.main = Box::from(main); | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🟡 (optional) Developers who rename "main" in a package.json and re-save the file that requires the package still get two copies of the package on the dev page, and stay there until they also re-save a file that imports the package by name. refresh_pairs at incremental_graph.rs:1343-1356 only updates a Pair whose "module" file was resolved in this bundle, and a require() resolution carries no secondary path (resolver.rs:5799), so pair.main keeps the old file. find_forwards at incremental_graph.rs:1366-1372 then looks up the old main, which no edge reaches, and forwards nothing. Fix: re-read the "main" path for every live Pair from the resolver (or from the new main file's package.json) on each bundle, not only for "module" files resolved this bundle.
Why this was flagged
A route forwards pkg's "module" file to "main" (Pair{main: old path}). The user edits pkg/package.json to point "main" at another file and saves the file holding require("pkg"). Only that importer is rebundled; its require record resolves to the new main with secondary None (resolver.rs:5799), so SecondaryPaths has no entry for the "module" file, and refresh_pairs (incremental_graph.rs:1343-1356) skips the Pair. On the next page load find_forwards (incremental_graph.rs:1366-1372) resolves pair.main to the old file, which is in bundled_files but unvisited, so no forward is pushed; the page runs the "module" file plus the new "main" file. On the base the page also runs two copies, but this change promised one copy per route and leaves the fix dependent on a save of a different file. Remedy: refresh every live Pair's main path on each bundle from the package's current package.json, or key the pair by package rather than by resolved file.
Verification: SecondaryPaths::note (src/bundler/secondary_paths.rs:33-35) returns early when resolved.path_pair.secondary is None, and the resolver only fills secondary for non-Require kinds (src/resolver/resolver.rs:5799, :5818). So a rebundle of only the requirer yields a SecondaryPaths with no entry keyed by the "module" file.
There was a problem hiding this comment.
Not changed in this PR. A file keeps its resolution until it is bundled again, and only an import by name carries the secondary path. So the pair gets the new "main" file when a file that imports the package by name is bundled again (refresh_pairs). The test "a route binds a package to the main file that the package.json names now" pins that.
A read of package.json for each pair on each bundle would add work to every bundle of an app with such a package. The PR notes list the case.
…les that it runs The stylesheet list of a hot update was a walk with the forwards that the route has now. A page that was open when its route started to forward a "module" file lost the stylesheets of that file and still ran the file. An HTML route now keeps the forwards of the client bundle that it built last, and the stylesheet list of a hot update walks with those. The mark of a route walk has no count of failures: a walk that is taken back is never a walk for errors. The mark is `Copy`, so clippy accepts it by value.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Each comment that the two commits before this one added under src/ is now one line. No code changes.
|
Changes since the review, in 4acc37e and ba13605:
Not changed: the two optional findings. One is a cached file that a hot update does not carry. The other is a package.json that names another "main" file. Their threads have the reasons, and the PR notes list both cases. |
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked the pair_edges refcount in incremental_graph.rs — every marked edge is unmarked either in the post-process_chunk_dependencies loop (seen, not pair) or in free_edge, and a reused free-list slot starts cleared — and the 'static secondary-path text stored by SecondaryPaths::note (both resolve sites in bundle_v2.rs bind the record to an Fs::Path<'static> from the resolver, so the expect in mark_pair_edge holds for every flagged record). The dead TraceMark.failures field from the earlier round is gone, and the hot-update stylesheet walk now reads the route's recorded forwards.
Extended reasoning...
The change adds per-route "module"-to-"main" forwarding to the Bake dev server across the bundler resolve sites, the client IncrementalGraph, the route trace, and the hmr-module loader, with ~1,100 lines of new tests; it touches no auth, crypto, or input-parsing surface. Two confirmed findings are posted inline; this note records only the memory-safety and refcount checks that were examined and ruled out, plus what changed since the previous review round.
Still open from earlier reviews (2):
- Unresolved: 2 minor or pre-existing.
| .trace_imports(html.bundled_file, gts, goal) | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🟡 nit (optional): a dev page open in two tabs of the same HTML route can still lose or gain stylesheets on a hot update when the tabs were loaded with different forwards. html.forwards is one vector per route, overwritten by every generate_client_bundle (render.rs:584), but each open tab keeps the config.forwards of the chunk it loaded. trace_open_page_imports at trace.rs:40 walks with the latest vector for every viewer, so an older tab still running the "module" file gets the CSS list of the newer one. Fix: make the hot-update stylesheet walk reflect every open page of the route, e.g. keep the set of forwards vectors of live viewers (or skip forwards when they differ) rather than one per route.
Why this was flagged
Tab 1 loads route / whose page imports "dual" only; the chunk has no forwards and the page runs node_modules/dual/esm.js, which imports module.css. The user edits other.js to add require("dual"): tab 1 hot-updates in place and keeps running esm.js, and the route's client bundle is invalidated…
Verification: nit — triggers when two viewers of one HTML route were loaded on either side of an edit that changed the route's forwards, and a later edit adjusts edges. Html::forwards is a single Vec per route. trace_open_page_imports walks with html.forwards (trace.rs:37-39) and emits ONE css list per route id, broadcast to every viewer. The base branch had no forwards, so this inconsistency is new.
There was a problem hiding this comment.
Not changed in this PR. The case needs two open pages of one route that loaded on the two sides of a change of the forwards. One list per route can serve only one of them, and the server does not know which forwards each open page has. The PR notes list the case.
A reload of the open pages of a route when its forwards change removes the case, because no page then keeps older forwards. That reload is the follow-up that the notes name (see the other thread).
| if had_adjusted_edges { | ||
| ctx.gts.clear(); | ||
| dev.client_graph.current_css_files.clear(); | ||
| // SAFETY: `trace_all_route_imports` does not mutate `route_bundles`; | ||
| // SAFETY: `trace_open_page_imports` does not mutate `route_bundles`; | ||
| // shared, call-scoped reborrow. | ||
| let route_bundle_ref = unsafe { &*route_bundle }; | ||
| dev.trace_all_route_imports(route_bundle_ref, ctx.gts, TraceImportGoal::FindCss)?; | ||
| dev.trace_open_page_imports(route_bundle_ref, ctx.gts, TraceImportGoal::FindCss)?; |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: A developer who adds a require() of a package the open page already imports gets two copies of it after the hot update, until a manual reload that nothing prompts. The base fails the same way (pre-existing), but this is the hazard the change sets out to fix. At finalize.rs:963-980 the route is hot-updated with the stale html.forwards; the HMR chunk carries no forwards, so the page keeps the "module" file while the new require loads "main". Fix: when a bundle changes what an html route forwards (compare the route's new find_forwards with html.forwards), list the route for a reload instead of a hot update, so an open page never runs both files.
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
A page of an HTML route is open with index.js doing import moment from "moment" and no file requiring it, so the chunk has no forwards (hmr-module.ts:39 reads config.forwards once at load). The user saves other.js adding require("moment"). src/runtime/bake/dev_server/finalize.rs:963-980 only recomputes the stylesheet list with the old html.forwards, and the HMR chunk at finalize.rs:1074 ships other.js plus the newly parsed moment "main" file with no forwards. In the browser loadModuleSync at hmr-module.ts:320 finds no forward for the main id and loads it as a second instance, while index.js keeps the "module" instance. The page is now in the dual-package state (moment.tz is not a function) and stays there: no update puts the route in List 1 (finalize.rs:840-848), which is what makes hmr-runtime-client.ts:198 call performRouteReload/fullReload. The base branch always runs two copies, so the state is not worse than base, but the PR's own page load would give one copy.
Verification: pre-existing; acknowledged in diff. /home/claude/bun/src/runtime/bake/hmr-module.ts:39 reads the map once from the initial-response config; the hot-update chunk at /home/claude/bun/src/runtime/bake/dev_server/finalize.rs:1071-1079 has forwards: &[], and finalize.rs:963-969 only recomputes CSS via the stale html.forwards. test/bake/dev/bundle.test.ts:2050-2060 pins exactly this.
There was a problem hiding this comment.
Not in this PR. This PR makes a page load bind one file. It leaves an open page as the base has it until its next load (Downsides, and "What still differs from a build" in the notes). The reload that you describe is the follow-up that the notes name. It is more than the compare:
- The compare must run before List 1 is written. So each save that adds or removes an import walks a route with such a package one more time.
- List 1 reloads a page by the route of its socket. A page whose socket has the route of another URL (for example
/admin/*beside/*) then loads again for the other route and not for its own. dev server: tell a page that loaded an outdated client bundle to reload #42041 works on the reload of a page with an outdated client bundle. - The reload of List 1 prints "Server-side code changed, reloading!". That is not the reason here, and a true reason needs a change in
hmr-runtime-client.ts.
If a maintainer wants it in this PR, I can add it here.
|
Second review round: no code change. Both new findings describe one follow-up: load the open pages of a route again when the forwards of the route change. Their threads have the reasons why that reload is not in this PR. The notes in the PR body list both cases:
The two threads from the first round stay open for the same kind of reason: the cases are listed in the notes and are not changed here. |
Related to #46649 (the base branch).
Problem
import x from "pkg"runs the "module" file ofpkgandrequire("pkg")runs "main". moment with moment-timezone throwsimport_moment.default.tz is not a functionsince v1.4.0.BundleV2::scan_for_secondary_paths(src/bundler/bundle_v2.rs:2618) gives a build one copy. The dev server never runs it.Fix
DevServer::trace_all_route_importsdecides per HTML route. A route that reaches both files, and imports the package by name, forwards "module" to "main". The page chunk omits that file, and the loader (hmr-module.ts) sends ids through the forwards in the chunk config.test/bake/dev/bundle.test.ts(21 new),test/bake/dev-and-prod.test.ts(10 new),test/bake/dev/ecosystem.test.ts(1 new). Also all oftest/bake/.Background
PathPair: primary is the "module" file, secondary is "main".Downsides
Notes
Questions for a maintainer.
Bun.serverun from source builds each HTML route alone, also withdevelopment: false, and gives the same result. A build ahead of time is one unit: onebun buildof two pages gives the page that only imports the package the "main" file, because the other page requires it. Is per route the wanted unit for the dev server?bun buildmoves it to "main" with the package if an import by name reaches the file first, and leaves both on "module" if the import by path does. This change moves it in each order, because a forward is keyed by the file. If the wanted rule is the one of esbuild, the forward needs a key per import record.bun buildbinds such a package by parse order. Not changed here. Only the import record that first reaches a file keeps the secondary path (bundle_v2.rs:7396,:3021). With an importer by path and an importer by name in sibling files, one input gives two outputs: 16 of 40 builds on the base had two copies (byName: "module", required: "main"), and 5 and 14 of 40 on this branch in the two entry orders. moment has this shape:moment/dist/locale/fr.jsimports../momentby path. So a build of a page with moment, moment-timezone and a locale still depends on parse order, where the dev page now does not. The repair is a change of the bundler on main with its own tests. No issue tracks it.Origin. No user reported this. It was found in the review of #46649. #35447 (v1.4.0) gave
require()the "main" file of a package with "jsnext:main", so moment 2.29.4 with moment-timezone 0.5.48 works under HMR on 1.3.14 and throws on 1.4.x. For "module" the gap is older: #22231 gavebun buildthe rule in its present form and said "The thing to watch out for with this PR is how it impacts the dev server". esbuild 0.7.0 introduced the rule and rejected "require()gets main,importgets module" because "the same package would be loaded into memory more than once". #17633 (moment-timezone in the dev server) was another failure, which #17954 repaired.Real packages. moment 2.29.4, moment-timezone 0.5.48 and
moment/locale/frin one page: the base throwsTypeError: import_moment.default.tz is not a function. This branch prints the time in the zone and the French month name. The test inecosystem.test.tsuses the installedimmutable5.1.3:require("immutable").Map() instanceof Mapis false on the base.Workaround on released versions. Pin the package to one file in tsconfig.json:
{ "compilerOptions": { "paths": { "pkg": ["./node_modules/pkg/main-file.js"] } } }.Why it is based on #46649. A forward gives the importer the real "main" module. When "main" is CommonJS from tsc or Babel (
exports.__esModule), the default import depends on the module type of the importer. The loader gets that rule in #46649. The test "the default import of a tsc package with main and module follows a require" fails without it.The contract, per HTML route.
requirebelow the "module" file counts, as it does in a build.sideEffects: falsebarrel that both routes import (see below).How it works.
SecondaryPaths::note(src/bundler/secondary_paths.rs) runs for each import record that the dev server resolves for a client file. If the resolver returned a secondary path, it setsImportRecordFlags::HAS_SECONDARY_PATHand keeps the path for this bundle, by primary path.IncrementalGraphkeeps one bit per edge (pair_edges): an import behind the edge had the flag.pairshas one entry per "module" file with such an edge: the path of its "main" file and the count of those edges. An entry goes when its count is 0. The graph gets no slot for a "main" file that no file names.trace_all_route_importswalks the route as before. Ifpairsis not empty,find_forwardsreads the forwards off that walk intoGraphTraceState. Only a route with a forward walks a second time, from a mark thatrewind_tracegoes back to.forwards: { "<module id>": "<main id>" }in its config. A route with no forward gets no key.hmr-module.tssends an id throughforwardsinloadModuleSyncandloadModuleAsyncwhen the load has an importer, in the dependency list of an ES module, and inimport.meta.hot.accept(specifier).route_bundle::Htmlkeeps the forwards of the client bundle that the route built last. The stylesheet list of a hot update walks with those (trace_open_page_imports), so an open page gets the stylesheets of the files that it runs.Measurements. Release builds of the base (d79de18) and of this branch. Instruction counts are user-mode
instructionsfromperf_event_open, median of 11 interleaved runs, on a generated app of 2,000 modules. "Such imports" are 6,000 imports by name of 300 packages with "main" and "module".size)bake.client.js(runtime of each dev page)bake.server.jsbake.error.jsbun build, no such importbun build, such importssize_of:Edge32,File56,ImportRecord96 andTempLookup8 do not change.IncrementalGraph280 to 384,BundleV22,576 to 2,608,GraphTraceState56 to 80,RouteBundle72 to 96.const SIDEguards).bun builddoes not reach the new code: both call sites are inside the dev server branches of the bundler.Self-review, not done.
Tests. 30 mutations of the new code: 27 fail a test. The other 3 change only memory, speed or a state that no caller reaches (the removal of an entry at count 0, the check of the "module" file before the path lookup, the importer condition in the synchronous loader). A differential check with random edits of 8 files, 3 routes and 3 packages (imports by name and by path,
require,import(), syntax errors, batches) compares each page chunk with the contract above: 160 sequences pass. It is not in the tree.What still differs from a build.
require()orimport()of asideEffects: falsebarrel asks for every export in the dev server, the requests stay until the server starts again, and they are per dev server. So the client graph of a route can reach a file with therequirewhere a build leaves that file out. Example:libis such a barrel,lib/b.jsrequires the package, and the page hasimport { kind } from "pkg"andrequire("lib").a. This branch gives "main". The base andbun buildgive "module" (the base page also runs "main"). The first request of another route can add such a file. dev server: keep a barrel's requests across builds, or never defer it again #46648 owns barrel requests. No issue tracks this case.Reloaded."use client"file and loses the directive, its HTML routes run two copies until the server starts again, as on the base.HAS_SECONDARY_PATHinImportRecordFlags: 12 open PRs add a flag at1 << 19(2026-10-26), and one of them lands first and keeps that bit. This PR takes1 << 26at its next push. Check first that main and no other open PR use that bit by then. Two different flags with one bit compile with no error and no warning, and each then reads as the other.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bake/dev-and-prod.test.ts