Skip to content

Comments

refactor: introduce TsconfigResolver trait to share resolver instance#7581

Open
shulaoda wants to merge 1 commit intomainfrom
12-19-refactor_introduce_tsconfigresolver_trait_to_share_resolver_instance
Open

refactor: introduce TsconfigResolver trait to share resolver instance#7581
shulaoda wants to merge 1 commit intomainfrom
12-19-refactor_introduce_tsconfigresolver_trait_to_share_resolver_instance

Conversation

@shulaoda
Copy link
Member

@shulaoda shulaoda commented Dec 18, 2025 •
edited
Loading

Shares tsconfig cache between module resolution and transform, avoiding duplicate resolver creation.

@shulaoda shulaoda changed the title refactor: introduce TsconfigResolver trait to share resolver instance refactor: introduce TsconfigResolver trait to share resolver instance Dec 18, 2025
@shulaoda Graphite App
Copy link
Member Author

shulaoda commented Dec 18, 2025 •
edited
Loading

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.

@shulaoda shulaoda mentioned this pull request Dec 18, 2025
Merged
@shulaoda shulaoda marked this pull request as ready for review December 18, 2025 20:57
@shulaoda shulaoda requested a review from sapphi-red December 18, 2025 20:57
@shulaoda shulaoda force-pushed the 12-19-refactor_introduce_tsconfigresolver_trait_to_share_resolver_instance branch from a37ca1a to 8e2bd78 Compare December 18, 2025 21:08
@shulaoda shulaoda force-pushed the 12-17-fix_tsconfig_enable_project_references_support_in_manual_mode branch from 3ea8135 to 50d8c62 Compare December 18, 2025 21:08
sapphi-red
sapphi-red reviewed Dec 19, 2025
View reviewed changes
crates/rolldown_common/src/inner_bundler_options/types/transform_options.rs
/// Cache key: tsconfig path, or empty PathBuf for files without tsconfig
pub cache: FxDashMap<PathBuf, Arc<OxcTransformOptions>>,
resolver: Arc<Resolver>,
pub resolver: Arc<dyn TsconfigResolver>,
Copy link
Member

@sapphi-red sapphi-red Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's better to use a generics here to reduce the indirection caused by dyn.

@hyf0
Copy link
Member

hyf0 commented Dec 19, 2025

  • If tsconfig resolution is a buitin feature, would it be more straightforward to just add code in resolver directly?
  • What's the goal for passing a specific Arc<dyn TsconfigResolver>?

@hyf0 hyf0 assigned sapphi-red and unassigned shulaoda Dec 19, 2025
@shulaoda shulaoda force-pushed the 12-17-fix_tsconfig_enable_project_references_support_in_manual_mode branch from e6f9af5 to 9eaf5e5 Compare December 23, 2025 07:09
@shulaoda shulaoda force-pushed the 12-19-refactor_introduce_tsconfigresolver_trait_to_share_resolver_instance branch from 8e2bd78 to 7dda978 Compare December 23, 2025 07:12
@shulaoda shulaoda force-pushed the 12-17-fix_tsconfig_enable_project_references_support_in_manual_mode branch from 9eaf5e5 to 190f923 Compare December 23, 2025 07:12
@shulaoda shulaoda force-pushed the 12-19-refactor_introduce_tsconfigresolver_trait_to_share_resolver_instance branch from 7dda978 to 49b3183 Compare December 23, 2025 07:15
@shulaoda shulaoda force-pushed the 12-17-fix_tsconfig_enable_project_references_support_in_manual_mode branch from 190f923 to 42459f2 Compare December 23, 2025 07:15
@github-actions
Copy link
Contributor

github-actions bot commented Dec 23, 2025

Benchmarks Rust

  • target: 12-17-fix_tsconfig_enable_project_references_support_in_manual_mode(42459f2)
  • pr: 12-19-refactor_introduce_tsconfigresolver_trait_to_share_resolver_instance(49b3183)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     62.7±1.85ms        ? ?/sec    1.00     62.5±2.08ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     68.8±1.67ms        ? ?/sec    1.00     68.6±1.41ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    105.6±1.67ms        ? ?/sec    1.00    105.3±1.49ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.01    118.4±2.64ms        ? ?/sec    1.00    117.6±1.45ms        ? ?/sec
bundle/bundle@threejs                                        1.03     39.7±2.49ms        ? ?/sec    1.00     38.6±0.96ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     42.5±0.60ms        ? ?/sec    1.00     42.4±0.71ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    387.5±4.11ms        ? ?/sec    1.00    389.3±5.25ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    447.2±3.63ms        ? ?/sec    1.00    446.4±3.38ms        ? ?/sec
scan/scan@rome_ts                                            1.01     83.6±2.32ms        ? ?/sec    1.00     83.0±1.67ms        ? ?/sec
scan/scan@threejs                                            1.00     28.3±0.44ms        ? ?/sec    1.00     28.3±0.34ms        ? ?/sec
scan/scan@threejs10x                                         1.02    297.1±3.95ms        ? ?/sec    1.00    292.5±5.34ms        ? ?/sec

@graphite-app graphite-app bot changed the base branch from 12-17-fix_tsconfig_enable_project_references_support_in_manual_mode to graphite-base/7581 December 23, 2025 07:32
@shulaoda shulaoda force-pushed the 12-19-refactor_introduce_tsconfigresolver_trait_to_share_resolver_instance branch from 49b3183 to 641324e Compare December 23, 2025 07:43
@graphite-app graphite-app bot changed the base branch from graphite-base/7581 to main December 23, 2025 07:43
@graphite-app
Copy link
Contributor

graphite-app bot commented Dec 23, 2025 •
edited
Loading

Merge activity

  • Dec 23, 7:43 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.
  • Dec 30, 7:06 AM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
  • Dec 30, 7:06 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 mentioned this pull request Feb 22, 2026
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sapphi-red sapphi-red sapphi-red left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@sapphi-red sapphi-red

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shulaoda @hyf0 @sapphi-red