Skip to content

Commit

Permalink
fix: incorrect resolution when using shared resolvers with different …
Browse files Browse the repository at this point in the history
…`main_fields`
  • Loading branch information
Boshen committed Apr 22, 2024
1 parent db051d5 commit 0224a85
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 30 deletions.
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
cached_path.package_json(&self.cache.fs, &self.options, ctx)?
{
// b. If "main" is a falsy value, GOTO 2.
for main_field in &package_json.main_fields {
for main_field in package_json.main_fields(&self.options.main_fields) {
// c. let M = X + (json main field)
let main_field_path = cached_path.path().normalize_with(main_field);
// d. LOAD_AS_FILE(M)
Expand Down Expand Up @@ -1149,7 +1149,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
// 6. Otherwise, if packageSubpath is equal to ".", then
if subpath == "." {
// 1. If pjson.main is a string, then
for main_field in &package_json.main_fields {
for main_field in package_json.main_fields(&self.options.main_fields) {

Check warning on line 1152 in src/lib.rs

View check run for this annotation

Codecov / codecov/patch

src/lib.rs#L1152

Added line #L1152 was not covered by tests
// 1. Return the URL resolution of main in packageURL.
let path = cached_path.path().normalize_with(main_field);
let cached_path = self.cache.value(&path);
Expand Down
43 changes: 19 additions & 24 deletions src/package_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,14 @@ pub struct PackageJson {
/// Realpath to `package.json`. Contains the `package.json` filename.
pub realpath: PathBuf,

#[cfg(feature = "package_json_raw_json_api")]
pub(crate) raw_json: std::sync::Arc<serde_json::Value>,
raw_json: std::sync::Arc<serde_json::Value>,

/// The "name" field defines your package's name.
/// The "name" field can be used in addition to the "exports" field to self-reference a package using its name.
///
/// <https://nodejs.org/api/packages.html#name>
pub name: Option<String>,

/// The "main" field defines the entry point of a package when imported by name via a node_modules lookup. Its value is a path.
/// When a package has an "exports" field, this will take precedence over the "main" field when importing the package by name.
///
/// Values are dynamically added from [ResolveOptions::main_fields].
///
/// <https://nodejs.org/api/packages.html#main>
pub main_fields: Vec<String>,

/// The "exports" field allows defining the entry points of a package when imported by name loaded either via a node_modules lookup or a self-reference to its own name.
///
/// <https://nodejs.org/api/packages.html#exports>
Expand Down Expand Up @@ -64,7 +55,6 @@ impl PackageJson {
let mut raw_json: Value = serde_json::from_str(json)?;
let mut package_json = Self::default();

package_json.main_fields.reserve_exact(options.main_fields.len());
package_json.exports.reserve_exact(options.exports_fields.len());
package_json.browser_fields.reserve_exact(options.alias_fields.len());

Expand Down Expand Up @@ -92,15 +82,6 @@ impl PackageJson {
.transpose()?
.map(Box::new);

// Dynamically create `main_fields`.
for main_field_key in &options.main_fields {
// Using `get` + `clone` instead of remove here
// because `main_fields` may contain `browser`, which is also used in `browser_fields.
if let Some(serde_json::Value::String(value)) = json_object.get(main_field_key) {
package_json.main_fields.push(value.clone());
}
}

// Dynamically create `browser_fields`.
let dir = path.parent().unwrap();
for object_path in &options.alias_fields {
Expand Down Expand Up @@ -138,10 +119,7 @@ impl PackageJson {

package_json.path = path;
package_json.realpath = realpath;
#[cfg(feature = "package_json_raw_json_api")]
{
package_json.raw_json = std::sync::Arc::new(raw_json);
}
package_json.raw_json = std::sync::Arc::new(raw_json);
Ok(package_json)
}

Expand Down Expand Up @@ -189,6 +167,23 @@ impl PackageJson {
self.realpath.parent().unwrap()
}

/// The "main" field defines the entry point of a package when imported by name via a node_modules lookup. Its value is a path.
///
/// When a package has an "exports" field, this will take precedence over the "main" field when importing the package by name.
///
/// Values are dynamically retrieved from [ResolveOptions::main_fields].
///
/// <https://nodejs.org/api/packages.html#main>
pub(crate) fn main_fields<'a>(
&'a self,
main_fields: &'a [String],
) -> impl Iterator<Item = &'a str> + '_ {
main_fields
.iter()
.filter_map(|main_field| self.raw_json.get(main_field))
.filter_map(|value| value.as_str())
}

/// Resolve the request string for this package.json by looking at the `browser` field.
///
/// # Errors
Expand Down
8 changes: 4 additions & 4 deletions src/tests/main_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ use crate::{ResolveOptions, Resolver};
fn test() {
let f = super::fixture().join("restrictions");

let resolver = Resolver::new(ResolveOptions {
let resolver1 = Resolver::new(ResolveOptions {
main_fields: vec!["style".into()],
..ResolveOptions::default()
});

let resolution = resolver.resolve(&f, "pck2").map(|r| r.full_path());
let resolution = resolver1.resolve(&f, "pck2").map(|r| r.full_path());
assert_eq!(resolution, Ok(f.join("node_modules/pck2/index.css")));

let resolver = Resolver::new(ResolveOptions {
let resolver2 = resolver1.clone_with_options(ResolveOptions {
main_fields: vec!["module".into(), "main".into()],
..ResolveOptions::default()
});

let resolution = resolver.resolve(&f, "pck2").map(|r| r.full_path());
let resolution = resolver2.resolve(&f, "pck2").map(|r| r.full_path());
assert_eq!(resolution, Ok(f.join("node_modules/pck2/module.js")));
}

0 comments on commit 0224a85

Please sign in to comment.