[robobun]

Problem

The guarded form import.meta.hot?.accept() (or ?.data, ?.on(...), etc.) throws at module evaluation in the dev server:

Error: import.meta.hot.accept cannot be used indirectly.

The same source is a no-op in production and in plain bun build (where import.meta.hot is undefined), so a file that ships fine dies only under Bun.serve dev. The non-optional import.meta.hot.accept() works; the one-character difference is the ?..

Cause

e_dot/e_index in the parser visitor skip maybe_rewrite_property_access when optional_chain is set. For import.meta.hot?.accept:

1. The inner import.meta.hot access (no ?.) rewrites to E::Special::HotEnabled.
2. The outer .accept access has optional_chain = Start, so the rewrite is skipped and the result stays as EDot(HotEnabled, "accept", optional_chain=Start).
3. HotEnabled prints as hmr.indirectHot, yielding hmr.indirectHot?.accept().
4. indirectHot is a truthy Proxy whose every property get throws, so ?. cannot short-circuit and the module throws.

Fix

import.meta.hot is always defined when HMR is enabled, so the ?. after it is a no-op guard intended for production. After visiting the target, if it resolved to HotEnabled and optional_chain == Start, clear optional_chain so the existing direct-access rewrite fires and lowers to the appropriate HMR module member (hmr.accept, hmr.data, hmr.on, ...). Production (HotDisabled) is untouched; it already prints as (void 0)?.x which short-circuits correctly at runtime.

Test

test/bake/dev/hot.test.ts gets a new import.meta.hot with optional chaining case that exercises ?.data, ?.dispose, ?.on, and ?.accept in a served module and asserts the module evaluates, self-accepts, and the dispose callback runs across reloads.

---

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bake/dev/hot.test.ts

[robobun]

^{Updated 5:29 PM PT - Jul 21st, 2026}

❌ @robobun, your commit b476f28 has some failures in Build #77206 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 35009

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

bun-35009 --bun

[coderabbitai]

Walkthrough

Changes

Hot-module optional chaining

Layer / File(s)	Summary
Hot-module property access lowering src/js_parser/visit/visit_expr.rs	e_index and e_dot remove the optional-chain marker for HotEnabled targets so property-access rewriting handles them as direct access.
Optional hot API development test test/bake/dev/hot.test.ts	Adds coverage for optional data, dispose, and on access, and verifies disposal across repeated updates.

Suggested reviewers: jarred-sumner

🚥 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 is concise and accurately summarizes the main change: fixing optional chaining on import.meta.hot in dev server.
Description check	✅ Passed	The description covers what changed and how it was verified, even though it uses custom headings instead of the template’s exact section names.

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/js_parser/visit/visit_expr.rs`:
- Around line 1433-1438: Shorten the new explanatory comment in the
import.meta.hot optional-chaining handling to three lines or fewer while
preserving its essential explanation of treating optional access like direct
access in development and allowing maybe_rewrite_property_access to lower it
correctly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 237dd624-1d00-438c-bde3-234c288b7e21

📥 Commits

Reviewing files that changed from the base of the PR and between f759d1e and f814e9d.

📒 Files selected for processing (2)

• src/js_parser/visit/visit_expr.rs
• test/bake/dev/hot.test.ts

[claude]

LGTM — the bracket-access coverage I asked for is in, and both visitor hunks are now load-bearing.

What was reviewed:

• Verified the optional_chain = None clear is gated on HotEnabled + Start only, so it cannot affect non-HMR code or HotDisabled (which already short-circuits via (void 0)?.x).
• Traced the rewrite through maybe_rewrite_property_access (fold.rs:666) → HotAccept/HotData/etc. → printer output; the orphaned Continue on the enclosing ECall is harmless (has_chain_parent is write-only, printer emits hmr.acceptSpecifiers(...)).
• Test exercises ?.data, ?.dispose(...), ?.["on"](...), ?.accept() end-to-end and asserts self-accept + dispose actually fire across reloads.

Extended reasoning...

Overview

Two symmetric 5-line insertions in src/js_parser/visit/visit_expr.rs (e_dot at 1436-1440, e_index at 932-936) that clear optional_chain when it is Some(Start) and the visited target resolved to ESpecial(HotEnabled). This lets the immediately-following optional_chain.is_none() gate pass so maybe_rewrite_property_access lowers import.meta.hot?.X to the appropriate HMR member instead of leaving it as indirectHot?.X (a truthy proxy whose getter throws). One new dev-server test in test/bake/dev/hot.test.ts.

Security risks

None. The change is gated on Data::ESpecial(E::Special::HotEnabled), which is only produced by maybe_rewrite_property_access for the literal import.meta.hot expression when hot_module_reloading is on (fold.rs:499-509). No user-controlled input reaches the new branch; it cannot fire outside dev-server bundling.

Level of scrutiny

Medium. The file is the core expression visitor, but the guard is extremely narrow — it matches exactly one synthetic AST node variant that exists solely for HMR. I traced the downstream: after clearing, maybe_rewrite_property_access returns HotAccept/HotData/an EDot(hmr_ref, name), and for the call forms the enclosing ECall (which retains optional_chain=Continue) reaches the HotAccept arm at visit_expr.rs:2138 unchanged. has_chain_parent is set but never read, and the printer's ECall path doesn't special-case a dangling Continue, so no orphaned-chain hazard. HotDisabled is deliberately excluded — production prints (void 0)?.x, which short-circuits correctly without the rewrite.

Other factors

My earlier review asked for bracket-form coverage so the e_index hunk is tested; the author swapped ?.on(...) to ?.["on"](...) and confirmed reverting the e_index hunk fails the test. The test follows the neighboring import.meta.hot.data persistence / dispose cleanup patterns exactly (same harness helpers, await using, expectMessage on observable output, writeNoChanges to trigger reloads). CodeRabbit's comment-length nit was addressed. No open reviewer threads.

[robobun]

CI on b476f28 has only known-flaky failures that passed on retry (webview-chrome.test.ts animation timing, bake/dev/css.test.ts import-before-create race, test-http-server-connections-checking-leak.js). None touch the parser or HMR transform. test/bake/dev/hot.test.ts (including the new optional-chaining test) is green on all lanes. Ready for review.

[github-actions]

Closing this PR because it has been inactive for more than 90 days.