Skip to content

fix(bake): preserve module identity and HMR update order - #46704

Open
RentnerKev wants to merge 3 commits into
oven-sh:mainfrom
RentnerKev:fix/bake-stable-hmr-modules
Open

RentnerKev wants to merge 3 commits into
oven-sh:mainfrom
RentnerKev:fix/bake-stable-hmr-modules

Conversation

@RentnerKev

@RentnerKev RentnerKev commented Oct 24, 2026 •

Copy link
Copy Markdown

What does this PR do?

Keep unchanged client modules stable across accepted HMR updates. Deferred-export barrels can be reanalyzed even when their cached module remains valid; their output currently depends on sibling modules in the incremental chunk. Use the existing per-file nested renamer for unminified dev-server factories and reserve each factory's unbound and pinned names locally.

Suppress a non-stale module's replacement only when its code, kind, and complete source map match. Explicitly invalidated or previously failed modules still emit. Include source content in the existing source-map hash so comment-only edits receive a new immutable map URL.

Also preserve update order at both client boundaries:

  • Set script.async = false before assigning an HMR blob's src, so a small second update cannot execute before a large first update that supplies its dependency.
  • Invalidate every affected module before evaluating any importer. Snapshot the previous accept callbacks, deduplicate shared disposal boundaries, and invoke acceptance after the existing Promise.all has settled asynchronous dependencies. This prevents stale sibling exports, duplicate disposal, and callbacks receiving the previous async export.

These changes are limited to development output and the existing HMR runtime. They add no dependencies or production bundler modes.

How did you verify your code works?

Base: a8fcd319be58ad4d284330b86602cfb3c8e49c83. Native release-with-assertions build on Debian 13 x64 in Bun's development container, using Clang 23, the pinned Rust nightly, and --lto=off:

bun run build:assert --lto=off -j2 test test/bake/dev/bundle.test.ts test/bake/dev/hot.test.ts test/bake/dev/plugins.test.ts test/bake/dev/sourcemap.test.ts test/bake/serve-plugins-dev-server.test.ts test/bake/dev/esm.test.ts
bun run build:assert --lto=off -j2 test test/js/bun/webview/webview-chrome.test.ts -t 'HMR applies a large update before the following small update'
  • Original stability/map regressions: 15 pass and 4 fail before this PR; all 19 pass after it.
  • Four new batch cases cover sync/async dependencies with and without disposal, exact exports, the old callback, and factory evaluation counts. An intermediate isolated build exposed stale async callback values despite a correct DOM; waiting on the existing load promises fixes that case too.
  • The final native build combining the four own drafts passes all 125 tests across the six suites above, without skips, plus the real-Chrome regression. This build excludes the foreign input-map and cycle patches used by the separate Router candidate.
  • The same four-draft source passes bun run rust:check-all for all 12 configured targets, none skipped. These are cross-target compilation checks.
  • The combined Windows x64 Router candidate, built as stable release with LLVM 23.1.1 and the pinned nightly, passes the six new watcher/client regressions, 114 tests across nine native suites (1 existing skip and 6 todos), and the real-Chrome ordering test. Router browser coverage passes 7 production and 14 development tests without skips.

The Chrome regression uses the existing WebView Chrome backend and availability detection. Only a root process supplies Chrome's required --no-sandbox through the existing isRoot harness flag.

Final combined native validation on Windows x64, Linux x64, and macOS ARM64 passes 7 production and 14 development Router browser tests, plus all 9 imported-helper generator cases and 9 live-browser helper cases per platform. Native macOS: 118 pass and 3 existing todos across nine suites, all 12 selected regressions pass individually, and the Chrome ordering test passes. Complete macOS run. These combined-stack results are separate from the isolated own-draft build above. Upstream Buildkite remains pending. This PR does not include input-map composition, plugin watch dependencies, cached dependency closure delivery, or general cycle-initialization changes. Prepared with AI assistance.

@RentnerKev RentnerKev changed the title fix(bake): keep unchanged HMR modules stable fix(bake): preserve module identity and HMR update order Oct 24, 2026
@RentnerKev
RentnerKev marked this pull request as ready for review October 24, 2026 13:54

@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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: oven-sh/bun
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: 811afe7
  • Layers: Repository YAML
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 0713576c-b8f7-4b6b-b339-baeed374cbeb
Commits

Reviewing files that changed from the base of the PR and between 811afe7 and 954b2d3.

Files selected for processing: 10
  • src/bundler/linker_context/renameSymbolsInChunk.rs
  • src/js_printer/renamer.rs
  • src/runtime/bake/dev_server/finalize.rs
  • src/runtime/bake/dev_server/incremental_graph.rs
  • src/runtime/bake/dev_server/packed_map.rs
  • src/runtime/bake/hmr-module.ts
  • src/runtime/bake/hmr-runtime-client.ts
  • test/bake/dev/hot.test.ts
  • test/bake/dev/sourcemap.test.ts
  • test/js/bun/webview/webview-chrome.test.ts

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

Walkthrough

This change adjusts InternalBakeDev name reservation, client chunk and source-map comparison, and HMR reload ordering. It adds regression tests for barrel imports, batched updates, immutable source maps, and browser updates.

Changes

HMR update handling

Layer / File(s) Summary
Nested name reservation
src/bundler/linker_context/renameSymbolsInChunk.rs, src/js_printer/renamer.rs, test/bake/dev/hot.test.ts
InternalBakeDev reserves names from each file’s module scope before nested renaming. Tests cover barrel dependencies, nested shadowing, and namespace imports.
Client output and source-map comparison
src/runtime/bake/dev_server/incremental_graph.rs, src/runtime/bake/dev_server/finalize.rs, src/runtime/bake/dev_server/packed_map.rs, test/bake/dev/sourcemap.test.ts
The dev server compares client output with the previous file and adds code to the current chunk only when output changed. Source-map hashes include quoted source contents and their byte lengths. Tests check that earlier HMR maps retain their source contents.
HMR reload ordering
src/runtime/bake/hmr-module.ts, src/runtime/bake/hmr-runtime-client.ts, test/bake/dev/hot.test.ts, test/js/bun/webview/webview-chrome.test.ts
replaceModules waits for asynchronous reloads before invoking saved self-accept callbacks. The runtime client sets HMR script elements to load synchronously. Tests cover batched updates and browser update ordering.

Suggested reviewers: alii, jarred-sumner

Priority: Normal

Merge Risk: Minimal · up to 954b2

No concrete issue in the supplied evidence currently prevents merging, subject to the pending upstream checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely summarizes the primary changes: preserving module identity and maintaining HMR update order.
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.
Description check Passed The pull request description includes both required sections. It clearly explains the changes, scope, exclusions, and verification steps with detailed test results.
  • 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.

❤️ Share

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant