diff --git a/crates/oxc_resolver/src/lib.rs b/crates/oxc_resolver/src/lib.rs index ee2596b36..0e85e83f4 100644 --- a/crates/oxc_resolver/src/lib.rs +++ b/crates/oxc_resolver/src/lib.rs @@ -78,6 +78,7 @@ impl ResolveContext { fully_specified: false, query: ctx.query.clone(), fragment: ctx.fragment.clone(), + resolving_alias: ctx.resolving_alias.clone(), depth: ctx.depth, })) } @@ -92,6 +93,10 @@ impl ResolveContext { self.borrow_mut().fragment = fragment.map(ToString::to_string); } + fn with_resolving_alias(&self, alias: String) { + self.borrow_mut().resolving_alias = Some(alias); + } + fn test_for_infinite_recursion(&self) -> Result<(), ResolveError> { self.borrow_mut().depth += 1; // 64 should be more than enough for detecting infinite recursion. @@ -107,6 +112,8 @@ struct ResolveContextImpl { fully_specified: bool, query: Option, fragment: Option, + /// The current resolving alias for bailing recursion alias. + resolving_alias: Option, /// For avoiding infinite recursion, which will cause stack overflow. depth: u8, } @@ -626,12 +633,13 @@ impl ResolverGeneric { package_json: &PackageJson, ctx: &ResolveContext, ) -> ResolveState { - if !self.options.alias_fields.iter().any(|field| field == "browser") { - return Ok(None); - } let Some(specifier) = package_json.resolve_browser_field(path, specifier)? else { return Ok(None); }; + if ctx.borrow().resolving_alias.as_ref().is_some_and(|s| s == specifier) { + return Ok(None); + } + ctx.with_resolving_alias(specifier.to_string()); let cached_path = self.cache.value(package_json.directory()); let ctx = ResolveContext::clone_from(ctx); self.require(&cached_path, specifier, &ctx).map(Some) diff --git a/crates/oxc_resolver/src/package_json.rs b/crates/oxc_resolver/src/package_json.rs index faa1314e0..0b6dc3ab0 100644 --- a/crates/oxc_resolver/src/package_json.rs +++ b/crates/oxc_resolver/src/package_json.rs @@ -49,9 +49,11 @@ pub struct PackageJson { pub imports: Box, /// The "browser" field is provided by a module author as a hint to javascript bundlers or component tools when packaging modules for client side use. + /// Multiple values are configured by [ResolveOptions::alias_fields]. /// /// - pub browser: Option, + #[serde(skip)] + pub browser_fields: Vec, } /// `matchObj` defined in `PACKAGE_IMPORTS_EXPORTS_RESOLVE` @@ -113,6 +115,8 @@ pub enum BrowserField { } impl PackageJson { + /// # Panics + /// # Errors pub fn parse( path: PathBuf, json: &str, @@ -120,14 +124,40 @@ impl PackageJson { ) -> Result { let mut package_json_value: serde_json::Value = serde_json::from_str(json.clone())?; - // Dynamically create `main_fields`. - let mut main_fields = vec![]; + let mut main_fields = Vec::with_capacity(options.main_fields.len()); + let mut browser_fields = Vec::with_capacity(options.alias_fields.len()); + if let Some(package_json_value) = package_json_value.as_object_mut() { + // 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)) = - package_json_value.remove(main_field_key) + package_json_value.get(main_field_key) { - main_fields.push(value); + main_fields.push(value.clone()); + } + } + // Dynamically create `browser_fields`. + let dir = path.parent().unwrap(); + for browser_field_key in &options.alias_fields { + if let Some(value) = package_json_value.remove(browser_field_key) { + let mut browser_field: BrowserField = serde_json::from_value(value)?; + // Normalize all relative paths to make browser_field a constant value lookup + if let BrowserField::Map(map) = &mut browser_field { + let relative_paths = map + .keys() + .filter(|path| path.starts_with(".")) + .cloned() + .collect::>(); + for relative_path in relative_paths { + if let Some(value) = map.remove(&relative_path) { + let normalized_path = dir.normalize_with(relative_path); + map.insert(normalized_path, value); + } + } + } + browser_fields.push(browser_field); } } } @@ -135,20 +165,7 @@ impl PackageJson { // TODO: can this clone be avoided? let mut package_json: Self = serde_json::from_str(json.clone())?; package_json.main_fields = main_fields; - - // Normalize all relative paths to make browser_field a constant value lookup - // TODO: fix BrowserField::String - if let Some(BrowserField::Map(map)) = &mut package_json.browser { - let relative_paths = - map.keys().filter(|path| path.starts_with(".")).cloned().collect::>(); - let dir = path.parent().unwrap(); - for relative_path in relative_paths { - if let Some(value) = map.remove(&relative_path) { - let normalized_path = dir.normalize_with(relative_path); - map.insert(normalized_path, value); - } - } - } + package_json.browser_fields = browser_fields; package_json.path = path; Ok(package_json) @@ -170,20 +187,21 @@ impl PackageJson { path: &Path, request: Option<&str>, ) -> Result, ResolveError> { - // TODO: return ResolveError if the provided `alias_fields` is not `browser` for future proof - match self.browser.as_ref() { - Some(BrowserField::Map(field_data)) => { - // look up by full path if request is empty - request - .map_or_else( - || field_data.get(path), - |request| field_data.get(Path::new(request)), - ) - .map_or_else(|| Ok(None), |value| Self::alias_value(path, value)) + let request = request.map_or(path, |r| Path::new(r)); + for browser in &self.browser_fields { + match browser { + BrowserField::Map(field_data) => { + // look up by full path if request is empty + if let Some(value) = field_data.get(request) { + return Self::alias_value(path, value); + } + } + BrowserField::String(value) => { + return Ok(Some(value.as_str())); + } } - // TODO: implement - _ => Ok(None), } + Ok(None) } fn alias_value<'a>( diff --git a/crates/oxc_resolver/src/tests/browser_field.rs b/crates/oxc_resolver/src/tests/browser_field.rs index 1becc2fca..a0af5ceb4 100644 --- a/crates/oxc_resolver/src/tests/browser_field.rs +++ b/crates/oxc_resolver/src/tests/browser_field.rs @@ -32,6 +32,8 @@ fn replace_file() { let resolver = Resolver::new(ResolveOptions { alias_fields: vec!["browser".into()], + // Not part of enhanced-resolve. Added to make sure no interaction between these two fields. + main_fields: vec!["browser".into()], ..ResolveOptions::default() }); @@ -50,6 +52,8 @@ fn replace_file() { // TODO: resolve `innerBrowser2` field in `browser-module/pakckage.json` // ("should resolve in nested property 2", f.clone(), "./lib/main2.js", f.join("lib/browser.js")), ("should check only alias field properties", f.clone(), "./toString", f.join("lib/toString.js")), + // not part of enhanced-resolve + ("recursion", f.clone(), "module-c", f.join("node_modules/module-c.js")), ]; for (comment, path, request, expected) in data { diff --git a/crates/oxc_resolver/tests/enhanced_resolve/test/fixtures/browser-module/package.json b/crates/oxc_resolver/tests/enhanced_resolve/test/fixtures/browser-module/package.json index 217e0a9b8..c5704ce28 100644 --- a/crates/oxc_resolver/tests/enhanced_resolve/test/fixtures/browser-module/package.json +++ b/crates/oxc_resolver/tests/enhanced_resolve/test/fixtures/browser-module/package.json @@ -4,6 +4,7 @@ "./lib/replaced.js": "./lib/browser", "module-a": "./browser/module-a.js", "module-b": "module-c", + "module-c": "module-c", "./toString": "./lib/toString.js", ".": false },