Repository navigation
Conversation
The visit pass was writing the generated default-export binding symbol
(`<basename>_default`) into `func.func.name` for `export default function () {}`
(and its async/generator variants), so the printer emitted
`export default function <basename>_default() {}` and the function's
observable `.name` became `"<basename>_default"` instead of `"default"`.
Per ES2015 15.2.3.11, an anonymous `export default` function declaration
gets its name via SetFunctionName(F, "default"); node prints "default".
The class path already only injects a name when decorator lowering
requires it; this brings the function path in line.
`data.default_name` is still recorded via `record_on_exit!()`, and the
bundler's own conversion in convertStmtsForChunk assigns the name
separately, so nothing else relies on this assignment.
|
Reproduced with: $ printf 'export default function () {}\nimport self from "./mod.mjs";\nconsole.log(JSON.stringify(self.name));\n' > /tmp/mod.mjs
$ bun /tmp/mod.mjs
"mod_default"
$ node /tmp/mod.mjs
"default"PR: #34932 The name injection is kept for the React Fast Refresh dev path (which needs a binding for The diff is green on every lane that runs it ( |
WalkthroughChangesDefault export naming
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 9:07 AM PT - Jul 21st, 2026
❌ @robobun, your commit 259c30d has some failures in 🧪 To try this PR locally: bunx bun-pr 34932That installs a local version of the PR into your bun-34932 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…ests
The React Fast Refresh transform emits $RefreshReg$(default_name_ref, ...)
for an anonymous default-exported function, and the HMR export lowering
relies on func.func.name being set to produce a binding for that ref.
Keep the name injection on that dev-only path so bake/dev react-spa
works, while the runtime transpiler (bun run / --no-bundle) still leaves
the function anonymous.
Also:
- 31401.test.ts: the third case asserted the --no-bundle identifier;
switch it to a bundled build, which still surfaces _1_default via
convertStmtsForChunk and preserves the original leading-digit check.
- runtime-transpiler.test.ts: assert {stdout, stderr, exitCode} as one
object instead of expect(stderr).toBe("").
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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_stmt.rs`:
- Around line 584-587: Shorten the comment near the generated function-name
handling to no more than three lines while preserving the essential points:
leave the name unset for default assignment, and inject it only for the React
Fast Refresh binding path.
In `@test/regression/issue/31401.test.ts`:
- Around line 48-49: Remove the prose comments in the regression test, including
the comment describing --no-bundle and convertStmtsForChunk; retain only the
issue URL comment required by the project’s regression-test convention.
🪄 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: 0525b3dc-1123-4c95-a29b-faf7034d3681
📒 Files selected for processing (3)
src/js_parser/visit/visit_stmt.rstest/bundler/transpiler/runtime-transpiler.test.tstest/regression/issue/31401.test.ts
runtime-transpiler.test.ts has an in-process dynamic .hbs import that is sensitive to the release transpiler cache; keep the new subprocess-based tests in a separate file so the gate's release lane stays deterministic.
There was a problem hiding this comment.
Beyond the inline nit, I re-checked the downstream consumers after the react_fast_refresh gating: convertStmtsForChunk.rs:525 still force-assigns s.default_name to func.name when stripping exports (so bundled output and the updated 31401 test are covered), and opts.features.react_fast_refresh is only ever set true on bundler/bake paths (ParseTask.rs:2475, bake/*) — never on the runtime transpiler — so the .name === "default" fix holds for bun run and --no-bundle while HMR keeps its binding.
Extended reasoning...
The only finding this run is a test-organization nit; the substantive concerns from my earlier passes (31401 breakage, HMR ReferenceError) were addressed in e4da3ef and I verified the current head still satisfies them: the bundler linker unconditionally names the function when lowering SExportDefault, and the feature flag the new gate reads is exclusively a bundler/dev-server flag. Deferring rather than approving because this is a parser visit-pass change that fans out to four output modes; the gating choice (keep name-injection under react_fast_refresh vs. teaching ConvertESMExportsForHmr to handle the anonymous case) is a small design call worth a maintainer glance.
There was a problem hiding this comment.
No issues found in this pass; all earlier feedback (HMR/$RefreshReg$ binding, 31401 test, stderr assertion, comment length, test-file placement) has been addressed. Deferring to a human because this is a user-observable parser output change whose first revision broke the Fast Refresh / HMR export-lowering path — someone familiar with bake/HMR should confirm the react_fast_refresh gate is the right seam.
Checked: the else { data.default_name.ref_ } arm in the fast-refresh block is now only reachable when func.func.name was set (gate matches), so ConvertESMExportsForHmr still gets a binding.
Checked: convertStmtsForChunk assigns default_name itself when stripping exports, and the updated 31401 test exercises that.
Checked: the server_components.wraps_exports() path without fast-refresh now wraps an anonymous function expr — the reference uses the "default" string arg, not .name, so no behavior loss.
The one CI failure (bun-server.test.ts on macOS aarch64) is unrelated to the parser.
Extended reasoning...
Overview
The PR gates a single assignment in src/js_parser/visit/visit_stmt.rs (s_export_default → SFunction arm): func.func.name = Some(data.default_name) now only fires when p.options.features.react_fast_refresh is set. This keeps export default function () {} anonymous in runtime-transpiler and --no-bundle output so the engine's SetFunctionName assigns .name = "default", matching Node and the ES2015 spec. A new test file covers function/async/generator/async-generator/class/arrow/named variants, and test/regression/issue/31401.test.ts is re-expressed against bundled output where convertStmtsForChunk still emits _1_default.
Security risks
None. This changes emitted identifier presence for anonymous default-exported functions; if anything it stops leaking the source file's basename via .name. No untrusted-input parsing, no allocation, no FFI.
Level of scrutiny
Moderate-to-high. The production diff is ~6 lines, but it sits in the parser visit pass and the removed assignment was doing double duty — it was both the source of the .name bug and the thing ConvertESMExportsForHmr relied on to produce a binding for $RefreshReg$. The first revision of this PR unconditionally removed the assignment and would have caused a runtime ReferenceError under bake HMR for anonymous default components in .tsx files; the current revision gates it correctly and test/bake/dev/react-spa.test.ts was verified locally. That history is exactly why a maintainer who owns bake/HMR should confirm react_fast_refresh is the right flag to gate on (vs. e.g. hot_module_reloading or checking both).
Other factors
- All four inline findings from earlier passes are resolved and marked as such in the thread.
- Test coverage is good: the variant matrix (all four function forms, class, arrow, named regression guard) is covered via subprocess spawns with combined
{stdout, stderr, exitCode}assertions; the bundler path's name-injection is still exercised by the updated 31401 test. - The separate
export-default-name.test.tsfile was justified by the author (a pre-existing cache-poisoning bug inruntime-transpiler.test.ts). - The one CI failure (
test/js/bun/http/bun-server.test.ts, macOS aarch64) is an HTTP test with no plausible connection to a parser identifier-emission change.
|
Findings from work on the same hunk, while I looked at #44721. I ran each item on release builds of main bd599f5 and of the branch below, except where it says "read". Gate. This PR sets the name only under Labels. Without a name, JSC binds the function to its private name
JavaScriptCore composes the last two itself ( Cache version. The printed text of a module changes, so One more effect. The runtime transpiler cache keys an entry on the content, not on the path. On main, a 73 KB |
This branch has not been deployed
1 participant
What
export default function () {}(and itsasync/generator variants) produced a function whose observable.namewas"<basename>_default"instead of the spec-required"default". Node prints"default".export default class {}andexport default () => 1were already correct at runtime.Why
The visit pass wrote the generated default-export binding symbol (
<basename>_default, created bycreate_default_name) intofunc.func.namewhen the source had no name, so the printer emittedexport default function mod_default() {}. Per ES2015 15.2.3.11, an anonymous default-exported function declaration receives its name viaSetFunctionName(F, "default")at evaluation time, which only happens when the emitted declaration stays anonymous.The class path only injects
data.default_nameintoclass_namewhen decorator lowering needs a concrete binding; this change brings the function path in line. The assignment is kept underreact_fast_refreshbecause that dev-only transform emits a$RefreshReg$(default_name_ref, ...)call that needs a concrete binding, andConvertESMExportsForHmrrelies onfunc.nameto produce one.react_fast_refreshis only set on bundler/bake paths, never on the runtime transpiler, sobun run/--no-bundleoutput stays anonymous.data.default_nameis still recorded viarecord_on_exit!(), and the bundler'sconvertStmtsForChunkassigns the name itself when stripping exports.This is observable via
.name-keyed dispatch/telemetry and leaked the source file's basename.Verification
New tests in
test/bundler/transpiler/export-default-name.test.tscoverfunction,async function,function*,async function*,class, arrow, and a named function (regression guard). The four function cases fail on main with"mod_default"and pass with this change.test/bake/dev/react-spa.test.ts(React Fast Refresh with anonymous default components) passes.test/regression/issue/31401.test.tswas updated to check the_1_defaultidentifier in bundled output, whereconvertStmtsForChunkstill emits it.[review] gate passed · iteration 4 · 3 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 3 passed · 1 rejected · iteration 4
evidence per changed file