Skip to content

refactor(rust): rename Module#id to Module#id_as_str#7706

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

refactor(rust): rename Module#id to Module#id_as_str#7706
graphite-app[bot] merged 1 commit intomainfrom
12-30-refactor_rust_rename_module_id_to_module_id_as_str_

Conversation

@hyf0
Copy link
Member

@hyf0 hyf0 commented Dec 30, 2025

id() should return ModuleId

Copy link
Member Author

hyf0 commented Dec 30, 2025

@netlify
Copy link

netlify bot commented Dec 30, 2025

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit f26c993
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69539562c23e0000089e3c5b

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 renames the Module#id() method to Module#id_as_str() in preparation for a future change where id() should return ModuleId instead of &str. This is a purely mechanical refactoring that updates the method definition and all its call sites across the codebase.

  • Renamed Module::id() to Module::id_as_str() in the core module definition
  • Updated all 30+ call sites across the codebase to use the new method name
  • Maintained the same functionality (returning &str)

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/rolldown_common/src/module/mod.rs Renamed the method definition from id() to id_as_str()
crates/rolldown/src/utils/chunk/mod.rs Updated method calls for facade module ID and module IDs collection
crates/rolldown/src/types/scan_stage_cache.rs Updated method call in module ID lookup
crates/rolldown/src/types/generator.rs Updated method call in error message formatting
crates/rolldown/src/stages/link_stage/tree_shaking/include_statements.rs Updated method call in debug tracing
crates/rolldown/src/stages/link_stage/mod.rs Updated method call in entry points sorting
crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs Updated method calls in error diagnostic construction
crates/rolldown/src/stages/generate_stage/mod.rs Updated multiple method calls for chunk naming and module ID retrieval
crates/rolldown/src/stages/generate_stage/compute_cross_chunk_links.rs Updated method calls in module ID collection and error messages
crates/rolldown/src/module_loader/module_loader.rs Updated method call in importer record creation
crates/rolldown/src/module_finalizers/mod.rs Updated method call in require call generation
crates/rolldown/src/hmr/hmr_stage.rs Updated method calls in module sorting for HMR updates
crates/rolldown/src/chunk_graph.rs Updated method calls in runtime module detection and sorting
crates/rolldown/src/bundle/bundle.rs Updated method call in module import action creation

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

@hyf0 hyf0 force-pushed the 12-30-refactor_rust_rename_module_id_to_module_id_as_str_ branch from 130f3bf to b4ff474 Compare December 30, 2025 08:43
@github-actions
Copy link
Contributor

Benchmarks Rust

  • target: main(246ff3d)
  • pr: 12-30-refactor_rust_rename_module_id_to_module_id_as_str_(b4ff474)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     65.1±1.45ms        ? ?/sec    1.00     65.1±1.39ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     71.9±1.45ms        ? ?/sec    1.00     71.9±1.65ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    109.5±3.54ms        ? ?/sec    1.00    109.0±1.55ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    120.8±1.65ms        ? ?/sec    1.00    121.4±1.68ms        ? ?/sec
bundle/bundle@threejs                                        1.01     40.1±1.95ms        ? ?/sec    1.00     39.8±0.62ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     43.8±0.69ms        ? ?/sec    1.00     43.6±0.61ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    401.1±5.99ms        ? ?/sec    1.01    403.6±4.85ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    460.0±3.79ms        ? ?/sec    1.00    460.9±4.65ms        ? ?/sec
scan/scan@rome_ts                                            1.00     83.7±1.38ms        ? ?/sec    1.00     83.7±1.39ms        ? ?/sec
scan/scan@threejs                                            1.01     28.8±1.74ms        ? ?/sec    1.00     28.6±1.66ms        ? ?/sec
scan/scan@threejs10x                                         1.00    295.2±4.17ms        ? ?/sec    1.00    294.9±4.16ms        ? ?/sec

@graphite-app
Copy link
Contributor

graphite-app bot commented Dec 30, 2025

Merge activity

@graphite-app graphite-app bot force-pushed the 12-30-refactor_rust_rename_module_id_to_module_id_as_str_ branch from b4ff474 to f26c993 Compare December 30, 2025 09:03
@graphite-app graphite-app bot merged commit f26c993 into main Dec 30, 2025
33 checks passed
@graphite-app graphite-app bot deleted the 12-30-refactor_rust_rename_module_id_to_module_id_as_str_ branch December 30, 2025 09:21
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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments