Skip to content

Remove dead code from the bake HMR runtime - #43828

Merged
Jarred-Sumner merged 2 commits into
mainfrom
robobun/416596a9/dead-code-sweep
Sep 25, 2026
Merged

Jarred-Sumner merged 2 commits into
mainfrom
robobun/416596a9/dead-code-sweep

Conversation

@robobun

@robobun robobun commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • hasExportStar in src/runtime/bake/hmr-module.ts has no caller. Its only call site is a block that fix dev server regressions from 1.2.5's hmr rewrite #18109 commented out on 2025-03-14. The availableExportKeys local above that block is read only by the commented-out code.
  • firstConnection in src/runtime/bake/client/websocket.ts is assigned once and never read.
  • No lint reports them. Earlier sweeps ran tsc --noUnusedLocals over src/js and scripts only, not over src/runtime/bake.

Fix

  • Delete hasExportStar, the commented-out check, availableExportKeys, and firstConnection. 2 files, 40 lines removed.
  • Correct because the bundler already drops hasExportStar. The generated bake.client.js, bake.server.js and bake.error.js differ from main only by the two removed local declarations.
  • Verified: tsc -p src/runtime/bake/tsconfig.json --noUnusedLocals no longer reports either file. test/bake/dev/esm.test.ts (17 pass), hot.test.ts (11 pass) and bundle.test.ts (23 pass) with the debug build.

Behaviour change: none

Background

  • hmr-module.ts is the module loader that the dev server sends to the browser and to the SSR realm. parseEsmDependencies walks the dependency list of an ES module. Each entry carries the export names that the importer uses.
  • The removed check compared those names with the exports of the dependency and threw a SyntaxError for a missing one. It has been off for 18 months. A missing export fails at the use site.
  • A deletion has one possible place, so no other design was weighed.

Downsides

Notes

Why this run is small

Every other hit of this run is live, is platform code with a user on another target, or is a line that one of the 34 open dead-code pull requests already deletes. Each removed line here was checked against those diffs. #40492 and #43378 touch websocket.ts in other hunks.

Scans of this run, all clean or already claimed


no test proof · iteration 1 · the description declares no behaviour change, so there is no failing test to prove; the existing suite in CI is the check

hmr-module.ts: remove hasExportStar, the commented-out export check that
was its only caller (disabled since #18109), and the availableExportKeys
local that only that check read.

client/websocket.ts: remove the firstConnection local. Nothing reads it.
@robobun

robobun commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 10:57 PM PT - Sep 22nd, 2026

❌ @robobun, your commit b3276f7 has 1 failures in Build #119941 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 43828

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

bun-43828 --bun

@robobun

robobun commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review. The diff is green. The one red CI job does not come from this change.

How this was verified:

  • tsc -p src/runtime/bake/tsconfig.json --noUnusedLocals reported hasExportStar, availableExportKeys and firstConnection on main. It reports none of them on this branch.
  • The generated bake.client.js, bake.server.js and bake.error.js differ from main only by the two removed local declarations.
  • bun bd test test/bake/dev/esm.test.ts (17 pass), hot.test.ts (11 pass), bundle.test.ts (23 pass).

CI on b3276f7 (build 119941): the build is finished, 180 of 181 jobs passed. No bake test failed or needed a retry on any lane.

  • One job is red: debian 13 x64-asan - test-bun. test/js/bun/spawn/spawn.test.ts ("an idle reader stopped at the highwater mark does not keep the process alive") fails on all 4 attempts. The child's stderr holds the LeakSanitizer line WARNING: ptrace appears to be blocked (is seccomp enabled?), and the test expects an empty stderr. The same failure is on main in build 119887.
  • This PR changes only src/runtime/bake/hmr-module.ts and src/runtime/bake/client/websocket.ts. The spawn test loads neither file.
  • The other annotations (hoist, bun-audit, filesink, bun-install-registry, serve-http2-lifecycle, fs.watch.rewrite, css/color) passed on retry.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: f8f27ee9-f85a-45ce-851d-2309dae76c1f

📥 Commits

Reviewing files that changed from the base of the PR and between 6d504dd and 25cdd1c.

📒 Files selected for processing (2)
  • src/runtime/bake/client/websocket.ts
  • src/runtime/bake/hmr-module.ts
💤 Files with no reviewable changes (2)
  • src/runtime/bake/hmr-module.ts
  • src/runtime/bake/client/websocket.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


Walkthrough

The changes remove an unused local from WebSocket initialization and remove imported-key validation and its export-star search helper from ESM dependency parsing.

Changes

WebSocket cleanup

Layer / File(s) Summary
Remove unused WebSocket local
src/runtime/bake/client/websocket.ts
initWebSocket no longer declares the unused firstConnection local. The WebSocket connection behavior is otherwise unchanged.

ESM dependency parsing

Layer / File(s) Summary
Remove imported-key validation
src/runtime/bake/hmr-module.ts
parseEsmDependencies no longer checks imported keys against direct exports or export-star graphs. The hasExportStar helper is removed.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to b3276

The inspected removals leave WebSocket connection handling and ESM dependency parsing unchanged. No merge-blocking risk was identified.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the main change: removing dead code from the bake HMR runtime.
Description check ✅ Passed The description explains the problem, fix, verification steps, behavior impact, and downsides. It does not use the exact template headings, but it provides the required change summary and verification…

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

@Jarred-Sumner
Jarred-Sumner merged commit abc26b7 into main Sep 25, 2026
4 of 5 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the robobun/416596a9/dead-code-sweep branch September 25, 2026 00:56
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.

2 participants