From d74da2f3bf61bdfbddabf748873f170f81502bb3 Mon Sep 17 00:00:00 2001 From: Boshen Date: Sat, 2 Sep 2023 14:45:10 +0800 Subject: [PATCH] fix(resolver): fix cases with conflicting node_modules path (#835) --- crates/oxc_resolver/src/lib.rs | 24 ++++++++---------------- crates/oxc_resolver/src/tests/resolve.rs | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/crates/oxc_resolver/src/lib.rs b/crates/oxc_resolver/src/lib.rs index 7e1ec0353..ad8f524ef 100644 --- a/crates/oxc_resolver/src/lib.rs +++ b/crates/oxc_resolver/src/lib.rs @@ -579,21 +579,20 @@ impl ResolverGeneric { ctx: &ResolveContext, ) -> ResolveState { // 1. let DIRS = NODE_MODULES_PATHS(START) - // Use a buffer to reduce total memory allocation. - let mut node_module_path = cached_path.path().to_path_buf(); // 2. for each DIR in DIRS: - loop { + for path in cached_path.path().ancestors() { for module_name in &self.options.modules { // node_module_path = foo/node_modules/ - node_module_path.push(module_name); - let cached_path = self.cache.value(&node_module_path); + let cached_path = if path.ends_with(module_name) { + self.cache.value(path) + } else { + self.cache.value(&path.join(module_name)) + }; // Skip if foo/node_modules does not exist if !cached_path.is_dir(&self.cache.fs) { - node_module_path.pop(); continue; } - // Optimize node_modules lookup by inspecting whether the package exists // From LOAD_PACKAGE_EXPORTS(X, DIR) // 1. Try to interpret X as a combination of NAME and SUBPATH where the name @@ -601,7 +600,7 @@ impl ResolverGeneric { let (package_name, subpath) = Self::parse_package_specifier(specifier); if !package_name.is_empty() { - let package_path = node_module_path.join(package_name); + let package_path = cached_path.path().join(package_name); let cached_path = self.cache.value(&package_path); // Try foo/node_modules/package_name if cached_path.is_dir(&self.cache.fs) { @@ -612,7 +611,6 @@ impl ResolverGeneric { } else { // foo/node_modules/package_name is not a directory, so useless to check inside it if !subpath.is_empty() { - node_module_path.pop(); continue; } } @@ -621,17 +619,11 @@ impl ResolverGeneric { // Try as file or directory for all other cases // b. LOAD_AS_FILE(DIR/X) // c. LOAD_AS_DIRECTORY(DIR/X) - let node_module_file = node_module_path.normalize_with(specifier); + let node_module_file = cached_path.path().normalize_with(specifier); let cached_path = self.cache.value(&node_module_file); if let Some(path) = self.load_as_file_or_directory(&cached_path, specifier, ctx)? { return Ok(Some(path)); } - - node_module_path.pop(); - } - - if !node_module_path.pop() { - break; } } Ok(None) diff --git a/crates/oxc_resolver/src/tests/resolve.rs b/crates/oxc_resolver/src/tests/resolve.rs index 9f85debf4..819c70500 100644 --- a/crates/oxc_resolver/src/tests/resolve.rs +++ b/crates/oxc_resolver/src/tests/resolve.rs @@ -51,8 +51,18 @@ fn resolve() { } } -// #[test] -// fn issue238_resolve() {} +#[test] +fn issue238_resolve() { + let f = super::fixture().join("issue-238"); + let resolver = Resolver::new(ResolveOptions { + extensions: vec![".js".into(), ".jsx".into(), ".ts".into(), ".tsx".into()], + modules: vec!["src/a".into(), "src/b".into(), "src/common".into(), "node_modules".into()], + ..ResolveOptions::default() + }); + let resolved_path = + resolver.resolve(f.join("src/common"), "config/myObjectFile").map(|r| r.full_path()); + assert_eq!(resolved_path, Ok(f.join("src/common/config/myObjectFile.js")),); +} #[test] fn prefer_relative() {