fix(rolldown): keep enum declaration for optional-chain access#9229
Merged
fix(rolldown): keep enum declaration for optional-chain access#9229
Conversation
✅ Deploy Preview for rolldown-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
sapphi-red
reviewed
Apr 27, 2026
IWANABETHATGUY
approved these changes
Apr 27, 2026
Member
|
Sorry, I did not notice that the request review has been canceled. Wait for @sapphi-red to approve the pull request. |
Contributor
Author
Thank you for reviewing! I think that's an accident. |
sapphi-red
approved these changes
Apr 27, 2026
The tree-shaker was bypassing inclusion of enum declarations for any single-property member access (`E.x`, `E?.x`), assuming the finalizer would inline it to a literal. But the finalizer's `try_inline_enum_access` only matches `StaticMemberExpression`/`ComputedMemberExpression` — not the `ChainExpression` produced by `?.`. Result: optional-chained references survived in the output while their enum declarations were tree-shaken, producing `ReferenceError` at runtime. Tighten the bypass to only fire when the access is actually inlinable: non-optional and not a write target. Closes #9181
The test is no longer failing meaningfully — only stylistic differences remain (`var` vs `let`, arrow vs `function` IIFE). Removing the entry and re-running `just ued` deletes the stale diff.md and updates stats/summary.
3c7bdd2 to
677e6df
Compare
This was referenced Apr 29, 2026
Closed
Merged
shulaoda
added a commit
that referenced
this pull request
Apr 29, 2026
## [1.0.0-rc.18] - 2026-04-29 ### 💥 BREAKING CHANGES - optimization: default unspecified inlineConst.mode to smart (#9248) by @IWANABETHATGUY ### 🐛 Bug Fixes - rolldown_plugin_vite_import_glob: return error instead of panicking when virtual module uses a relative glob (#9241) by @shulaoda - binding: treat empty inlineConst object as omitted (#9247) by @IWANABETHATGUY - rolldown: keep enum declaration for optional-chain access (#9229) by @Dunqing - link_stage: restore inline let-else in exports-kind filter (#9237) by @IWANABETHATGUY - dev/lazy: avoid module reinitialization in lazy compilation patches (#9179) by @h-a-n-a - dev: visit identifier references for runtime rewrites in HMR finalizer (#9191) by @h-a-n-a - chunk-optimizer: pick dominator for runtime placement to avoid cycles (#9164) by @IWANABETHATGUY - make `this.emitFile` chunk path synchronous to avoid deadlock (#9031) by @lazarv - use sentinel id for `browser: false` ignored modules (#9192) by @shulaoda - prevent chunk optimizer from creating import cycles (#9228) by @IWANABETHATGUY ### 🚜 Refactor - replace tokio::sync::Mutex with std::sync::Mutex for non-IO data (#9176) by @shulaoda - rolldown_plugin_vite_import_glob: do not rewrite import path for absolute base (#9195) by @shulaoda - runtime_helper: wrap DependedRuntimeHelperMap in a struct (#9215) by @IWANABETHATGUY - drop redundant clear() in determine_safely_merge_cjs_ns (#9206) by @IWANABETHATGUY - clean up generate_lazy_export (#9208) by @IWANABETHATGUY - bitset: return bool from set_bit to fuse guard-and-set (#9207) by @IWANABETHATGUY - link_stage: simplify exports-kind filter and clarify safety comments (#9205) by @IWANABETHATGUY ### 📚 Documentation - determine_module_exports_kind (#9252) by @IWANABETHATGUY - fix dead link to esbuild ESM/CJS interop tests (#9230) by @Copilot - remove CSS bundling references (#9234) by @shulaoda - correct IncrementalFullBuild row in BundleMode table (#9214) by @IWANABETHATGUY - design: add bundler data lifecycle design doc (#9212) by @hyf0 - remove minifier alpha status notices (#9202) by @sapphi-red ### ⚙️ Miscellaneous Tasks - upgrade oxc to 0.128.0 (#9260) by @shulaoda - deps: bump rolldown-ariadne to 0.6.0 (#9254) by @IWANABETHATGUY - deps: update github actions (#9259) by @renovate[bot] - deps: update github actions (#9258) by @renovate[bot] - remove renovate overrides (#9257) by @Boshen - use ubuntu-latest for security workflow (#9256) by @Boshen - notify Discord around release publish (#9251) by @Boshen - add release environment to npm publish workflow (#9250) by @Boshen - justfile: drop the `--` separator before forwarded args in `vp run` (#9246) by @shulaoda - deps: update test262 submodule for tests (#9243) by @sapphi-red - add more tracing instrumentations (#9220) by @sapphi-red - rolldown_plugin_vite_import_glob: remove outdated sourcemap doc comment (#9213) by @shulaoda - update security workflow (#9201) by @Boshen ### ❤️ New Contributors * @lazarv made their first contribution in [#9031](#9031) Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the bug where optional-chained accesses to a cross-module enum (
E?.x,E?.['x']) referenced an undeclared variable at runtime because the enum's declaration had been tree-shaken away.Cause
The tree-shaker in
include_statementwas bypassing inclusion of an enum's declaration for any single-property member access on it, assuming the finalizer would inline the access to a literal. But the finalizer'stry_inline_enum_accessonly matches bareStaticMemberExpression/ComputedMemberExpression— not theChainExpressionproduced by optional chaining. The two phases disagreed: the tree-shaker dropped the IIFE, the finalizer left the?.xreference intact, and the output crashed withReferenceError.Fix
Tighten the bypass condition in
crates/rolldown/src/stages/link_stage/tree_shaking/include_statements.rsso it only fires when the access is actually inlinable:!prop.optional— fix for [Bug]: optional chain access to an enum declared in a different file compiles wrongly #9181: optional chains aren't inlined.!member_expr_ref.is_write— defensive: write targets (E.x = …) flow throughvisit_simple_assignment_targetin the finalizer and aren't inlined either.The existing esbuild conformance test
ts/ts_enum_cross_module_inlining_accessalready covers both inlined (E.x,E['x']) and not-inlined (E?.x,E?.['x'], bareE) accesses; its snapshot is updated to reflect the corrected output anddiff.mdis regenerated.Closes #9181
Test plan
cargo test -p rolldown --test integration ts_enum_cross_module_inlining_accessscripts/.../snap-diffoutput;diff.mdnow shows only stylistic differences vs esbuild (varvslet, arrow vsfunctionIIFE form)