refactor: use `NormalizedId` to ensure consistent behavior for id by hyf0 · Pull Request #7688 · rolldown/rolldown

hyf0 · 2025-12-29T07:49:17Z

NormalizedId is a temporary abstraction to represent normalized resolved id, but if we call it NormalizedResolvedId, it may confuse with ResolvedId.

hyf0 · 2025-12-29T07:49:37Z

refactor: use NormalizedId to ensure consistent behavior for id #7688 : 2 dependent PRs (#7638 , #7693 ) 👈 (View in Graphite)
refactor(types): rename resolved_request_info.rs to resolved_id.rs and move its contents #7687
main

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 introduces a new NormalizedId type to ensure consistent handling of module identifiers across the codebase. The key change is wrapping the id field in ResolvedId with NormalizedId, which automatically normalizes Windows paths by converting backslashes to forward slashes for absolute paths.

Key changes:

New NormalizedId type that wraps ArcStr and performs path normalization on Windows
Refactored ResolvedId.id from ArcStr to NormalizedId
Updated all usages throughout the codebase to work with the new type using .as_str() and .as_arc_str() accessor methods

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`crates/rolldown_common/src/types/normalized_id.rs`	New type definition for `NormalizedId` with Windows path normalization logic and trait implementations
`crates/rolldown_common/src/types/mod.rs`	Adds the new `normalized_id` module to exports
`crates/rolldown_common/src/lib.rs`	Exports `NormalizedId` for public use
`crates/rolldown_common/src/types/resolved_id.rs`	Changes `id` field type from `ArcStr` to `NormalizedId`, updates usages to call `.as_str()`
`crates/rolldown_resolver/src/resolver.rs`	Wraps resolved path with `NormalizedId::new()` when creating `ResolvedId`
`crates/rolldown_plugin/src/utils/resolve_id_with_plugins.rs`	Updates all `ResolvedId` constructions to use `NormalizedId::new()`
`crates/rolldown_plugin/src/utils/resolve_id_check_external.rs`	Wraps external IDs with `NormalizedId::new()`
`crates/rolldown_plugin/src/types/hook_resolve_id_output.rs`	Accesses `id` via `.as_arc_str().clone()` instead of direct field access
`crates/rolldown_plugin/src/plugin_context/native_plugin_context.rs`	Wraps specifier with `NormalizedId::new()` when creating `ResolvedId`
`crates/rolldown_plugin_vite_import_glob/src/utils.rs`	Converts `resolved_id.id` to string using `.to_string()` instead of `.into()`
`crates/rolldown_plugin_vite_import_glob/src/utils_2.rs`	Converts `resolved_id.id` to string using `.to_string()` instead of `.into()`
`crates/rolldown/src/module_loader/resolve_utils.rs`	Updates all ID accesses to use `.as_str()` or `.as_arc_str().clone()`
`crates/rolldown/src/module_loader/runtime_module_task.rs`	Wraps runtime module ID with `NormalizedId::new()`
`crates/rolldown/src/module_loader/module_task.rs`	Updates ID accesses to use `.as_str()` or `.as_arc_str().clone()`
`crates/rolldown/src/module_loader/module_loader.rs`	Updates all map lookups and insertions to use `.as_str()` or `.as_arc_str().clone()`
`crates/rolldown/src/module_loader/external_module_task.rs`	Adds explicit type annotation for `ArcStr` and uses accessor methods
`crates/rolldown/src/ecmascript/ecma_module_view_factory.rs`	Uses `.as_str()` when creating `ModuleId`
`crates/rolldown/tests/rolldown/plugin/plugin_context/custom_arg_in_resolve/mod.rs`	Uses `.as_str()` for string comparison in test

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

Copilot · 2025-12-29T07:51:55Z

crates/rolldown_common/src/types/normalized_id.rs

@@ -0,0 +1,50 @@
+// This's a temporary abstraction to represent `normalized` resolved id, but if we call it `NormalizedResolvedId`, it may confuse with `ResolvedId`.


The comment contains a grammatical error. "This's" should be "This is".

Suggested change

// This's a temporary abstraction to represent `normalized` resolved id, but if we call it `NormalizedResolvedId`, it may confuse with `ResolvedId`.

// This is a temporary abstraction to represent `normalized` resolved id, but if we call it `NormalizedResolvedId`, it may confuse with `ResolvedId`.

Copilot · 2025-12-29T07:51:56Z

crates/rolldown_common/src/types/normalized_id.rs

+#[derive(Debug, Default, Clone, PartialEq, Eq, Hash)]
+pub struct NormalizedId(ArcStr);


According to Rust API Guidelines (C-COMMON-TRAITS), common traits should be implemented where applicable. Consider implementing the PartialOrd and Ord traits for NormalizedId since it wraps ArcStr which implements these traits. This would allow NormalizedId instances to be used in contexts that require ordering, such as BTreeMap keys or sorting collections.

github-actions · 2025-12-29T08:05:24Z

Benchmarks Rust

