refactor(rust): remove `ModuleId#resource_id` and use `as_arc_str` directly by hyf0 · Pull Request #7710 · rolldown/rolldown

hyf0 · 2025-12-30T08:49:50Z

It's meaningless now
resource_id concept is related to import attributes
let's only reconsider this abstraction when we're working on import attributes

hyf0 · 2025-12-30T08:50:19Z

How to use the Graphite Merge Queue

Add the label graphite: merge-when-ready to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copilot

Pull request overview

This PR simplifies the ModuleId API by removing the resource_id abstraction layer. The changes rename the internal field from resource_id to inner and remove the resource_id() method, with all call sites updated to use as_arc_str() directly. This refactoring aligns with the plan to reconsider the resource_id abstraction only when working on import attributes support.

Key Changes

Renamed ModuleId::resource_id field to inner for simplicity
Removed ModuleId::resource_id() accessor method
Updated all call sites across the codebase to use as_arc_str() instead

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
crates/rolldown_common/src/types/module_id.rs	Core change: renamed field and removed resource_id() method, updated all internal method implementations
crates/rolldown_plugin_vite_html/src/lib.rs	Updated facade_module_id comparison to use as_arc_str()
crates/rolldown_plugin_vite_css_post/src/utils.rs	Updated CSS request check to use as_arc_str()
crates/rolldown_plugin_vite_css_post/src/lib.rs	Updated module_id access for CSS processing to use as_arc_str()
crates/rolldown_plugin_chunk_import_map/src/lib.rs	Updated hashing logic to use as_arc_str()
crates/rolldown_plugin/src/plugin_driver/mod.rs	Updated module info insertion to use as_arc_str()
crates/rolldown/src/types/scan_stage_cache.rs	Updated module path indexing to use as_arc_str()
crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs	Updated diagnostic creation to use as_arc_str()
crates/rolldown/src/ast_scanner/namespace_call_analyzer.rs	Updated diagnostic creation to use as_arc_str()
crates/rolldown/src/ast_scanner/import_assign_analyzer.rs	Updated diagnostic creation to use as_arc_str()
crates/rolldown/src/ast_scanner/impl_visit.rs	Updated resource id parsing to use as_arc_str()

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

crates/rolldown_plugin_chunk_import_map/src/lib.rs

crates/rolldown_plugin_vite_css_post/src/utils.rs

crates/rolldown_plugin_vite_css_post/src/lib.rs

crates/rolldown_plugin_chunk_import_map/src/lib.rs

hyf0 · 2025-12-30T09:03:27Z

Addressed all review comments:

Simplified .as_arc_str().as_str() to .as_str() in all 5 locations:
- crates/rolldown_plugin_chunk_import_map/src/lib.rs (lines 53, 62)
- crates/rolldown_plugin_vite_css_post/src/lib.rs (lines 188, 213)
- crates/rolldown_plugin_vite_css_post/src/utils.rs (line 202)

github-actions · 2025-12-30T09:19:25Z

Benchmarks Rust

target: 12-30-refactor_rust_remove_unused_module_id_clone_(d13972d)
pr: 12-30-refactor_rust_remove_moduleid_resource_id_and_use_as_arc_str_directly(12bc698)

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.04     64.9±2.04ms        ? ?/sec    1.00     62.5±1.35ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.01     69.6±2.12ms        ? ?/sec    1.00     68.9±1.24ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    104.9±1.67ms        ? ?/sec    1.02    107.4±1.64ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    117.3±1.71ms        ? ?/sec    1.01    119.0±1.65ms        ? ?/sec
bundle/bundle@threejs                                        1.00     38.5±2.35ms        ? ?/sec    1.00     38.4±1.11ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     42.4±0.67ms        ? ?/sec    1.00     42.6±0.73ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    386.0±2.82ms        ? ?/sec    1.02    392.3±4.03ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    448.3±3.61ms        ? ?/sec    1.01    452.9±4.72ms        ? ?/sec
scan/scan@rome_ts                                            1.00     84.4±1.48ms        ? ?/sec    1.00     84.5±2.61ms        ? ?/sec
scan/scan@threejs                                            1.00     28.8±1.72ms        ? ?/sec    1.00     28.7±1.69ms        ? ?/sec
scan/scan@threejs10x                                         1.00    292.8±3.47ms        ? ?/sec    1.00    291.3±4.48ms        ? ?/sec

hyf0 · 2025-12-30T09:20:41Z

Merge activity

Dec 30, 9:20 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Dec 30, 10:14 AM UTC: hyf0 added this pull request to the Graphite merge queue.
Dec 30, 10:18 AM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #7708.
Dec 30, 12:12 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Dec 30, 3:02 PM UTC: hyf0 added this pull request to the Graphite merge queue.
Dec 30, 3:11 PM UTC: Merged by the Graphite merge queue.

netlify · 2025-12-30T14:42:59Z

✅ Deploy Preview for rolldown-rs canceled.

