Skip to content

Commit e16ca9d

Browse files
committed
refactor: remove AsRef<Path> from functions
While examining rolldown binary size, I noticed 6 copies of `pnp::resolve_to_unqualified_via_manifes`. ``` Lines Copies Function name ----- ------ ------------- 17436 (0.4%, 8.2%) 6 (0.0%, 7.0%) pnp::resolve_to_unqualified_via_manifes ``` Remove all `AsRef<Path>` to avoid accidental generic instantiation and code bloat, these functions do not need to be generic anyway.
1 parent 21b071a commit e16ca9d

File tree

2 files changed

+36
-40
lines changed

2 files changed

+36
-40
lines changed

src/lib.rs

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,13 @@ pub fn find_closest_pnp_manifest_path(path: &Path) -> Option<PathBuf> {
9494
None
9595
}
9696

97-
pub fn load_pnp_manifest<P: AsRef<Path>>(p: P) -> Result<Manifest, Error> {
98-
let manifest_content = std::fs::read_to_string(p.as_ref()).map_err(|err| {
97+
pub fn load_pnp_manifest(p: &Path) -> Result<Manifest, Error> {
98+
let manifest_content = std::fs::read_to_string(p).map_err(|err| {
9999
Error::FailedManifestHydration(Box::new(FailedManifestHydration {
100100
message: format!(
101101
"We failed to read the content of the manifest.\n\nOriginal error: {err}"
102102
),
103-
manifest_path: p.as_ref().to_path_buf(),
103+
manifest_path: p.to_path_buf(),
104104
}))
105105
})?;
106106

@@ -117,7 +117,7 @@ pub fn load_pnp_manifest<P: AsRef<Path>>(p: P) -> Result<Manifest, Error> {
117117
.unwrap_or_default()
118118
.ok_or_else(|| Error::FailedManifestHydration(Box::new(FailedManifestHydration {
119119
message: String::from("We failed to locate the PnP data payload inside its manifest file. Did you manually edit the file?"),
120-
manifest_path: p.as_ref().to_path_buf(),
120+
manifest_path: p.to_path_buf(),
121121
})))?;
122122

123123
let iter = manifest_content.chars().skip(manifest_match.end());
@@ -142,18 +142,18 @@ pub fn load_pnp_manifest<P: AsRef<Path>>(p: P) -> Result<Manifest, Error> {
142142
let mut manifest: Manifest = serde_json::from_str(&json_string.to_owned())
143143
.map_err(|err| Error::FailedManifestHydration(Box::new(FailedManifestHydration {
144144
message: format!("We failed to parse the PnP data payload as proper JSON; Did you manually edit the file?\n\nOriginal error: {err}"),
145-
manifest_path: p.as_ref().to_path_buf(),
145+
manifest_path: p.to_path_buf(),
146146
})))?;
147147

148-
init_pnp_manifest(&mut manifest, p.as_ref());
148+
init_pnp_manifest(&mut manifest, p);
149149

150150
Ok(manifest)
151151
}
152152

153-
pub fn init_pnp_manifest<P: AsRef<Path>>(manifest: &mut Manifest, p: P) {
154-
manifest.manifest_path = p.as_ref().to_path_buf();
153+
pub fn init_pnp_manifest(manifest: &mut Manifest, p: &Path) {
154+
manifest.manifest_path = p.to_path_buf();
155155

156-
manifest.manifest_dir = p.as_ref().parent().expect("Should have a parent directory").to_owned();
156+
manifest.manifest_dir = p.parent().expect("Should have a parent directory").to_owned();
157157

158158
for (name, ranges) in manifest.package_registry_data.iter_mut() {
159159
for (reference, info) in ranges.iter_mut() {
@@ -187,17 +187,14 @@ pub fn init_pnp_manifest<P: AsRef<Path>>(manifest: &mut Manifest, p: P) {
187187
}
188188

189189
pub fn find_pnp_manifest(parent: &Path) -> Result<Option<Manifest>, Error> {
190-
find_closest_pnp_manifest_path(parent).map_or(Ok(None), |p| Ok(Some(load_pnp_manifest(p)?)))
190+
find_closest_pnp_manifest_path(parent).map_or(Ok(None), |p| Ok(Some(load_pnp_manifest(&p)?)))
191191
}
192192

193193
pub fn is_dependency_tree_root<'a>(manifest: &'a Manifest, locator: &'a PackageLocator) -> bool {
194194
manifest.dependency_tree_roots.contains(locator)
195195
}
196196

197-
pub fn find_locator<'a, P: AsRef<Path>>(
198-
manifest: &'a Manifest,
199-
path: &P,
200-
) -> Option<&'a PackageLocator> {
197+
pub fn find_locator<'a>(manifest: &'a Manifest, path: &Path) -> Option<&'a PackageLocator> {
201198
let rel_path = pathdiff::diff_paths(path, &manifest.manifest_dir)
202199
.expect("Assertion failed: Provided path should be absolute");
203200

@@ -240,14 +237,14 @@ pub fn find_broken_peer_dependencies(
240237
[].to_vec()
241238
}
242239

243-
pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
240+
pub fn resolve_to_unqualified_via_manifest(
244241
manifest: &Manifest,
245242
specifier: &str,
246-
parent: P,
243+
parent: &Path,
247244
) -> Result<Resolution, Error> {
248245
let (ident, module_path) = parse_bare_identifier(specifier)?;
249246

250-
if let Some(parent_locator) = find_locator(manifest, &parent) {
247+
if let Some(parent_locator) = find_locator(manifest, parent) {
251248
let parent_pkg = get_package(manifest, parent_locator)?;
252249

253250
let mut reference_or_alias: Option<PackageDependency> = None;
@@ -281,7 +278,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
281278
} else {
282279
String::from("")
283280
},
284-
issuer_path = parent.as_ref().to_string_lossy(),
281+
issuer_path = parent.to_string_lossy(),
285282
)
286283
} else {
287284
format!(
@@ -293,7 +290,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
293290
} else {
294291
String::from("")
295292
},
296-
issuer_path = parent.as_ref().to_string_lossy(),
293+
issuer_path = parent.to_string_lossy(),
297294
)
298295
}
299296
} else if is_dependency_tree_root(manifest, parent_locator) {
@@ -305,7 +302,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
305302
} else {
306303
String::from("")
307304
},
308-
issuer_path = parent.as_ref().to_string_lossy(),
305+
issuer_path = parent.to_string_lossy(),
309306
)
310307
} else {
311308
format!(
@@ -318,7 +315,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
318315
} else {
319316
String::from("")
320317
},
321-
issuer_path = parent.as_ref().to_string_lossy(),
318+
issuer_path = parent.to_string_lossy(),
322319
)
323320
};
324321

@@ -327,7 +324,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
327324
request: specifier.to_string(),
328325
dependency_name: ident,
329326
issuer_locator: parent_locator.clone(),
330-
issuer_path: parent.as_ref().to_path_buf(),
327+
issuer_path: parent.to_path_buf(),
331328
})));
332329
}
333330

