Skip to content

refactor(rust): introduce StableModuleId type#7715

Merged
graphite-app[bot] merged 1 commit intomainfrom
12-30-refactor_rust_introduce_stablemoduleid_type
Dec 30, 2025
Merged

refactor(rust): introduce StableModuleId type#7715
graphite-app[bot] merged 1 commit intomainfrom
12-30-refactor_rust_introduce_stablemoduleid_type

Conversation

@hyf0
Copy link
Member

@hyf0 hyf0 commented Dec 30, 2025

No description provided.

Copy link
Member Author

hyf0 commented Dec 30, 2025

@hyf0 hyf0 marked this pull request as ready for review December 30, 2025 14:15
Copilot AI review requested due to automatic review settings December 30, 2025 14:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a new StableModuleId type to replace raw String values for stabilized module identifiers, improving type safety and making the distinction between module IDs and their stabilized versions explicit at the type level.

  • Introduces StableModuleId type with stabilization logic (path normalization, virtual module escaping)
  • Updates ModuleId::stabilize() to return StableModuleId instead of String
  • Updates all usage sites to work with the new type

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/rolldown_common/src/types/stable_module_id.rs New file defining StableModuleId struct with stabilization logic, trait implementations, and comprehensive tests
crates/rolldown_common/src/types/module_id.rs Updated stabilize() method to return StableModuleId instead of String
crates/rolldown_common/src/types/mod.rs Added module declaration for stable_module_id
crates/rolldown_common/src/module/normal_module.rs Changed stable_id field type from String to StableModuleId
crates/rolldown_common/src/lib.rs Added StableModuleId to public exports
crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs Updated to use .to_string() or .as_str() for converting StableModuleId to string where needed
crates/rolldown/src/module_loader/runtime_module_task.rs Updated to construct StableModuleId directly using new()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@graphite-app graphite-app bot force-pushed the 12-30-refactor_rust_reduce_unnecessary_id.as_arc_str_.clone_.into_ branch from 2f05628 to d0fcb2f Compare December 30, 2025 14:30
@graphite-app graphite-app bot force-pushed the 12-30-refactor_rust_introduce_stablemoduleid_type branch from 7b4dd89 to 77de7e4 Compare December 30, 2025 14:30
@hyf0 hyf0 force-pushed the 12-30-refactor_rust_introduce_stablemoduleid_type branch from 77de7e4 to afb9fe2 Compare December 30, 2025 14:41
@hyf0 hyf0 force-pushed the 12-30-refactor_rust_reduce_unnecessary_id.as_arc_str_.clone_.into_ branch from d0fcb2f to 4267c5d Compare December 30, 2025 14:41
@github-actions
Copy link
Contributor

github-actions bot commented Dec 30, 2025

Benchmarks Rust

  • target: main(d35c43b)
  • pr: 12-30-refactor_rust_introduce_stablemoduleid_type(70765f3)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.01     61.7±1.12ms        ? ?/sec    1.00     61.1±1.20ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     68.0±1.25ms        ? ?/sec    1.00     68.3±1.86ms        ? ?/sec
bundle/bundle@rome_ts                                        1.01    104.7±1.65ms        ? ?/sec    1.00    104.0±1.67ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.01    116.5±2.00ms        ? ?/sec    1.00    115.9±1.47ms        ? ?/sec
bundle/bundle@threejs                                        1.01     37.8±2.33ms        ? ?/sec    1.00     37.6±1.98ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.01     42.3±1.03ms        ? ?/sec    1.00     41.9±0.57ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    384.5±2.54ms        ? ?/sec    1.00    383.5±3.69ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.01    447.4±4.44ms        ? ?/sec    1.00    444.3±5.38ms        ? ?/sec
scan/scan@rome_ts                                            1.00     83.8±1.53ms        ? ?/sec    1.01     84.5±1.77ms        ? ?/sec
scan/scan@threejs                                            1.00     28.7±1.74ms        ? ?/sec    1.00     28.6±1.70ms        ? ?/sec
scan/scan@threejs10x                                         1.00    290.6±4.10ms        ? ?/sec    1.00    290.5±3.29ms        ? ?/sec

@graphite-app graphite-app bot force-pushed the 12-30-refactor_rust_reduce_unnecessary_id.as_arc_str_.clone_.into_ branch 2 times, most recently from 7e360fa to e55767d Compare December 30, 2025 15:12
@graphite-app graphite-app bot force-pushed the 12-30-refactor_rust_introduce_stablemoduleid_type branch from afb9fe2 to 8a366c5 Compare December 30, 2025 15:12
@hyf0 hyf0 force-pushed the 12-30-refactor_rust_introduce_stablemoduleid_type branch from 8a366c5 to d5e76e3 Compare December 30, 2025 19:04
@graphite-app
Copy link
Contributor

graphite-app bot commented Dec 30, 2025

Merge activity

