Skip to content

module_loader: generate bun:main wrapper on demand instead of caching it - #32493

Closed
robobun wants to merge 2 commits into
mainfrom
farm/7d613278/bun-main-on-demand
Closed

module_loader: generate bun:main wrapper on demand instead of caching it#32493
robobun wants to merge 2 commits into
mainfrom
farm/7d613278/bun-main-on-demand

module_loader: generate bun:main wrapper on demand instead of caching it

4714264
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jun 18, 2026 in 11m 33s

Code review found 3 potential issues

Found 6 candidates, confirmed 3. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 3
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit test/js/bun/resolve/bun-main-entry-point.test.ts:13 Stale comments still describe removed ServerEntryPoint.contents buffer
🟡 Nit test/js/bun/resolve/bun-main-entry-point.test.ts:90-99 Regression test comment should be just the issue URL
🟡 Nit src/bundler/entry_points.rs:268-275 Dead parameters and Result return type in ServerEntryPoint::generate

Annotations

Check warning on line 13 in test/js/bun/resolve/bun-main-entry-point.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Stale comments still describe removed ServerEntryPoint.contents buffer

The file-header comment here (lines 6-13) and the `--hot reloads` test comment (~lines 166-169) still describe `bun:main` as "backed by `ServerEntryPoint.contents`" with a "free-then-reallocate on each reload" — the exact mechanism this PR removes. Since this PR already edits this file and the whole point of the change is that there is no stored `contents` buffer anymore, these two comments should be updated in the same pass to describe the regenerate-on-fetch behavior.

Check warning on line 99 in test/js/bun/resolve/bun-main-entry-point.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Regression test comment should be just the issue URL

nit: per repo convention (root CLAUDE.md), regression-test comments should be just the issue URL — the Sentry signature, freed-mimalloc diagnosis, and fix mechanism here all duplicate the PR description and will drift as the implementation changes. Suggest reducing this block to `// https://github.com/oven-sh/bun/issues/27192` (and likewise dropping `(Sentry BUN-36H7)` from the new comment in `jsc_hooks.rs`).

Check warning on line 275 in src/bundler/entry_points.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Dead parameters and Result return type in ServerEntryPoint::generate

`ServerEntryPoint::generate()` now ignores both `_is_hot_reload_enabled` and `_path_to_use` and unconditionally returns `Ok(())`, leaving dead parameters and an infallible `Result` return type. Per CLAUDE.md ("Delete dead code in the same PR that makes it dead … parameters discarded in the body"), drop the dead params and change the return to `()`; the `jsc_hooks.rs` wrapper can then stop threading `watch`/`entry_path` and stop calling `.is_ok()` on a never-`Err` value (the `RuntimeHooks` fn-poi