@@ -354,7 +351,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
354351
} else {
355352
String::from("")
356353
},
357-
issuer_path = parent.as_ref().to_string_lossy(),
354+
issuer_path = parent.to_string_lossy(),
358355
)
359356
} else if !broken_ancestors.is_empty()
360357
&& broken_ancestors.iter().all(|locator| is_dependency_tree_root(manifest, locator))
@@ -369,7 +366,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
369366
} else {
370367
String::from("")
371368
},
372-
issuer_path = parent.as_ref().to_string_lossy(),
369+
issuer_path = parent.to_string_lossy(),
373370
)
374371
} else {
375372
format!(
@@ -382,7 +379,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
382379
} else {
383380
String::from("")
384381
},
385-
issuer_path = parent.as_ref().to_string_lossy(),
382+
issuer_path = parent.to_string_lossy(),
386383
)
387384
};
388385

@@ -391,7 +388,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
391388
request: specifier.to_string(),
392389
dependency_name: ident,
393390
issuer_locator: parent_locator.clone(),
394-
issuer_path: parent.as_ref().to_path_buf(),
391+
issuer_path: parent.to_path_buf(),
395392
broken_ancestors: [].to_vec(),
396393
})))
397394
}
@@ -400,13 +397,13 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
400397
}
401398
}
402399

