Skip to content

Comments

refactor(rolldown): Remove rolldown_rstr crate and use CompactStr directly#5542

Merged
Boshen merged 1 commit intomainfrom
copilot/fix-2a8a15b2-cceb-49e8-a57b-0708c5db31dd
Aug 1, 2025
Merged

refactor(rolldown): Remove rolldown_rstr crate and use CompactStr directly#5542
Boshen merged 1 commit intomainfrom
copilot/fix-2a8a15b2-cceb-49e8-a57b-0708c5db31dd

Conversation

Copy link
Contributor

Copilot AI commented Jul 31, 2025

This PR removes the intermediate rolldown_rstr crate entirely and migrates the codebase to use oxc::span::CompactStr directly throughout.

Changes Made

  • Removed rolldown_rstr crate: Deleted the entire crate directory and all associated files
  • Updated type definitions: Replaced all Rstr type references with CompactStr across the codebase
  • Simplified string construction: Replaced ToRstr trait usage with direct CompactStr::new() calls
  • Cleaned up dependencies: Removed rolldown_rstr from all Cargo.toml files
  • Fixed imports: Updated import statements throughout to use oxc::span::CompactStr

Benefits

  • Reduced complexity: Eliminates the wrapper layer and simplifies the type system
  • Better integration: Uses oxc's string type directly, improving consistency with the oxc ecosystem
  • Cleaner API: Removes the need for conversion traits and intermediate types
  • Smaller dependency graph: One less internal crate to maintain

Verification

  • All affected crates compile successfully
  • No remaining references to rolldown_rstr in the codebase
  • Build completes without errors (verified with cargo build)

This change maintains all existing functionality while simplifying the codebase architecture.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@netlify
Copy link

netlify bot commented Jul 31, 2025

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 35e4d0d
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/688c6a1c8f1a7700088fc171

@Boshen
Copy link
Member

Boshen commented Aug 1, 2025

@copilot please continue

Copy link
Contributor Author

Copilot AI commented Aug 1, 2025

@copilot please continue

Continuing with the migration! I've now completed the replacement of ToRstr trait usage with direct CompactStr construction throughout the codebase and removed rolldown_rstr dependencies from Cargo.toml files. Next step is to delete the rolldown_rstr crate directory and test the changes.

Copilot AI changed the title [WIP] Remove the crate @rolldown/rolldown/files/crates/rolldown_rstr, use CompactStr in callsites directly. Remove rolldown_rstr crate and use CompactStr directly Aug 1, 2025
@Boshen Boshen changed the title Remove rolldown_rstr crate and use CompactStr directly refactor(rolldown): Remove rolldown_rstr crate and use CompactStr directly Aug 1, 2025
@Boshen Boshen marked this pull request as ready for review August 1, 2025 06:44
@Boshen
Copy link
Member

Boshen commented Aug 1, 2025

@copilot rebase and apply the same changes

Copy link
Contributor Author

Copilot AI commented Aug 1, 2025

@copilot rebase and apply the same changes

Successfully rebased all migration changes onto the latest main branch (commit 7795064). The rebase resolved a merge conflict in ecma_view.rs and all changes have been preserved. The code compiles successfully after the rebase. Commit hash: a1fe78e

@Boshen Boshen force-pushed the copilot/fix-2a8a15b2-cceb-49e8-a57b-0708c5db31dd branch 2 times, most recently from b2c8e3b to 2cbd923 Compare August 1, 2025 07:17
Replace Rstr with CompactStr in rolldown_common type definitions

Co-authored-by: Boshen <[email protected]>

Changes before error encountered

Co-authored-by: Boshen <[email protected]>

Complete migration from rolldown_rstr to CompactStr

Co-authored-by: Boshen <[email protected]>
@Boshen Boshen force-pushed the copilot/fix-2a8a15b2-cceb-49e8-a57b-0708c5db31dd branch from 2cbd923 to 35e4d0d Compare August 1, 2025 07:17
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2025

Benchmarks Rust

  • target: main(7795064)
  • pr: copilot/fix-2a8a15b2-cceb-49e8-a57b-0708c5db31dd(35e4d0d)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.01     77.9±3.19ms        ? ?/sec    1.00     76.9±1.88ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     87.2±1.83ms        ? ?/sec    1.01     88.0±2.64ms        ? ?/sec
bundle/bundle@rome_ts                                        1.01    115.7±3.96ms        ? ?/sec    1.00    114.8±2.31ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.01    137.3±1.82ms        ? ?/sec    1.00    136.2±1.83ms        ? ?/sec
bundle/bundle@threejs                                        1.01     42.1±2.76ms        ? ?/sec    1.00     41.7±2.54ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     50.9±0.81ms        ? ?/sec    1.00     50.9±0.99ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    442.6±5.74ms        ? ?/sec    1.00    444.3±5.92ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    524.4±4.42ms        ? ?/sec    1.01    529.1±8.49ms        ? ?/sec
scan/scan@rome_ts                                            1.01     90.6±2.40ms        ? ?/sec    1.00     89.3±2.05ms        ? ?/sec
scan/scan@threejs                                            1.00     30.7±0.87ms        ? ?/sec    1.01     31.0±0.97ms        ? ?/sec
scan/scan@threejs10x                                         1.00    323.3±3.80ms        ? ?/sec    1.02    328.2±3.92ms        ? ?/sec

