Skip to content

Commit

Permalink
refactor(core): remove unnecessary enum for resolver (#7631)
Browse files Browse the repository at this point in the history
  • Loading branch information
hardfist authored Aug 20, 2024
1 parent 9efcf53 commit dd6e27a
Showing 1 changed file with 44 additions and 56 deletions.
100 changes: 44 additions & 56 deletions crates/rspack_core/src/resolver/resolver_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ impl<'a> ResolveInnerOptions<'a> {
///
/// Internal caches are shared.
#[derive(Debug)]
pub enum Resolver {
RspackResolver(rspack_resolver::Resolver),
pub struct Resolver {
resolver: rspack_resolver::Resolver,
}

impl Resolver {
Expand All @@ -89,14 +89,12 @@ impl Resolver {
fn new_rspack_resolver(options: Resolve) -> Self {
let options = to_rspack_resolver_options(options, false, DependencyCategory::Unknown);
let resolver = rspack_resolver::Resolver::new(options);
Self::RspackResolver(resolver)
Self { resolver }
}

/// Clear cache for all resolver instances
pub fn clear_cache(&self) {
match self {
Self::RspackResolver(resolver) => resolver.clear_cache(),
}
self.resolver.clear_cache();
}

/// Create a new resolver by cloning its internal cache.
Expand All @@ -105,41 +103,34 @@ impl Resolver {
options: Resolve,
options_with_dependency_type: &ResolveOptionsWithDependencyType,
) -> Self {
match self {
Self::RspackResolver(resolver) => {
let options = to_rspack_resolver_options(
options,
options_with_dependency_type.resolve_to_context,
options_with_dependency_type.dependency_category,
);
let resolver = resolver.clone_with_options(options);
Self::RspackResolver(resolver)
}
}
let resolver = &self.resolver;
let options = to_rspack_resolver_options(
options,
options_with_dependency_type.resolve_to_context,
options_with_dependency_type.dependency_category,
);
let resolver = resolver.clone_with_options(options);
Self { resolver }
}

/// Return the options from the resolver
pub fn options(&self) -> ResolveInnerOptions<'_> {
match self {
Self::RspackResolver(resolver) => ResolveInnerOptions::RspackResolver(resolver.options()),
}
ResolveInnerOptions::RspackResolver(self.resolver.options())
}

/// Resolve a specifier to a given path.
pub fn resolve(&self, path: &Path, request: &str) -> Result<ResolveResult, ResolveInnerError> {
match self {
Self::RspackResolver(resolver) => match resolver.resolve(path, request) {
Ok(r) => Ok(ResolveResult::Resource(Resource {
path: r.path().to_path_buf().assert_utf8(),
query: r.query().unwrap_or_default().to_string(),
fragment: r.fragment().unwrap_or_default().to_string(),
description_data: r
.package_json()
.map(|d| DescriptionData::new(d.directory().to_path_buf(), Arc::clone(d.raw_json()))),
})),
Err(rspack_resolver::ResolveError::Ignored(_)) => Ok(ResolveResult::Ignored),
Err(error) => Err(ResolveInnerError::RspackResolver(error)),
},
match self.resolver.resolve(path, request) {
Ok(r) => Ok(ResolveResult::Resource(Resource {
path: r.path().to_path_buf().assert_utf8(),
query: r.query().unwrap_or_default().to_string(),
fragment: r.fragment().unwrap_or_default().to_string(),
description_data: r
.package_json()
.map(|d| DescriptionData::new(d.directory().to_path_buf(), Arc::clone(d.raw_json()))),
})),
Err(rspack_resolver::ResolveError::Ignored(_)) => Ok(ResolveResult::Ignored),
Err(error) => Err(ResolveInnerError::RspackResolver(error)),
}
}

Expand All @@ -150,29 +141,26 @@ impl Resolver {
request: &str,
resolve_context: &mut ResolveContext,
) -> Result<ResolveResult, ResolveInnerError> {
match self {
Self::RspackResolver(resolver) => {
let mut context = Default::default();
let result = resolver.resolve_with_context(path, request, &mut context);
resolve_context
.file_dependencies
.extend(context.file_dependencies);
resolve_context
.missing_dependencies
.extend(context.missing_dependencies);
match result {
Ok(r) => Ok(ResolveResult::Resource(Resource {
path: r.path().to_path_buf().assert_utf8(),
query: r.query().unwrap_or_default().to_string(),
fragment: r.fragment().unwrap_or_default().to_string(),
description_data: r
.package_json()
.map(|d| DescriptionData::new(d.directory().to_path_buf(), Arc::clone(d.raw_json()))),
})),
Err(rspack_resolver::ResolveError::Ignored(_)) => Ok(ResolveResult::Ignored),
Err(error) => Err(ResolveInnerError::RspackResolver(error)),
}
}
let resolver = &self.resolver;
let mut context = Default::default();
let result = resolver.resolve_with_context(path, request, &mut context);
resolve_context
.file_dependencies
.extend(context.file_dependencies);
resolve_context
.missing_dependencies
.extend(context.missing_dependencies);
match result {
Ok(r) => Ok(ResolveResult::Resource(Resource {
path: r.path().to_path_buf().assert_utf8(),
query: r.query().unwrap_or_default().to_string(),
fragment: r.fragment().unwrap_or_default().to_string(),
description_data: r
.package_json()
.map(|d| DescriptionData::new(d.directory().to_path_buf(), Arc::clone(d.raw_json()))),
})),
Err(rspack_resolver::ResolveError::Ignored(_)) => Ok(ResolveResult::Ignored),
Err(error) => Err(ResolveInnerError::RspackResolver(error)),
}
}
}
Expand Down

4 comments on commit dd6e27a

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2024-08-20 46c0de9) Current Change
10000_development-mode + exec 2.36 s ± 21 ms 2.37 s ± 28 ms +0.14 %
10000_development-mode_hmr + exec 703 ms ± 6.6 ms 710 ms ± 6 ms +0.90 %
10000_production-mode + exec 3.02 s ± 31 ms 3.05 s ± 112 ms +1.07 %
arco-pro_development-mode + exec 1.9 s ± 92 ms 1.9 s ± 67 ms +0.06 %
arco-pro_development-mode_hmr + exec 437 ms ± 1.6 ms 437 ms ± 3.2 ms -0.10 %
arco-pro_production-mode + exec 3.47 s ± 77 ms 3.42 s ± 73 ms -1.37 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.54 s ± 75 ms 3.58 s ± 107 ms +0.91 %
threejs_development-mode_10x + exec 1.71 s ± 10 ms 1.69 s ± 15 ms -1.48 %
threejs_development-mode_10x_hmr + exec 816 ms ± 6.3 ms 798 ms ± 14 ms -2.15 %
threejs_production-mode_10x + exec 5.55 s ± 44 ms 5.52 s ± 15 ms -0.49 %

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Ran ecosystem CI: Open

suite result
modernjs ✅ success
_selftest ✅ success
nx ❌ failure
rspress ✅ success
rslib ✅ success
rsbuild ❌ failure
examples ✅ success

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2024-08-21 dd6e27a) Current Change
10000_development-mode + exec 2.36 s ± 30 ms 2.36 s ± 23 ms +0.10 %
10000_development-mode_hmr + exec 712 ms ± 10 ms 708 ms ± 24 ms -0.62 %
10000_production-mode + exec 3.02 s ± 24 ms 3.03 s ± 51 ms +0.27 %
arco-pro_development-mode + exec 1.91 s ± 60 ms 1.9 s ± 66 ms -0.48 %
arco-pro_development-mode_hmr + exec 438 ms ± 3.3 ms 437 ms ± 1.3 ms -0.15 %
arco-pro_production-mode + exec 3.47 s ± 86 ms 3.46 s ± 86 ms -0.33 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.52 s ± 83 ms 3.54 s ± 93 ms +0.79 %
threejs_development-mode_10x + exec 1.69 s ± 19 ms 1.69 s ± 19 ms +0.15 %
threejs_development-mode_10x_hmr + exec 797 ms ± 6 ms 798 ms ± 4.9 ms +0.14 %
threejs_production-mode_10x + exec 5.48 s ± 33 ms 5.54 s ± 19 ms +1.17 %

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Ran ecosystem CI: Open

suite result
modernjs ✅ success
_selftest ✅ success
nx ❌ failure
rspress ✅ success
rslib ✅ success
rsbuild ❌ failure
examples ✅ success

Please sign in to comment.