403-
pub fn resolve_to_unqualified<P: AsRef<Path>>(
400+
pub fn resolve_to_unqualified(
404401
specifier: &str,
405-
parent: P,
402+
parent: &Path,
406403
config: &ResolutionConfig,
407404
) -> Result<Resolution, Error> {
408-
if let Some(manifest) = (config.host.find_pnp_manifest)(parent.as_ref())? {
409-
resolve_to_unqualified_via_manifest(&manifest, specifier, &parent)
405+
if let Some(manifest) = (config.host.find_pnp_manifest)(parent)? {
406+
resolve_to_unqualified_via_manifest(&manifest, specifier, parent)
410407
} else {
411408
Ok(Resolution::Skipped)
412409
}

src/lib_tests.rs

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::path::Path;
2+
13
use serde::Deserialize;
24

35
use crate::{Manifest, Resolution};
@@ -28,18 +30,15 @@ mod tests {
2830

2931
#[test]
3032
fn example() {
31-
let manifest = load_pnp_manifest("data/pnp-yarn-v3.cjs").unwrap();
33+
let manifest = load_pnp_manifest(Path::new("data/pnp-yarn-v3.cjs")).unwrap();
3234

3335
let host =
3436
ResolutionHost { find_pnp_manifest: Box::new(move |_| Ok(Some(manifest.clone()))) };
3537

3638
let config = ResolutionConfig { host };
3739

38-
let resolution = resolve_to_unqualified(
39-
"lodash/cloneDeep",
40-
std::path::PathBuf::from("/path/to/file"),
41-
&config,
42-
);
40+
let resolution =
41+
resolve_to_unqualified("lodash/cloneDeep", Path::new("/path/to/file"), &config);
4342

4443
match resolution {
4544
Ok(Resolution::Resolved(_path, _subpath)) => {
@@ -60,10 +59,10 @@ mod tests {
6059

6160
#[test]
6261
fn test_load_pnp_manifest() {
63-
load_pnp_manifest("data/pnp-yarn-v3.cjs")
62+
load_pnp_manifest(Path::new("data/pnp-yarn-v3.cjs"))
6463
.expect("Assertion failed: Expected to load the .pnp.cjs file generated by Yarn 3");
6564

66-
load_pnp_manifest("data/pnp-yarn-v4.cjs")
65+
load_pnp_manifest(Path::new("data/pnp-yarn-v4.cjs"))
6766
.expect("Assertion failed: Expected to load the .pnp.cjs file generated by Yarn 4");
6867
}
6968

@@ -81,7 +80,7 @@ mod tests {
8180

8281
for test_suite in test_suites.iter_mut() {
8382
let manifest = &mut test_suite.manifest;
84-
init_pnp_manifest(manifest, PathBuf::from("/path/to/project/.pnp.cjs"));
83+
init_pnp_manifest(manifest, Path::new("/path/to/project/.pnp.cjs"));
8584

8685
for test in test_suite.tests.iter() {
8786
let specifier = &test.imported;
@@ -119,7 +118,7 @@ mod tests {
119118
std::env::current_dir().unwrap().join("./data/edge_case_manifest_state.json");
120119
let manifest_content = fs::read_to_string(&manifest_json_path).unwrap();
121120
let mut manifest = serde_json::from_str::<Manifest>(&manifest_content).unwrap();
122-
init_pnp_manifest(&mut manifest, manifest_json_path);
121+
init_pnp_manifest(&mut manifest, &manifest_json_path);
123122
manifest
124123
};
125124

0 commit comments

Comments
 (0)