target: main(938617b)
pr: 12-29-refactor_use_normalizedid_to_ensure_consistent_behavior_for_id(22353de)

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.07     65.9±1.52ms        ? ?/sec    1.00     61.6±1.49ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.09     73.6±1.71ms        ? ?/sec    1.00     67.6±1.09ms        ? ?/sec
bundle/bundle@rome_ts                                        1.02    107.7±2.17ms        ? ?/sec    1.00    105.8±1.55ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.03    121.3±2.81ms        ? ?/sec    1.00    118.1±1.66ms        ? ?/sec
bundle/bundle@threejs                                        1.01     39.6±2.05ms        ? ?/sec    1.00     39.3±2.28ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.03     43.8±0.71ms        ? ?/sec    1.00     42.3±0.57ms        ? ?/sec
bundle/bundle@threejs10x                                     1.02    401.0±6.91ms        ? ?/sec    1.00    393.5±6.55ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    453.3±5.90ms        ? ?/sec    1.00    454.2±8.27ms        ? ?/sec
scan/scan@rome_ts                                            1.00     85.9±1.68ms        ? ?/sec    1.01     86.8±1.93ms        ? ?/sec
scan/scan@threejs                                            1.02     29.1±1.66ms        ? ?/sec    1.00     28.6±0.49ms        ? ?/sec
scan/scan@threejs10x                                         1.00    299.1±5.30ms        ? ?/sec    1.01    301.5±5.60ms        ? ?/sec

netlify · 2025-12-29T11:34:33Z

✅ Deploy Preview for rolldown-rs canceled.

Name	Link
🔨 Latest commit	`22353de`
🔍 Latest deploy log	https://app.netlify.com/projects/rolldown-rs/deploys/69528130b355d5000873ea9d

Copilot

Pull request overview

Copilot reviewed 19 out of 19 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.

github-actions bot assigned hyf0 Dec 29, 2025

This was referenced Dec 29, 2025

feat(dev/lazy): support dynamic entries #7609

Closed

refactor(dev): use direct module id instead of stable id #7638

Closed

refactor(types): rename resolved_request_info.rs to resolved_id.rs and move its contents #7687

Merged

hyf0 marked this pull request as ready for review December 29, 2025 07:49

Copilot AI review requested due to automatic review settings December 29, 2025 07:49

Copilot started reviewing on behalf of hyf0 December 29, 2025 07:50 View session

Copilot AI reviewed Dec 29, 2025

View reviewed changes

hyf0 force-pushed the 12-29-refactor_types_rename_resolved_request_info.rs_to_resolved_id.rs_and_move_its_contents branch from df2b5a8 to 32127bd Compare December 29, 2025 10:58

hyf0 force-pushed the 12-29-refactor_use_normalizedid_to_ensure_consistent_behavior_for_id branch from 3f00fd2 to aaaea8d Compare December 29, 2025 10:58

graphite-app bot changed the base branch from 12-29-refactor_types_rename_resolved_request_info.rs_to_resolved_id.rs_and_move_its_contents to graphite-base/7688 December 29, 2025 11:18

graphite-app bot force-pushed the 12-29-refactor_use_normalizedid_to_ensure_consistent_behavior_for_id branch from aaaea8d to a3db8c3 Compare December 29, 2025 11:32

graphite-app bot force-pushed the graphite-base/7688 branch from 32127bd to 10262da Compare December 29, 2025 11:32

graphite-app bot changed the base branch from graphite-base/7688 to main December 29, 2025 11:33

graphite-app bot force-pushed the 12-29-refactor_use_normalizedid_to_ensure_consistent_behavior_for_id branch from a3db8c3 to cbe5fe8 Compare December 29, 2025 11:33

Copilot AI review requested due to automatic review settings December 29, 2025 11:55

hyf0 force-pushed the 12-29-refactor_use_normalizedid_to_ensure_consistent_behavior_for_id branch from cbe5fe8 to 30cbc23 Compare December 29, 2025 11:55

Copilot started reviewing on behalf of hyf0 December 29, 2025 11:55 View session

Copilot AI reviewed Dec 29, 2025

View reviewed changes

refactor: use NormalizedId to ensure consistent behavior for id

22353de

hyf0 mentioned this pull request Dec 29, 2025

refactor(devtools): replace NormalizedId with ModuleId #7693

Closed

hyf0 force-pushed the 12-29-refactor_use_normalizedid_to_ensure_consistent_behavior_for_id branch from 30cbc23 to 22353de Compare December 29, 2025 13:25

hyf0 closed this Dec 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: use `NormalizedId` to ensure consistent behavior for id#7688

refactor: use `NormalizedId` to ensure consistent behavior for id#7688
hyf0 wants to merge 1 commit intomainfrom
12-29-refactor_use_normalizedid_to_ensure_consistent_behavior_for_id

hyf0 commented Dec 29, 2025 •

edited

Loading

Uh oh!

hyf0 commented Dec 29, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 29, 2025

Uh oh!

Copilot AI Dec 29, 2025

Uh oh!

github-actions bot commented Dec 29, 2025 •

edited

Loading

Uh oh!

netlify bot commented Dec 29, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,50 @@
		// This's a temporary abstraction to represent `normalized` resolved id, but if we call it `NormalizedResolvedId`, it may confuse with `ResolvedId`.

	// This's a temporary abstraction to represent `normalized` resolved id, but if we call it `NormalizedResolvedId`, it may confuse with `ResolvedId`.
	// This is a temporary abstraction to represent `normalized` resolved id, but if we call it `NormalizedResolvedId`, it may confuse with `ResolvedId`.

		#[derive(Debug, Default, Clone, PartialEq, Eq, Hash)]
		pub struct NormalizedId(ArcStr);

Conversation

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

Uh oh!

Uh oh!

hyf0 commented Dec 29, 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

Reviewed changes

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks Rust

Uh oh!

netlify bot commented Dec 29, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hyf0 commented Dec 29, 2025 •

edited

Loading

hyf0 commented Dec 29, 2025 •

edited

Loading

github-actions bot commented Dec 29, 2025 •

edited

Loading

netlify bot commented Dec 29, 2025 •

edited

Loading