Repository navigation
Conversation
`ParseResult.pending_imports` has no writer, so the block in
`transpile_source_code` that fills the module queue never runs and the
queue stays empty. Remove the field and what only it reaches:
- `AsyncModule`, `InitOpts`, `Queue::{map, scheduled, enqueue,
run_tasks, poll_modules}` and the error builders in
`src/jsc/AsyncModule.rs`. The wake handler, `on_poll` and `fulfill`
stay. `fulfill` is now a free function.
- The `AsyncModule` task tag and its two dispatch arms.
- `Error::AsyncModule`, `UnexpectedPendingResolution` and its message.
- `TranspileExtra.promise_ptr`, `ParseResult::empty`,
`ParseResult::is_pending_import`, `ScopedAstAlloc::take_state`,
`cache::Fs::reset_shared_buffer`, `JSValue::as_internal_promise`.
`QueueRunTasksCallbacks` was the one `RunTasksCallbacks` impl that set `HAS_ON_RESOLVE` and `HAS_ON_PACKAGE_MANIFEST_ERROR`, and the one that overrode `on_package_download_error_pkg`. With it gone those branches of `run_tasks` cannot run. Remove them and what only they reach: - `HAS_ON_RESOLVE`, `HAS_ON_PACKAGE_MANIFEST_ERROR`, `on_resolve`, `on_package_manifest_error`, `on_package_download_error_pkg` and their `ErasedCallbacks` fields. - The `any_root` argument of `process_dependency_list_item` and `resolve_waiters_of_appended_package`, and `resolve_waiters_and_notify`, which only added the `on_resolve` call. `process_dependency_list_for_ctx` loses its two callback arguments and is now `process_dependency_list`. - `PackageManifestError` and `Error::PackageManifest`, built only for the manifest hook. - `GitCommitRequest.url`, read only to pass to the manifest hook. - `PackageManager::enqueue_dependency_list`, called only by `Queue::poll_modules`. The free function stays.
|
Updated 5:44 AM PT - Oct 27th, 2026
❌ @robobun, your commit 5589e8f has 1 failures in
🧪 To try this PR locally: bunx bun-pr 46612That installs a local version of the PR into your bun-46612 --bun |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Pending-import and transpile path src/bundler/*, src/jsc/ModuleLoader.rs, src/jsc/VirtualMachine.rs, src/runtime/jsc_hooks.rs, src/resolver/lib.rs, src/bun_alloc/ast_alloc.rs |
ParseResult and TranspileExtra no longer carry pending-import or promise-pointer state. Runtime hooks remove pending-import handling. The linker skips imports marked unused and processes the remaining records. |
AsyncModule task lifecycle and dispatch src/jsc/AsyncModule.rs, src/jsc/RuntimeTranspilerStore.rs, src/jsc/JSValue.rs, src/event_loop/ConcurrentTask.rs, src/runtime/dispatch.rs, src/jsc/error.rs, src/runtime/error.rs |
The queue’s module processing and AsyncModule task dispatch are removed. Fulfillment remains as a crate-visible function called by runtime transpiler completion. Related task tags and error variants are removed. |
Package-manager callbacks and dependency processing src/install/PackageManager/*, src/install/PackageManagerTask.rs, src/install/error.rs, src/install/lib.rs |
Dependency processing drops root-resolution tracking and removed callback paths. Manifest failures are logged and passed to waiting dependencies. Related manifest error types, the dependency-list forwarding method, and the Git commit request URL field are removed. |
Suggested reviewers: alii
Priority:
Merge Risk: 
38735
-
This change removes unreachable async-module queue code and the install callbacks that only it used, with no intended behavior change. No concrete regression was found, so it looks safe to merge after the usual checks.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: removing dead code from the auto-install module queue and its install hooks. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description is complete and relevant. It explains the problem, the fix, scope boundaries, behavior impact, merge considerations, and detailed verification results. It does not use the exact templa… Full details: Description check
-
Explanation
The description is complete and relevant. It explains the problem, the fix, scope boundaries, behavior impact, merge considerations, and detailed verification results. It does not use the exact template headings "What does this PR do?" and "How did you verify your code works?", but it provides the required information under equivalent sections.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Status: ready for review. The diff is green.
|
|
Merged main (8a0e49e) into this branch. Main changed code that this PR deletes, so the merge removes a little more than the PR did.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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 nit, I also checked the two refactors in this sweep that could have changed behavior: the collapse of ErasedCallbacks::on_package_download_error in src/install/PackageManager/runTasks.rs drops the !is_store_installer branch, and the only impl with HAS_ON_PACKAGE_DOWNLOAD_ERROR = true (src/install/isolated_install.rs) is also IS_STORE_INSTALLER = true, so that branch was unreachable; and allow_promise in Bun__transpileFile is still read by the RuntimeTranspilerStore async path, so dropping promise_ptr does not orphan it. A repo-wide grep for pending_imports after the change finds nothing, consistent with the no-writer claim.
Extended reasoning...
Dead-code sweep (+141/-1688) removing the AsyncModule queue, its task tag, two error variants, and the install-side hooks only it set; no security-sensitive surface. The branch collapse in the download-error dispatcher and the removal of the promise out-param are the two spots where behavior could have shifted, and both check out as behavior-preserving; the size of the deletion and its reach into module loading and the dispatch table are why a human look is still worthwhile.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟡
src/bundler/transpiler.rs— nit: maintainers keep aSourcefield that nothing reads any more. The deleted enqueue block intranspile_source_code_of_any_provenancewas the last reader ofcontents_is_recycled; after this sweep every remaining mention only writes or copies it (src/ast/lib.rs:2091, src/ast/lib.rs:2386, src/bundler/ParseTask.rs:2446, src/bundler/bundle_v2.rs:4222, src/bundler/linker_context/generateCodeForFileInChunkJS.rs:204, src/resolver/package_json.rs:1050) and the comment at src/bundler/transpiler.rs:1363 still says it "records that the bytes are externally-owned". Fix: delete the field and its writers in this PR, or state in the description why it stays, so the sweep leaves no new orphan.Why this was flagged
The removed block in src/runtime/jsc_hooks.rs read
parse_result.source.contents_is_recycledto decide whether to detach the resolver's shared buffer before handing the parse to the AsyncModule queue; that was its only reader. A repo-wide grep after the change finds only the declaration (src/ast/lib.rs:2091), the Default andinit_recycled_fileinitialisers (src/ast/lib.rs:2106, src/ast/lib.rs:2386), a literalfalsewrite in src/bundler/ParseTask.rs:2446, and three field-to-field copies (src/bundler/bundle_v2.rs:4222, src/bundler/linker_context/generateCodeForFileInChunkJS.rs:204, src/resolver/package_json.rs:1050). No code branches on the value. The PR description lists the orphans it removed and saysunused_pubfound no new orphan, but that lint covers items, not struct fields, so this one is left behind. This is a dead-code nit with no runtime effect; the base branch had the same field but with a live reader.Verification: nit. Triggering condition: none needed — it is a static property of the merged tree. The PR diff for src/runtime/jsc_hooks.rs removes
if parse_result.source.contents_is_recycled {, so that was the base branch's sole reader. Being apubfield it escapes rustc's dead_code lint and theunused_pubcheck the PR description cites. Nothing breaks at runtime.
The removed enqueue block in `transpile_source_code` was its only reader. The other mentions wrote or copied it.
|
Two related items, both in the Notes of the description:
|
There was a problem hiding this comment.
I reviewed the current head (including the new Source.contents_is_recycled commit) and found no bugs; since this deletes ~1.7k lines across the module loader, event-loop dispatch, and the install run_tasks loop, a maintainer look is still worthwhile.
What was reviewed:
- Traced the "cannot run" claim:
ParseResult.pending_importshad no writer, the oldQueue::on_pollreturned early on an emptymap, andmapwas only pushed byenqueue, whose sole caller was the removed block insrc/runtime/jsc_hooks.rs. - Checked the collapsed
on_package_download_errorbranch insrc/install/PackageManager/runTasks.rs: the only impl settingHAS_ON_PACKAGE_DOWNLOAD_ERROR(StoreRunTasksCallbacks) also setsIS_STORE_INSTALLER, so no non-store impl can now hit the_storedefault. - Checked
Bun__transpileFile's sync-success arm now returning null: the oldpromiselocal was only written throughpromise_ptr, which only the removed block wrote;allow_promisestill gates the surviving concurrent-transpiler path. - Confirmed no remaining references to any removed name (
contents_is_recycled,as_internal_promise,take_state,reset_shared_buffer,PackageManifestError, the removed hooks) and that the task-tag count assert matches the one removed tag.
Extended reasoning...
The change is a dead-code sweep of 25 files (+142/-1697) removing the auto-install AsyncModule queue, its task tag, two error variants, the pending_imports field and its linker consumers, Source.contents_is_recycled, and three RunTasksCallbacks hooks plus the any_root plumbing that only fed them. It touches no auth, crypto, or input-parsing surface; the sensitive aspects are the FFI return contract of Bun__transpileFile and the install loop's hook dispatch, both of which I traced as behavior-preserving. No findings, no CODEOWNERS coverage, and the one commit since the last posted review is a small follow-on removal I verified. Size and the subsystems involved (module loader, event-loop task dispatch, install run loop) are what tip this to defer rather than approve.
Problem
ParseResult.pending_importshas no writer, and the oneQueue::enqueuecall (src/runtime/jsc_hooks.rs) needspending_imports.len() > 0.maineditedsrc/jsc/AsyncModule.rssince Oct 14.--disallow-code-generation-from-strings=strictruns static code only, checked in one place in the engine #45173 addedInitOpts.is_staticafter a review comment.Fix
AsyncModule,InitOpts, most ofQueue, one task tag, two error variants,Source.contents_is_recycled.bun_install: two flags and three hooks ofRunTasksCallbacks, theany_rootarguments,PackageManifestError.bun bd,bun run rust:check-all,bun run rust:mordant, clippy, 12 test files. One known debug-build timeout (Notes).Background
RunTasksCallbacksis the hook trait of the install loop. Only the queue's impl set the removed hooks.Pendingvariants (open Remove dead code from bun_bundler, bun_install, bun_runtime, bun_parsers, and the JSC private host functions #41088 owns them) and the no-op wake task (that changes what a wake posts).Downsides
unused_pubfinds no new orphan.bun build --compile --aot#45631 (3), bundler: copy assets of 4 GiB or more whole, refuse other inputs of that size #46091, logger: key the line and column scan on a Source id, not on the address of its text #46277, Convert a byte order mark only when the file reader's caller asks, so bundled assets stay byte for byte #46621 (1 each).Bun.file()blob module an absolute source URL #46842 callintern_transpile_path, which has no other caller onmainand goes. Land them first, or they restore it.Behaviour change: none
Notes
Why the queue cannot run
ParseResultliteral setpending_imports: Default::default(). Nothing pushed to it or assigned it. Its readers wereLinker::link(is_deferred), the enqueue block intranspile_source_code, andAsyncModule.rs. The workspace compiles without the field, so no other access exists.Queue::enqueue, andenqueuewas the only writer ofQueue.map. Soon_pollalways took itsmap.is_empty()return, andrun_tasks,poll_modules, the error hooks,AsyncModule::{init, done, on_done, resolve_error, download_error, resume_loading_module}and theAsyncModuletask never ran.Error::AsyncModuleandUnexpectedPendingResolutioncame only from that block.TranspileExtra.promise_ptrwas read only there, so thepromiselocal inBun__transpileFilestayed null and the function returned null on this path, as it does now.Queue::run_tasks,QueueRunTasksCallbacks, therun_tasks()call inon_poll), which was live then. Its author closed it after 16 minutes for lack of a regression test, with no maintainer comment. install: consolidate the open package manager pull requests #44968 then made auto-install block in the resolver and added themap.is_empty()return, which made the drain unreachable. This change removes both parts.src/jsc/AsyncModule.rssince Oct 14 are install: consolidate the open package manager pull requests #44968,--disallow-code-generation-from-strings=strictruns static code only, checked in one place in the engine #45173, Hardening: fix crashes across the runtime, bundler, dev server, shell and CLI #46021 and bun_core::String can no longer borrow (fixes a source-map name use-after-free) #46825. Fix use-after-free ofBuildMessage.position.file: real lifetimes forSource,Path,ImportRecordand resolve results #46834 edited the enqueue block. 27 open pull requests change the file. Theis_staticplumbing from--disallow-code-generation-from-strings=strictruns static code only, checked in one place in the engine #45173 goes with the queue.=strictwith auto-install does not depend on it:transpile_source_code_innerstill setsis_staticfor the synchronous path.Removed, by file
src/jsc/AsyncModule.rs(1,159 to 110 lines):InitOpts,AsyncModuleand itsTaskableimpl,PackageDownloadError,PackageResolveError,Map,Queue::{map, scheduled, vm, on_resolve, enqueue, run_tasks, on_package_manifest_error, on_package_download_error, poll_modules},QueueRunTasksCallbacks. Kept:Queue(now a unit struct),WakeContext,Queue::on_wake_handler,Queue::on_poll,fulfill(wasAsyncModule::fulfill, same body).src/bundler/transpiler.rs:ParseResult::{pending_imports, empty, is_pending_import}.src/bundler/linker.rs:is_deferredandskip_deferred, which were always false.src/runtime/jsc_hooks.rs: the enqueue block, theError::AsyncModulearm and thepromiselocal inBun__transpileFile, thepromise_ptrinitialisers. Alsointern_transpile_pathand its thread-local set: since Fix use-after-free ofBuildMessage.position.file: real lifetimes forSource,Path,ImportRecordand resolve results #46834 the enqueue block was their only user, and a private function with no caller does not compile.src/ast/lib.rsand five places that wrote or copied it:Source.contents_is_recycled. The enqueue block was its only reader.src/event_loop/ConcurrentTask.rs,src/runtime/dispatch.rs: theAsyncModuletask tag and its two arms.task_tag::COUNTgoes from 86 to 85. Tags are not stored anywhere.src/runtime/error.rs,src/jsc/error.rs,src/jsc/VirtualMachine.rs:Error::AsyncModule,UnexpectedPendingResolution(both enums), the "Unexpected pending import" message inprocess_fetch_log.src/jsc/ModuleLoader.rs:TranspileExtra.promise_ptr.src/jsc/JSValue.rs:as_internal_promise.src/bun_alloc/ast_alloc.rs:ScopedAstAlloc::take_state.src/resolver/lib.rs:cache::Fs::reset_shared_buffer. Each had its last caller in the removed code.src/install/PackageManager/runTasks.rs(2,344 to 2,157 lines):HAS_ON_RESOLVE,HAS_ON_PACKAGE_MANIFEST_ERROR,on_resolve,on_package_manifest_error,on_package_download_error_pkg, theirErasedCallbacksfields, and the branches they guarded (the other branch stays).QueueRunTasksCallbackswas the only impl that set them.resolve_waiters_and_notifyonly added theon_resolvecall, so its two callers now callresolve_waiters_of_appended_package.process_dependency_list_for_ctxloses its two callback arguments and is nowprocess_dependency_list.src/install/PackageManager/processDependencyList.rs: theany_rootargument ofprocess_dependency_list_item, which only fedon_resolve.src/install/lib.rs,src/install/error.rs:PackageManifestErrorandError::PackageManifest, built only for the manifest hook.src/install/PackageManagerTask.rs,PackageManagerEnqueue.rs:GitCommitRequest.url, read only to pass to the manifest hook, andPackageManager::enqueue_dependency_list, called only byQueue::poll_modules. The free function of that name stays.Verification (head 5589e8f, which includes main up to 8a0e49e)
bun bd(debug, ASAN) builds.bun run rust:check-all: 12 of 12 targets pass.cargo check --workspace --all-targetsandcargo clippy --workspace --no-depspass.cargo fmt --checkreports nothing.bun test test/internal/source-lints/: 183 pass.bun run rust:mordant(Linux, Windows, macOS): exit 0.bun-add.test.ts220,bun-install-retry.test.ts208,bundler_cjs2esm.test.ts113,plugins.test.ts111,node-module-module.test.js80,bundler_plugin.test.ts68,bundler_loader.test.ts62,resolve-error.test.ts32,autoinstall-cached-manifest.test.ts4,resolve-autoinstall-invalid-name.test.ts4,resolve-autoinstall-log-dangling.test.ts2: all pass.run-autoinstall.test.ts: 299 pass, 1 fail.run-autoinstall.test.ts: main 297/3, 293/7, 294/6, 292/8, 288/12, branch 295/5, 292/8, 296/4, 297/3, 297/3.isolated-install.test.ts: main 273/10, branch 275/8.bun-install-git-deps.test.ts: main 192/9, branch 197/4.bun-install.test.ts: 772 pass on both.bun-install-streaming-extract.test.ts: 132 pass on both.bun run rust:mordantsees the orphans of a deletion: withPackageManager::enqueue_dependency_listput back it reportsunused_puband exits 101.Open pull requests (heads of all 5,857, fetched 2026-10-27)
Bun.file()blob module an absolute source URL #46842 each add a call ofintern_transpile_pathin new code. Onmainits only caller is the enqueue block, so this change has to remove it. If one of them lands first, the next merge ofmaininto this branch keeps the function. If this change lands first, they restore it (about 30 lines, with its thread-local set).contents_is_recycledin a newCloneimpl forSource: that one line goes. install: key overrides by the name a dependency is declared under #45750 (conflicts withmainalready) keeps theany_rootargument in a rewrite ofresolve_waiters_and_notify: it drops both on its next rebase.resume_loading_module, a function that is removed here. Remove dead code from bun_bundler, bun_install, bun_runtime, bun_parsers, and the JSC private host functions #41088 removesEnqueueResult::Pending, which is why the resolver chain is not in this change.main. A trial merge of each withmainand with this branch (git merge-tree) gives new conflicted files to five:AsyncModule.rs(take the deletion),jsc_hooks.rs(delete the enqueue block, keep its other edits andintern_transpile_path),RuntimeTranspilerStore.rs(keep its edits, callcrate::async_module::fulfill),ModuleLoader.rs(oneuseline).bun build --compile --aot#45631:AsyncModule.rs(take the deletion),dispatch.rs(its rewritten arms without thetask_tag::AsyncModulearm),ModuleLoader.rs(itsuselines withoutJSInternalPromise).ParseTask.rs. Its lines, withoutcontents_is_recycled: false,.bundle_v2.rs. Itsnew_source.clone().src/resolver/lib.rs. Its new doc line stays,reset_shared_bufferabove it goes.One follow-up, not in this change
Pendingchain.EnqueueResult::Pending(src/install_types/resolver_hooks.rs:1380) has no constructor. It feedsDependencyToResolve::Pending,MatchStatus::PendingandResultUnion::Pending(src/resolver/resolver.rs,src/resolver/result.rs), and every consumer maps them to not-found (src/jsc/VirtualMachine.rs:5274,src/runtime/jsc_hooks.rs:815). After this change nothing reads the fields ofPendingResolution. Open Remove dead code from bun_bundler, bun_install, bun_runtime, bun_parsers, and the JSC private host functions #41088 removes the root, so the chain should leave with it.Queue::on_wake_handlerpostsPollPendingModulesTaskfor each package manager wake, andQueue::on_pollnow only logs. Its removal takesQueue,WakeContext,VirtualMachine.modules, the task tag andWakeHandlerwith it. It also changes what a wake posts to the JS thread, so it is not a deletion with no effect.bun_jscnamesbun_installat four sites (thepackage_manageraccessor, two log swaps,CrateError::Install), so the crate edge can go in a later change.Left alone on purpose
FilenameStore::existsandDirnameStore::existshave no caller after this change. They stay, becauseintern_transpile_pathcallsexistsand three open pull requests need that function back.pub(crate):RunTasksCallbacks,run_tasks,drain_dependency_list,determine_preinstall_state,start_progress_bar_if_none,end_progress_bar,PackageManager.pending_tasks,PackageManager.total_tasks,cache::Fs::shared_buffer.Debugimpls (as Remove dead code: 49 trait impls nothing uses, across 12 crates #43664 decided), six unused impls and methods inbun_lintandbun_format, and 13#[bun_jsc::host_fn]attributes whose shim has no caller.IDLByte,IDLShortandIDLLongLonghave no user on main, but bun:dom and the Vitest work in one tree (not for merging) #45415 uses them.completions/bun-cli.jsonwithmisctools/generate-cli-completions.ts(5,883 lines, nothing reads the JSON),packages/h3blast,packages/bun-plugin-yaml, thelogsbuild option.Review
InitOpts.is_staticplumbing that--disallow-code-generation-from-strings=strictruns static code only, checked in one place in the engine #45173 added.Bun.file()blob module an absolute source URL #46842 first is the order with the least work, because ofintern_transpile_path. The other order works too: they restore the function.