Repository navigation
Conversation
|
Updated 2:49 PM PT - Oct 25th, 2026
✅ @robobun, your commit 4115e85b0bca4fd4f1e2a9bf6be6f1497bc1d1fb passed in 🧪 To try this PR locally: bunx bun-pr 45408That installs a local version of the PR into your bun-45408 --bun |
|
Status: ready for review. Reproduction: run |
|
#38605 now covers only the Two notes for this PR:
The two testsasync function expectManifestWithPlugin(specifier: string, plugin: BunPlugin) {
const result = await Bun.build({
entrypoints: ["/app/server.js"],
target: "bun",
files: {
"/app/server.js": `import page from "${specifier}"; export default page;`,
"/app/page.html": pageHtml,
"/app/page.css": pageCss,
"/app/client.js": clientJs,
},
plugins: [plugin],
});
const server = await outputText(result, "server.js");
expect(server).not.toContain(`from "${specifier}"`);
const [manifest, ...extra] = manifestsIn(server);
expect(extra).toBeEmpty();
expect(manifest.index).toMatch(/page\.html$/);
expect(manifest.files.map(f => f.loader).toSorted()).toEqual(["css", "html", "js"]);
expect(result.outputs.filter(o => o.path.endsWith(".html"))).toHaveLength(1);
const { path: jsPath } = manifest.files.find(f => f.loader === "js")!;
const client = await outputText(result, jsPath);
expect(client).toContain(clientJs);
expect(client).not.toContain("// @bun");
}
test("imported through an onResolve plugin that declines the import", async () => {
await expectManifestWithPlugin("./page.html", {
name: "decline-relative-imports",
setup(build) {
build.onResolve({ filter: /^\.\// }, () => undefined);
},
});
});
test("imported through an onResolve plugin that returns the in-memory file's path", async () => {
await expectManifestWithPlugin("app:page", {
name: "resolve-to-in-memory-page",
setup(build) {
build.onResolve({ filter: /^app:page$/ }, () => ({ path: "/app/page.html" }));
},
});
});Merge order: #38605 first. This PR then rebases over it, and the old |
8ee1185 to
77fc3f3
Compare
…solve plugins An import that matched an onResolve filter did not go through resolve_import_records. A declined import went through run_resolver, a hand copy of that pass. An answered import went through a third module maker in on_resolve. Both missed rules of the shared pass: tsconfig JSX settings, side effects, the files option, HTML imports, loaders of import attributes, the hash of an asset in the name of its importer. on_resolve now sends the one record back through resolve_import_records, process_resolve_queue and patch_import_record_source_indices, with the helper that barrel un-deferral already used. run_resolver and enqueue_parse_task are gone. A declined entry point of Bun.build takes the code of an entry point that no plugin matched, and keeps its place in the order of the outputs. A file that an onResolve callback names still gets the options of the build and no tsconfig.json or package.json settings. Co-authored-by: ankit9999999999 <69192484+TechDev009@users.noreply.github.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
…wers are back Each onResolve answer ran the barrel seeding pass over every import record of the importer. A file with R plugin-routed imports paid R passes over R records. The pass now runs once, when the last answer of the file is back.
…record a module An import record that is at the onResolve plugins keeps the specifier as written in `path`. `patch_import_record_source_indices` matches `path` to the modules of the build. So a specifier that is an absolute path got the module with that path: before the plugins answered, and again after they answered with a module that the build has already. - `ImportRecordFlags::AT_ON_RESOLVE_PLUGINS` marks the record from the dispatch to the answer. The patch leaves a record with the flag alone. - `resolve_records_of_parsed_file` does not patch the answered record when the pass gave it a module that exists. An external answer is now external also when the specifier names a module of the build. A redirect file (`module.exports = require(x)`) whose require() went to the plugins no longer sends a later importer to the module that the specifier names.
77fc3f3 to
4115e85
Compare
There was a problem hiding this comment.
I reviewed the latest push and didn't find any new bugs; commit 4115e85 addresses the answered-record overwrite flagged in the previous round. Because this is a ~2,000-line rewrite of the bundler's onResolve pipeline with user-visible changes to error text, output names and hashes for plugin users, a human look is still worthwhile.
What was reviewed:
- The AT_ON_RESOLVE_PLUGINS lifecycle: inserted at dispatch on the graph's own record, removed on both the answer path in resolve_records_of_parsed_file and the external early-return in on_resolve, and honored by the patch closure in patch_import_record_source_indices.
- The has_module guard: skipping the trailing patch also skips the redirect put into path_to_source_index_map, but a later importer then lands on the redirect row and the reachable-file visitor follows redirects, so the new redirect tests (existing module and external answer) exercise the real behavior.
- The rewritten "answer wins" tests: they now run the output and assert stdout, stderr and exit code together, and the external variant checks mock.js is absent from the bundle.
Extended reasoning...
The PR replaces the bundler's bespoke onResolve fall-through resolver and inline parse-task construction in src/bundler/bundle_v2.rs with a late re-entry into the shared resolve_import_records / process_resolve_queue / patch pass, adds an AT_ON_RESOLVE_PLUGINS import-record flag in src/ast/import_record.rs, trims src/bundler/barrel_imports.rs to a call into the shared pass, and adds tests across five files. It touches no auth, crypto or input-parsing surface; the risk is bundler correctness for plugin users (which module an import links to, entry-point ordering, dev-server HTML imports). The latest commit changes the flag lifecycle and the patch skip that the previous round's inline finding targeted, and the hunt ran dry on this version. The size of the rewrite and the documented user-visible output changes are why a human reviewer should still look rather than this being approved automatically.
Fixes #19951, fixes #22838
Problem
onResolvefilter builds differently, also when every callback returns nothing. Example:Could not resolve: "react/jsx-dev-runtime".run_resolver(src/bundler/bundle_v2.rs:2678on main) resolves a declined import as a hand copy ofresolve_import_records.on_resolvemakes the module of an answered import. Both miss rules of the shared pass, such as the tsconfig JSX settings.Fix
on_resolvesends a declined or answered import back throughresolve_import_records. A declinedBun.buildentry point takes the code of an unmatched one.run_resolveris gone.AT_ON_RESOLVE_PLUGINS: only their answer gives it a module. A file that a callback names gets no tsconfig or package.json settings, as in esbuild.Background
onResolveanswer runs as a task on the bundle thread, after the importer's records are on the graph.resolve_import_recordsis the pass for every import record of a parsed file.patch_import_record_source_indicesthen matches each record'spathto a module.run_resolver(bundler: keep the resolver's JSX and side effects for an import that onResolve declines #43190, bundler: keep the resolver's side effects when onResolve returns nothing #41380, bundler: resolve an import a dev-server onResolve plugin declined the way an unmatched one resolves #44471, bundler: resolve in-memory entry points when an onResolve plugin declines them #38600): the copy stays.Downsides
.textis 2,304 bytes smaller.Notes
Covers these PRs
onResolveroutes that bundler: resolvefilesimports through the shared steps (HTML import manifest, url assets) #38605 (open: HTML import manifest) no longer has. bundler: resolvefilesimports through the shared steps (HTML import manifest, url assets) #38605 now changes only thefilesbranch ofresolve_import_records. One of its former tests is here: an answer that names an in-memory page. The other one, a declined import of an in-memory page, needs thefilesbranch of bundler: resolvefilesimports through the shared steps (HTML import manifest, url assets) #38605. Here that import builds as it does without a plugin.Not taken from #43190: settings for a file that an onResolve callback names
sideEffectsin the onResolve result andbuild.resolve(). Bun has neither.Could not resolve: "./c.js". The testonResolve can write a file after it named another file of the directorycovers this.file that onResolve names is compiled with the options of the buildpins the behaviour of today.Visible changes
Bun.buildentry point: it is found infiles, an HTML entry of a server build is a browser page, the output has the name it has without the plugin, and the outputs keep the order ofentrypoints. It is still an output when an import of another entry point made its module first (main drops it with no error).with { type }decides the loader, an HTML file in a server build is a manifest, and an HTML url asset (manifest.json) is copied.require.resolve()of a declined or answered import prints what a build without plugins prints.Browser builds cannot import HTML files., as without plugins (main: no error). When an onLoad plugin also matches the file, the import waits for onLoad. It is a module if onLoad returns one, else the same error. An import that no onResolve plugin saw does not wait. That is unchanged.bun build --app: a declined import of a stylesheet is still registered in every graph, asrun_resolverdid. A declined import of another asset is no longer registered there. An import that no plugin saw is unchanged.module.exports = require(x)) whoserequire()went to the plugins: a file that imports it later gets the module of the answer. 1.4.3 gives it the module that the specifier names.Still different from a build without plugins
import.meta.mainis then false in it. The entry points must be resolved before any file is parsed to change that.bun build --appkeeps its arm inon_resolve, with itsresolve_entry_pointcall.bun build --app: a declined stylesheet is shared between the graphs and an unmatched one is not. bake: consolidate the open robobun dev-server, HMR runtime, production build and router fixes #39488 moves that rule to the shared pass. There is no test for this here: onebun build --apprun takes longer than the test timeout on a debug build. Checked by hand with a release build: a stylesheet that a page and a client component import builds to one file with a plugin that declines every import (main:Could not resolve: "bun-framework-react/ssr.tsx").resolve_import_records, together with the Windows path form of Dev server: plugin-resolved files, accepts through imports, Windows rename-saves, JSON and CSS modules #44743.on_load: a declining onLoad callback with an asset (Bun.build: keep an import'swith { type }loader through onResolve and onLoad plugins #42286, bundler: hash the asset path into [hash] for entry-point and onLoad assets #43191).filesoption in a server build is bundled for the server (same on main, with or without a plugin). It belongs with thefilesbranch of the pass.metafile.inputs.Self-review: the concerns that stay
html_import_after_on_loadmakes a graph row by hand inon_load. The row of the page can only be made after onLoad answered.answer winsvariants pass on 1.4.3. They guard the late pass: each failed on an earlier state of this branch.export *bookkeeping inon_parse_task_completeand the barrel code of the dev server still look up thepathof a record that is at the plugins. The only effect: the barrel optimization defers less for the module with that path.debug_assert!. In a release build the flag alone keeps that true.Other design considered: an extracted per-record function that
resolve_import_recordsandon_resolveboth call. It adds one call per record to every build.After the first review round
onResolve answer wins over a module that the specifier names as written.html_manifestalso when the manifest is made after onLoad.bun build --appkeeps a row that another graph already has.After the second review round
pathof that record is still the specifier as written, and it can name another module. Test:onResolve answer wins over a module that the specifier names as written (a module that the build has already).ImportRecordFlags::AT_ON_RESOLVE_PLUGINSreplaces the reset ofsource_indexbefore the late pass.patch_import_record_source_indicesleaves a record with the flag alone, so no code sees a module on a record that is at the plugins. Visible changes 9 and 10 come from this. Tests: the(an external module)variant andonResolve answer for the require() of a redirect file.bundler_files.test.ts: an answer that names an in-memory HTML page gives a server build the manifest of the page.Other open PRs on this code: #44493 and #39488 touch
on_resolveor callrun_resolver, so they conflict. Bit ofAT_ON_RESOLVE_PLUGINSinImportRecordFlags: 12 open PRs add a flag at1 << 19(2026-10-26), and one of them lands first and keeps that bit. This PR takes1 << 24at 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.Measurements without plugins (release builds of main 179664e and this branch 4115e85, linux x64)
resolve_import_recordspatch_import_record_source_indicesprocess_resolve_queueon_parse_task_completetestb,jne) for each record that the patch matches to a module. That is one record per module of the build. The table includes it..text82,162,313 B (main 82,164,617).on_resolve12,476 B to 5,517 B.enqueue_parse_task3,152 B to gone.resolve_import_records14,340 B to 12,946 B.patch_import_record_source_indices1,613 B to 701 B.process_resolve_queue2,638 B to 2,679 B.resolve_barrel_records1,139 B toresolve_records_of_parsed_file1,934 B.on_load3,140 B to 3,567 B. New symbols:html_import_after_on_load2,911 B,html_import_manifest_ast3,582 B (split out ofgenerate_server_html_module),enqueue_unmatched_entry_point755 B.Measurements with plugins (release builds of main 116dd64 and the first two commits before the rebase, not repeated after it)
ParseTask408 B andMiniImportRecord72 B (both unchanged),ResolveImportRecordCtx64 B (main 48),PatchImportRecordsCtx40 B (main 48).openatcalls under the project, 5 modules: no plugin 19, declined 19, answered 18. Main has the same three numbers.Tests (at 4115e85)
bundler_barrel,bundler_plugin_chain,bundler_loader,test/bake/dev/css.test.ts,test/bake/dev/bundle.test.ts,test/bake/deinitialization.test.ts,test/bake/serve-plugins-dev-server.test.ts,test/js/bun/http/bun-serve-html.test.ts, and the regression test 40606.test/bundler(7,847 pass, 2 fail inbundler_bytecode_portable.test.ts: one fails on main too, the other passes when the file runs alone) and all oftest/bake(277 pass).no test proof · iteration 2 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/bundler_plugin.test.ts