@graphite-app graphite-app bot changed the base branch from 12-30-refactor_rust_reduce_unnecessary_id.as_arc_str_.clone_.into_ to graphite-base/7715 December 30, 2025 22:00
@graphite-app graphite-app bot changed the base branch from graphite-base/7715 to main December 30, 2025 22:11
@graphite-app graphite-app bot force-pushed the 12-30-refactor_rust_introduce_stablemoduleid_type branch from d5e76e3 to 70765f3 Compare December 30, 2025 22:12
@netlify
Copy link

netlify bot commented Dec 30, 2025

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 70765f3
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69544e4f8e51600008260c7f

@graphite-app graphite-app bot merged commit 70765f3 into main Dec 30, 2025
40 checks passed
@graphite-app graphite-app bot deleted the 12-30-refactor_rust_introduce_stablemoduleid_type branch December 30, 2025 22:24
This was referenced Dec 31, 2025
shulaoda added a commit that referenced this pull request Dec 31, 2025
## [1.0.0-beta.58] - 2025-12-31

### 💥 BREAKING CHANGES

- experimental/devtools: rename InputOptions#debug to InputOptions#devtools (#7686) by @Copilot

### 🚀 Features

- implement target feature check in `should_transform_js` for raw options (#7697) by @shulaoda
- support `output.dynamicImportInCjs` option (#7677) by @shulaoda
- types: expose `ChecksOptions` type (#7653) by @sapphi-red

### 🐛 Bug Fixes

- export runtime helpers for cross-chunk access (#7658) by @shulaoda
- cjs namespace merging regression (#7665) by @IWANABETHATGUY
- replace panic with proper error handling for hash placeholder generation (#7661) by @shulaoda
- remove the blank line between shebang and postBanner (#7643) by @btea
- rolldown_plugin_vite_reporter: apply padding before ANSI coloring for proper size column alignment (#7649) by @shulaoda

### 🚜 Refactor

- rust: use `StableModuleId` as the map key if possible (#7718) by @hyf0
- rust: return `StableModuleId` instead of `&str` from `Module#stable_id()` (#7717) by @hyf0
- rust: return correct stable id of external module from `Module#stable_id()` (#7716) by @hyf0
- rust: introduce `StableModuleId` type (#7715) by @hyf0
- rust: reduce unnecessary `id.as_arc_str().clone().into()` (#7714) by @hyf0
- rust: remove `ModuleId#resource_id` and use `as_arc_str` directly (#7710) by @hyf0
- rust: remove unused `Module#id_clone` (#7709) by @hyf0
- rust: remove `Module#id_as_str` and use `Module#id` directly (#7708) by @hyf0
- consolidate namespace call analysis into import analyzer (#7657) by @IWANABETHATGUY
- rust: make `ExternalModule#id` have the type `ModuleId` (#7707) by @hyf0
- rust: rename `Module#id` to `Module#id_as_str` (#7706) by @hyf0
- rust: use `ModuleId` instead of raw `ArcStr` for `ScanStageCache` (#7701) by @hyf0
- simplify error propagation in cache merge (#7702) by @shulaoda
- use `ModuleId` as the type of `ResolvedId#id` (#7694) by @hyf0
- types: rename `resolved_request_info.rs` to `resolved_id.rs` and move its contents (#7687) by @hyf0
- devtools: emit data to `<CWD>/node_modules/.rolldown` (#7692) by @hyf0
- use `InvalidOption` for hash placeholder generation errors (#7674) by @shulaoda
- rolldown_error: remove dependency on rolldown_utils (#7672) by @shulaoda
- use nodejs-built-in-modules v1.0.0 directly in callsites (#7667) by @Boshen

### 📚 Documentation

- migrate input options content from options to auto gen docs (#7663) by @mdong1909
- create reference index page (#7659) by @mdong1909
- tweak auto-generated reference output (#7654) by @sapphi-red
- initialize auto-gen docs (#7252) by @mdong1909

### ⚙️ Miscellaneous Tasks

- deps: update napi (#7705) by @renovate[bot]
- pin Node.js version to 24.12.0 LTS in .node-version file (#7713) by @Copilot
- update esbuild test reasons (#7703) by @sapphi-red
- deps: update crate-ci/typos action to v1.40.1 (#7696) by @renovate[bot]
- deps: update oxc to v0.106.0 (#7512) by @renovate[bot]
- js: replace dprint with oxfmt (#7214) by @Boshen
- deps: update dependency oxlint to v1.36.0 (#7691) by @renovate[bot]
- deps: update github-actions (#7679) by @renovate[bot]
- deps: update npm packages (#7680) by @renovate[bot]
- deps: update rust crates (#7678) by @renovate[bot]
- deps: update oxc resolver to v11.16.2 (#7668) by @renovate[bot]
- add API reference files to knip entry points (#7669) by @Copilot
- deps: update notify (#7651) by @sapphi-red
- add `homepage` field to package.json (#7648) by @trivikr
- deps: update oxc resolver to v11.16.1 (#7647) by @renovate[bot]
- deps: update rolldown-plugin-dts to 0.20.0 (#7645) by @shulaoda

Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
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.

3 participants