fix: is_top_level incorrectly treats strict-mode scopes as top-level#8878
Merged
fix: is_top_level incorrectly treats strict-mode scopes as top-level#8878
is_top_level incorrectly treats strict-mode scopes as top-level#8878Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
b6f24e1 to
5e00ad6
Compare
5e00ad6 to
f926213
Compare
Merging this PR will not alter performance
Comparing Footnotes
|
IWANABETHATGUY
approved these changes
Mar 23, 2026
This was referenced Mar 25, 2026
Closed
Merged
shulaoda
added a commit
that referenced
this pull request
Mar 25, 2026
## [1.0.0-rc.12] - 2026-03-25 ### 🚀 Features - chunk-optimizer: skip circular dependency check when strict execution order is enabled (#8886) by @hyf0 ### 🐛 Bug Fixes - emit build warnings during watch mode rebuilds (#8897) by @IWANABETHATGUY - lazy-barrel: load import-then-export specifiers when barrel has local exports (#8895) by @shulaoda - correct execution order of transferred CJS init calls (#8877) by @IWANABETHATGUY - mcs: `entriesAware` should calculate sizes without duplication (#8887) by @hyf0 - non-deterministic chunk generation (#8882) by @sapphi-red - `is_top_level` incorrectly treats strict-mode scopes as top-level (#8878) by @Dunqing ### 🚜 Refactor - treeshake: migrate SideEffectDetector to Oxc's MayHaveSideEffects trait (#8624) by @Dunqing ### 🧪 Testing - make dev server tests deterministic by replacing fixed sleeps with event-driven polling (#8561) by @Boshen ### ⚙️ Miscellaneous Tasks - deps: update dependency vite-plus to v0.1.14 (#8902) by @camc314 - deps: update dependency oxfmt to ^0.42.0 (#8891) by @renovate[bot] - deps: update rust crate oxc_sourcemap to v6.1.1 (#8890) by @renovate[bot] - remove Rolldown MF plan (#8883) by @shulaoda - deps: update rollup submodule for tests to v4.60.0 (#8881) by @sapphi-red - deps: update test262 submodule for tests (#8880) by @sapphi-red - deps: upgrade oxc crates to 0.122.0 (#8879) by @shulaoda Co-authored-by: shulaoda <[email protected]>
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
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.
Closes #8793
Summary
Fix
is_top_level()incrates/rolldown_ecmascript_utils/src/scope.rswhich incorrectly treated scopes withScopeFlags::StrictModeas "top-level" (eagerly evaluated during module evaluation).Root Cause
is_top_level()determines whether the current AST position executes eagerly during module evaluation (as opposed to being deferred inside a function). It checks that all scopes in the stack are "transparent" — i.e.,Top,ClassStaticBlock, or empty block scopes.The bug was that
ScopeFlags::StrictModewas included in the transparency check viaintersects:StrictModeis a modifier flag inherited by child scopes, not a scope boundary indicator. An arrow function with"use strict"gets scope flags likeArrow | Function | StrictMode— theintersects(StrictMode)check made this appear "transparent", so code inside the arrow function was incorrectly considered top-level.How This Causes Colliding Variable Names
Given this input:
The chain of failure:
is_top_level()returnstrueinside the arrow function — because itsStrictModeflag matches theintersectscheckreturn 0as a top-level return — top-levelreturnis only valid in CommonJS, so the module getsExportsKind::CommonJs__commonJSMin(() => { ... })closurevar Ub(array) is NOT registered in the root scope renamerUb— since the localvar Ubwasn't registered, no conflict is detected, and the imported function keeps the nameUbstatic p = Ub({})resolves to the local array["open", "opening", "toggle"]instead of the imported function, causingTypeError: Ub is not a functionFix
This correctly handles all cases:
Topafter stripping → transparent ✓Arrow|Functionafter stripping → NOT transparent ✓Test plan
crates/rolldown/tests/rolldown/issues/8793/rolldown_ecmascript_utilsunit tests pass🤖 Generated with Claude Code