Skip to content

Comments

refactor(rolldown_resolver): tweak code structure#6978

Closed
shulaoda wants to merge 1 commit intomainfrom
11-13-refactor_rolldown_resolver_tweak_code_structure
Closed

refactor(rolldown_resolver): tweak code structure#6978
shulaoda wants to merge 1 commit intomainfrom
11-13-refactor_rolldown_resolver_tweak_code_structure

Conversation

@shulaoda
Copy link
Member

@shulaoda shulaoda commented Nov 13, 2025 •
edited
Loading

  • Improve the resolver error handle logic
  • Split resolver.rs into two separate files

@shulaoda Graphite App
Copy link
Member Author

shulaoda commented Nov 13, 2025

How to use the Graphite Merge Queue

Add the label graphite: merge 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.

@netlify
Copy link

netlify bot commented Nov 13, 2025 •
edited
Loading

✅ Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 3058407
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69154c68e6d52e000877ec74

@shulaoda shulaoda marked this pull request as ready for review November 13, 2025 03:04
@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 13, 2025 •
edited
Loading

Merge activity

  • Nov 13, 3:04 AM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
  • Nov 13, 3:04 AM UTC: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..

@shulaoda shulaoda force-pushed the 11-13-refactor_rolldown_resolver_tweak_code_structure branch from 7e98b3a to f152cd8 Compare November 13, 2025 03:07
@shulaoda shulaoda force-pushed the 11-13-refactor_rolldown_resolver_tweak_code_structure branch from f152cd8 to 3058407 Compare November 13, 2025 03:11
@github-actions
Copy link
Contributor

github-actions bot commented Nov 13, 2025

Benchmarks Rust

  • target: main(49ce17e)
  • pr: 11-13-refactor_rolldown_resolver_tweak_code_structure(3058407)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.02     65.6±1.74ms        ? ?/sec    1.00     64.1±1.78ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.02     72.3±1.56ms        ? ?/sec    1.00     70.7±2.05ms        ? ?/sec
bundle/bundle@rome_ts                                        1.01    109.3±2.46ms        ? ?/sec    1.00    107.8±3.01ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.01    122.7±1.98ms        ? ?/sec    1.00    121.3±2.38ms        ? ?/sec
bundle/bundle@threejs                                        1.00     40.4±2.45ms        ? ?/sec    1.00     40.3±2.34ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     43.0±0.62ms        ? ?/sec    1.00     42.9±0.87ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    395.0±5.78ms        ? ?/sec    1.00    394.9±7.99ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.01    457.3±5.73ms        ? ?/sec    1.00    454.1±5.89ms        ? ?/sec
scan/scan@rome_ts                                            1.01     86.4±2.08ms        ? ?/sec    1.00     85.7±1.50ms        ? ?/sec
scan/scan@threejs                                            1.02     29.2±0.55ms        ? ?/sec    1.00     28.5±1.36ms        ? ?/sec
scan/scan@threejs10x                                         1.01    300.2±5.20ms        ? ?/sec    1.00    296.5±5.96ms        ? ?/sec

@hyf0
Copy link
Member

hyf0 commented Nov 13, 2025

@copilot Move ResolverConfig to resolver_config.rs and Resolver to resolver.rs.

@hyf0 hyf0 self-assigned this Nov 13, 2025

This comment was marked as off-topic.

Sign in to view
Copilot AI mentioned this pull request Nov 13, 2025
Closed
@hyf0
Copy link
Member

hyf0 commented Nov 13, 2025 •
edited
Loading

Improve the resolver error handle logic

Would be nice to split the refactor and improvement into different PRs if possible. Moving a file causes everything changed, and it's hard to distinguish what part code is new added.

Split resolver.rs into two separate files

Good job on this one and a few thoughts:

  • rolldown_resolver doesn't have many files, so you don't need to create extra resolver folder.
  • use resolver_config.rs instead config.rs. The underlying rule is that file name should match the main struct's in that file.
  • Keep Resolver under resolver.rs.

And why we want to do this?

Because, this gives you a clear mindset when you want to inspect some structs or jump into some files.

When you want to find ResolverConfig struct, you know that it's stored in resolver_config.rs and you just type resolver_config. config is a good semantic name, but it's not good when you want to navigate between files.

@shulaoda shulaoda marked this pull request as draft November 13, 2025 08:22
This was referenced Nov 13, 2025
Merged
Merged
graphite-app bot pushed a commit that referenced this pull request Nov 13, 2025
@shulaoda
refactor(rolldown_resolver): improve error messages (#6982)
710a785 
Related to #6978
graphite-app bot pushed a commit that referenced this pull request Nov 13, 2025
@shulaoda
refactor(rolldown_resolver): extract configuration logic into separat…
3a5452d 
…e module (#6983)

Related to #6978
@shulaoda shulaoda mentioned this pull request Nov 13, 2025
Merged
@shulaoda shulaoda closed this Nov 13, 2025
@shulaoda shulaoda deleted the 11-13-refactor_rolldown_resolver_tweak_code_structure branch November 13, 2025 14:45
shulaoda added a commit that referenced this pull request Nov 13, 2025
@shulaoda
refactor(rolldown_resolver): improve resolve method clarity and docum…
ac23895 
…entation (#6986)

closes #6978

---------

Signed-off-by: dalaoshu <[email protected]>
Co-authored-by: Copilot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hyf0 hyf0 Awaiting requested review from hyf0

@IWANABETHATGUY IWANABETHATGUY Awaiting requested review from IWANABETHATGUY

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Assignees

@hyf0 hyf0

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shulaoda @hyf0