Name	Link
🔨 Latest commit	`8afbf65`
🔍 Latest deploy log	https://app.netlify.com/projects/rolldown-rs/deploys/6953e97f6be7d100087156f2

Copilot

Pull request overview

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

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

…rectly (#7710) - It's meaningless now - `resource_id` concept is related to import attributes - let's only reconsider this abstraction when we're working on import attributes

@shulaoda

## [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]>

github-actions bot assigned hyf0 Dec 30, 2025

This was referenced Dec 30, 2025

refactor(rust): remove unused Module#id_clone #7709

Merged

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

Merged

refactor(rust): make ExternalModule#id have the type ModuleId #7707

Merged

hyf0 mentioned this pull request Dec 30, 2025

refactor(rust): remove Module#id_as_str and use Module#id directly #7708

Merged

hyf0 marked this pull request as ready for review December 30, 2025 08:51

Copilot AI review requested due to automatic review settings December 30, 2025 08:51

Copilot started reviewing on behalf of hyf0 December 30, 2025 08:51 View session

Copilot AI reviewed Dec 30, 2025

View reviewed changes

hyf0 requested review from IWANABETHATGUY and shulaoda December 30, 2025 09:12

hyf0 added the graphite: merge-when-ready label Dec 30, 2025

IWANABETHATGUY approved these changes Dec 30, 2025

View reviewed changes

graphite-app bot removed the graphite: merge-when-ready label Dec 30, 2025

hyf0 added the graphite: merge-when-ready label Dec 30, 2025

hyf0 force-pushed the 12-30-refactor_rust_remove_unused_module_id_clone_ branch from d13972d to a970fe1 Compare December 30, 2025 14:12

hyf0 force-pushed the 12-30-refactor_rust_remove_moduleid_resource_id_and_use_as_arc_str_directly branch from 12bc698 to 45d66bd Compare December 30, 2025 14:12

This was referenced Dec 30, 2025

refactor(rust): reduce unnecessary id.as_arc_str().clone().into() #7714

Merged

refactor(rust): introduce StableModuleId type #7715

Merged

graphite-app bot force-pushed the 12-30-refactor_rust_remove_moduleid_resource_id_and_use_as_arc_str_directly branch from 45d66bd to 1c65de4 Compare December 30, 2025 14:29

graphite-app bot changed the base branch from 12-30-refactor_rust_remove_unused_module_id_clone_ to main December 30, 2025 14:29

Copilot AI review requested due to automatic review settings December 30, 2025 14:41

hyf0 force-pushed the 12-30-refactor_rust_remove_moduleid_resource_id_and_use_as_arc_str_directly branch from 1c65de4 to 0859859 Compare December 30, 2025 14:41

Copilot started reviewing on behalf of hyf0 December 30, 2025 14:42 View session

Copilot AI reviewed Dec 30, 2025

View reviewed changes

refactor(rust): remove ModuleId#resource_id and use as_arc_str di…

8afbf65

…rectly (#7710) - It's meaningless now - `resource_id` concept is related to import attributes - let's only reconsider this abstraction when we're working on import attributes

graphite-app bot force-pushed the 12-30-refactor_rust_remove_moduleid_resource_id_and_use_as_arc_str_directly branch from 0859859 to 8afbf65 Compare December 30, 2025 15:02

graphite-app bot merged commit 8afbf65 into main Dec 30, 2025
33 checks passed

graphite-app bot deleted the 12-30-refactor_rust_remove_moduleid_resource_id_and_use_as_arc_str_directly branch December 30, 2025 15:11

graphite-app bot removed the graphite: merge-when-ready label Dec 30, 2025

This was referenced Dec 31, 2025

release: v1.0.0-beta.58 #7719

Closed

release: v1.0.0-beta.58 #7721

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(rust): remove `ModuleId#resource_id` and use `as_arc_str` directly#7710

refactor(rust): remove `ModuleId#resource_id` and use `as_arc_str` directly#7710
graphite-app[bot] merged 1 commit intomainfrom
12-30-refactor_rust_remove_moduleid_resource_id_and_use_as_arc_str_directly

hyf0 commented Dec 30, 2025 •

edited

Loading

Uh oh!

hyf0 commented Dec 30, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hyf0 commented Dec 30, 2025

Uh oh!

github-actions bot commented Dec 30, 2025

Uh oh!

hyf0 commented Dec 30, 2025 •

edited by graphite-app bot

Loading

Uh oh!

netlify bot commented Dec 30, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

hyf0 commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hyf0 commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hyf0 commented Dec 30, 2025

Uh oh!

github-actions bot commented Dec 30, 2025

Benchmarks Rust

Uh oh!

hyf0 commented Dec 30, 2025 • edited by graphite-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

netlify bot commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for rolldown-rs canceled.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

hyf0 commented Dec 30, 2025 •

edited

Loading

hyf0 commented Dec 30, 2025 •

edited

Loading

hyf0 commented Dec 30, 2025 •

edited by graphite-app bot

Loading

netlify bot commented Dec 30, 2025 •

edited

Loading