Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: incorrect resolution when using shared resolvers with different main_fields #134

Merged
merged 1 commit into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 @@
// 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")));
}