@Boshen Boshen requested a review from hyf0 August 1, 2025 07:37
@Boshen Boshen merged commit ce3d0f4 into main Aug 1, 2025
24 checks passed
@Boshen Boshen deleted the copilot/fix-2a8a15b2-cceb-49e8-a57b-0708c5db31dd branch August 1, 2025 07:38
This was referenced Aug 4, 2025
shulaoda added a commit that referenced this pull request Aug 4, 2025
## [1.0.0-beta.31] - 2025-08-04

### 🚀 Features

- support function for asset_inline_limit (#5453) by @AliceLanniste
- hmr: generate initializer for `export * from ...` (#5577) by @hyf0
- hmr: remove meaningless stmt instead of replacing with empty stmt (#5570) by @hyf0
- option `watch.clearScreen` (#5518) by @situ2001
- rolldown: oxc v0.80.0 (#5538) by @Boshen
- plugin/generateBundle: add support for modifying chunk filename (#5564) by @shulaoda
- assetFileNames: support `name` and `originalFileName` (#5555) by @shulaoda
- add optional `onInvalidate` schema to validator (#5528) by @situ2001
- don't throw resolve errors for dynamic import inside try block (#5537) by @shulaoda
- rolldown: oxc v0.79.0 (#5512) by @Boshen
- rolldown_plugin_asset: align `render_chunk` logic (#5511) by @shulaoda
- rolldown_plugin_utls: finish `render_asset_url_in_js` (#5508) by @shulaoda
- rolldown_plugin_utils: align replacement logic for `__VITE_ASSET__` (#5507) by @shulaoda
- rolldown_plugin_utils: add partial `render_asset_url_in_js` support (#5504) by @shulaoda
- expose `sourcemapBaseUrl` from NormalizedOutputOptions (#5513) by @sapphi-red

### 🐛 Bug Fixes

- modify output filename correctly in `generateBundle` and `writeBundle` (#5580) by @shulaoda
- hmr: ensure correct runtime behavior for `export * from ...` (#5578) by @hyf0
- hmr: repeated imports should point to the same binding (#5571) by @hyf0
- hmr: handle `export * from ...` (#5566) by @hyf0
- rolldown_plugin_web_worker_post: handle `import.meta` correctly (#5565) by @shulaoda
- antd bundle output not correct (#5549) by @IWANABETHATGUY
- hmr: module namespace object should be included via treeshaking mechanism normally (#5543) by @hyf0
- plugin/vite-resolve: use `import` / `require` conditions for external modules resolution (#5540) by @sapphi-red
- rolldown_plugin_asset: InvalidAsset#is should match InvalidAsset::True (#5529) by @AliceLanniste
- __name lacks require('./chunk-containing-runtime') when format: 'cjs' + keepNames: true (#5526) by @IWANABETHATGUY
- plugin/vite-resolve: resolve externalized packages with `resolve.externalConditions` (#5491) by @sapphi-red
- optimization.inlineConst: true does not keep export names when output.minifyInternalExports: true is enabled (#5520) by @IWANABETHATGUY
- scanner: identify `module.exports.__esModule = true` (#5503) by @overlookmotel
- error instead of panic if `sourcemapBaseUrl` is an invalid URL (#5514) by @sapphi-red

### 🚜 Refactor

- hmr: use `__rolldown_exports__` as the name for esm module namespace object (#5579) by @hyf0
- tests: rename `_configName` to `_testName` in configuration files and update variant formatting in snapshots (#5569) by @hyf0
- rolldown: remove unnecessary comments (#5552) by @situ2001
- rolldown: Remove `rolldown_rstr` crate and use `CompactStr` directly (#5542) by @Copilot
- hide namespace statement index knowledge inside StmtInfos (#5545) by @sapphi-red
- use pascal case when naming bitflags field for `ImportRecordMeta` (#5539) by @IWANABETHATGUY
- use concat_string! to simplify string concatenation (#5527) by @IWANABETHATGUY
- use combinator function handle `sourcemap_base_url` parsing (#5517) by @IWANABETHATGUY
- rolldown_plugin_utils: improve `to_output_file_path_in_js` (#5505) by @shulaoda

### 📚 Documentation

- wording nit (#5521) by @nullxone

### 🧪 Testing

- rust: rename `configName` to `_configName` and add comments (#5568) by @hyf0
- rust: re-organize `rolldown_testing_config` (#5567) by @hyf0

### ⚙️ Miscellaneous Tasks

- deps: lock file maintenance npm packages (#5582) by @renovate[bot]
- deps: lock file maintenance (#5581) by @renovate[bot]
- deps: lock file maintenance rust crates (#5576) by @renovate[bot]
- deps: lock file maintenance npm packages (#5574) by @renovate[bot]
- deps: update github-actions (#5573) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to v0.15.1 (#5572) by @renovate[bot]
- deps: update dependency tsdown to v0.13.2 (#5562) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to ^0.15.0 (#5563) by @renovate[bot]
- ai: Add AGENTS.md and GitHub Copilot setup workflow (#5556) by @Copilot
- deps: update dependency tsdown to v0.13.1 (#5551) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to v0.14.3 (#5544) by @renovate[bot]
- ensure all diagnostics in self.errors have Severity::Error (#5494) by @sapphi-red
- vscode: remove `code-spell-checker` extension (#5530) by @shulaoda
- deps: update dependency rolldown-plugin-dts to v0.14.2 (#5522) by @renovate[bot]
- add link to the REPL in the reproduction field description (#5519) by @sapphi-red
- enable some rollup tests (#5515) by @sapphi-red

### ❤️ New Contributors

* @Copilot made their first contribution in [#5556](#5556)
* @nullxone made their first contribution in [#5521](#5521)

Co-authored-by: shulaoda <[email protected]>
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.

2 participants