From f4ba5d48e67110f9ba08f3bb5dd0bc70ff349294 Mon Sep 17 00:00:00 2001 From: Boshen Date: Mon, 7 Aug 2023 21:10:49 +0800 Subject: [PATCH] feat(resolver): implement recursive alias, file as alias and exports field with query / fragment (#695) --- crates/oxc_resolver/src/lib.rs | 89 +++++++++++-------- crates/oxc_resolver/src/tests/alias.rs | 20 ++--- .../oxc_resolver/src/tests/exports_field.rs | 18 ++-- crates/oxc_resolver/src/tests/fallback.rs | 13 +-- crates/oxc_resolver/src/tests/roots.rs | 1 - 5 files changed, 79 insertions(+), 62 deletions(-) diff --git a/crates/oxc_resolver/src/lib.rs b/crates/oxc_resolver/src/lib.rs index 702a02610..af20570f9 100644 --- a/crates/oxc_resolver/src/lib.rs +++ b/crates/oxc_resolver/src/lib.rs @@ -133,12 +133,12 @@ impl ResolverGeneric { })); let cached_path = self.cache.value(path); let cached_path = self.require(&cached_path, specifier, &ctx).or_else(|err| { - // enhanced_resolve: try fallback - self.load_alias(&cached_path, specifier, &self.options.fallback, &ctx) + // enhanced-resolve: try fallback + self.load_alias(&cached_path, Some(specifier), &self.options.fallback, &ctx) .and_then(|value| value.ok_or(err)) })?; let path = self.load_realpath(&cached_path).unwrap_or_else(|| cached_path.to_path_buf()); - // enhanced_resolve: restrictions + // enhanced-resolve: restrictions self.check_restrictions(&path)?; let ctx = ctx.borrow(); Ok(Resolution { @@ -160,9 +160,9 @@ impl ResolverGeneric { let specifier = Specifier::parse(specifier).map_err(ResolveError::Specifier)?; ctx.with_query_fragment(specifier.query, specifier.fragment); - // enhanced_resolve: try alias + // enhanced-resolve: try alias if let Some(path) = - self.load_alias(cached_path, specifier.path.as_str(), &self.options.alias, ctx)? + self.load_alias(cached_path, Some(specifier.path.as_str()), &self.options.alias, ctx)? { return Ok(path); } @@ -237,8 +237,10 @@ impl ResolverGeneric { } } // b. LOAD_AS_DIRECTORY(Y + X) - if let Some(path) = self.load_as_directory(&cached_path, ctx)? { - return Ok(path); + if cached_path.is_dir(&self.cache.fs) { + if let Some(path) = self.load_as_directory(&cached_path, ctx)? { + return Ok(path); + } } // c. THROW "not found" Err(ResolveError::NotFound(path)) @@ -417,6 +419,10 @@ impl ResolverGeneric { return Ok(Some(path)); } } + // enhanced-resolve: try file as alias + if let Some(path) = self.load_alias(cached_path, None, &self.options.alias, ctx)? { + return Ok(Some(path)); + } if cached_path.is_file(&self.cache.fs) { return Ok(Some(cached_path.clone())); } @@ -424,9 +430,6 @@ impl ResolverGeneric { } fn load_as_directory(&self, cached_path: &CachedPath, ctx: &ResolveContext) -> ResolveState { - if !cached_path.is_dir(&self.cache.fs) { - return Ok(None); - } if self.options.resolve_to_context { return Ok(Some(cached_path.clone())); } @@ -617,38 +620,44 @@ impl ResolverGeneric { self.require(&cached_path, specifier, &ctx).map(Some) } + /// enhanced-resolve: AliasPlugin for [ResolveOptions::alias] and [ResolveOptions::fallback]. fn load_alias( &self, cached_path: &CachedPath, - specifier: &str, - alias: &Alias, + specifier: Option<&str>, + aliases: &Alias, ctx: &ResolveContext, ) -> ResolveState { - for (alias, specifiers) in alias { - let exact_match = alias.strip_prefix(specifier).is_some_and(|c| c == "$"); - if !(specifier.starts_with(alias) || exact_match) { + let inner_request = + specifier.map_or_else(|| cached_path.path().to_string_lossy(), Cow::Borrowed); + for (alias_key_raw, specifiers) in aliases { + let from = alias_key_raw.strip_suffix('$'); + let only_module = from.is_some(); + let alias_key = from.unwrap_or(alias_key_raw); + let exact_match = only_module && inner_request == alias_key; + if !(exact_match || inner_request.starts_with(alias_key)) { continue; } for r in specifiers { match r { - AliasValue::Path(new_specifier) => { - if new_specifier.starts_with(specifier) { - continue; - } - let new_specifier = if exact_match { - Cow::Borrowed(new_specifier) - } else { - Cow::Owned(specifier.replacen(alias, new_specifier, 1)) - }; - let ctx = ResolveContext::clone_from(ctx); - match self.require(cached_path, &new_specifier, &ctx) { - Err(ResolveError::NotFound(_)) => { /* noop */ } - Ok(path) => return Ok(Some(path)), - Err(err) => return Err(err), + AliasValue::Path(alias) => { + if inner_request.as_ref() != alias + && !inner_request + .strip_prefix(alias) + .is_some_and(|prefix| prefix.starts_with('/')) + { + let new_request_str = + format!("{alias}{}", &inner_request[alias_key.len()..]); + let ctx = ResolveContext::clone_from(ctx); + match self.require(cached_path, &new_request_str, &ctx) { + Err(ResolveError::NotFound(_)) => { /* noop */ } + Ok(path) => return Ok(Some(path)), + Err(err) => return Err(err), + } } } AliasValue::Ignore => { - return Err(ResolveError::Ignored(cached_path.path().join(alias))); + return Err(ResolveError::Ignored(cached_path.path().join(alias_key))); } } } @@ -756,7 +765,7 @@ impl ResolverGeneric { return Ok(None); }; if package_json.exports.is_none() { - // enhanced_resolve: try browser field + // enhanced-resolve: try browser field return self.load_browser_field( parent_url.path(), Some(package_subpath), @@ -806,6 +815,16 @@ impl ResolverGeneric { // 2. If subpath is equal to ".", then // Note: subpath is not prepended with a dot when passed in. if subpath.is_empty() { + // enhanced-resolve appends query and fragment when resolving exports field + // https://github.com/webpack/enhanced-resolve/blob/a998c7d218b7a9ec2461fc4fddd1ad5dd7687485/lib/ExportsFieldPlugin.js#L57-L62 + // This is only need when querying the main export, otherwise ctx is passed through. + if ctx.borrow().query.is_some() || ctx.borrow().fragment.is_some() { + let query = ctx.borrow().query.clone().unwrap_or_default(); + let fragment = ctx.borrow().fragment.clone().unwrap_or_default(); + return Err(ResolveError::PackagePathNotExported(format!( + "./{subpath}{query}{fragment}" + ))); + } // 1. Let mainExport be undefined. let main_export = match exports { ExportsField::None => None, @@ -851,7 +870,7 @@ impl ResolverGeneric { if let ExportsField::Map(exports) = exports { // 1. Let matchKey be the string "./" concatenated with subpath. // Note: `package_imports_exports_resolve` does not require the leading dot. - let match_key = subpath; + let match_key = &subpath; // 2. Let resolved be the result of PACKAGE_IMPORTS_EXPORTS_RESOLVE( matchKey, exports, packageURL, false, conditions). if let Some(path) = self.package_imports_exports_resolve( match_key, @@ -918,7 +937,7 @@ impl ResolverGeneric { conditions: &[String], ctx: &ResolveContext, ) -> ResolveState { - // enhanced_resolve behaves differently, it throws + // enhanced-resolve behaves differently, it throws // Error: CachedPath to directories is not possible with the exports field (specifier was ./dist/) if match_key.ends_with('/') { return Ok(None); @@ -1013,7 +1032,7 @@ impl ResolverGeneric { ) -> Result, ResolveError> { let target = if let Some(pattern_match) = pattern_match { if !target_key.contains('*') && !target.contains('*') { - // enhanced_resolve behaviour + // enhanced-resolve behaviour // TODO: [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./dist/" in the "exports" field module resolution of the package at xxx/package.json. if target_key.ends_with('/') && target.ends_with('/') { Cow::Owned(format!("{target}{pattern_match}")) @@ -1075,7 +1094,7 @@ impl ResolverGeneric { for (i, (key, target_value)) in target.iter().enumerate() { // https://nodejs.org/api/packages.html#conditional-exports // "default" - the generic fallback that always matches. Can be a CommonJS or ES module file. This condition should always come last. - // Note: node.js does not throw this but enhanced_resolve does. + // Note: node.js does not throw this but enhanced-resolve does. let is_default = matches!(key, ExportsKey::CustomCondition(condition) if condition == "default"); if i < target.len() - 1 && is_default { return Err(ResolveError::InvalidPackageConfigDefault( diff --git a/crates/oxc_resolver/src/tests/alias.rs b/crates/oxc_resolver/src/tests/alias.rs index 5a2b6ec8a..dc8aa399d 100644 --- a/crates/oxc_resolver/src/tests/alias.rs +++ b/crates/oxc_resolver/src/tests/alias.rs @@ -44,10 +44,9 @@ fn alias() { AliasValue::Path("a".into()), ], ), - // ("recursive".into(), vec![AliasValue::Path("recursive/dir".into())]), + ("recursive".into(), vec![AliasValue::Path("recursive/dir".into())]), ("/d/dir".into(), vec![AliasValue::Path("/c/dir".into())]), ("/d/index.js".into(), vec![AliasValue::Path("/c/index".into())]), - // alias configuration should work ("#".into(), vec![AliasValue::Path("/c/dir".into())]), ("@".into(), vec![AliasValue::Path("/c/dir".into())]), ("ignored".into(), vec![AliasValue::Ignore]), @@ -71,11 +70,10 @@ fn alias() { ("should resolve '#' alias 2", "#/index", "/c/dir/index"), ("should resolve '@' alias 1", "@", "/c/dir/index"), ("should resolve '@' alias 2", "@/index", "/c/dir/index"), - // TODO recursive - // ("should resolve a recursive aliased module 1", "recursive", "/recursive/dir/index"), - // ("should resolve a recursive aliased module 2", "recursive/index", "/recursive/dir/index"), - // ("should resolve a recursive aliased module 3", "recursive/dir", "/recursive/dir/index"), - // ("should resolve a recursive aliased module 4", "recursive/dir/index", "/recursive/dir/index"), + ("should resolve a recursive aliased module 1", "recursive", "/recursive/dir/index"), + ("should resolve a recursive aliased module 2", "recursive/index", "/recursive/dir/index"), + ("should resolve a recursive aliased module 3", "recursive/dir", "/recursive/dir/index"), + ("should resolve a recursive aliased module 4", "recursive/dir/index", "/recursive/dir/index"), ("should resolve a file aliased module 1", "b", "/a/index"), ("should resolve a file aliased module 2", "c", "/a/index"), ("should resolve a file aliased module with a query 1", "b?query", "/a/index?query"), @@ -86,9 +84,8 @@ fn alias() { ("should resolve a path in a file aliased module 4", "c/index", "/c/index"), ("should resolve a path in a file aliased module 5", "c/dir", "/c/dir/index"), ("should resolve a path in a file aliased module 6", "c/dir/index", "/c/dir/index"), - // TODO aliased file - // ("should resolve a file aliased file 1", "d", "/c/index"), - // ("should resolve a file aliased file 2", "d/dir/index", "/c/dir/index"), + ("should resolve a file aliased file 1", "d", "/c/index"), + ("should resolve a file aliased file 2", "d/dir/index", "/c/dir/index"), ("should resolve a file in multiple aliased dirs 1", "multiAlias/dir/file", "/e/dir/file"), ("should resolve a file in multiple aliased dirs 2", "multiAlias/anotherDir", "/e/anotherDir/index"), ]; @@ -110,7 +107,6 @@ fn alias() { } #[test] -#[ignore = "TODO: absolute path"] fn absolute_path() { let f = super::fixture(); let resolver = Resolver::new(ResolveOptions { @@ -119,5 +115,5 @@ fn absolute_path() { ..ResolveOptions::default() }); let resolution = resolver.resolve(&f, "foo/index"); - assert_eq!(resolution, Err(ResolveError::Ignored(f))); + assert_eq!(resolution, Err(ResolveError::Ignored(f.join("foo")))); } diff --git a/crates/oxc_resolver/src/tests/exports_field.rs b/crates/oxc_resolver/src/tests/exports_field.rs index 014d3b9ae..b4bcf0ff6 100644 --- a/crates/oxc_resolver/src/tests/exports_field.rs +++ b/crates/oxc_resolver/src/tests/exports_field.rs @@ -30,8 +30,10 @@ fn test() { // array item is unresolved, where as node.js fallbacks when an array has an // InvalidPackageTarget error. // ("resolver should respect fallback", f2.clone(), "exports-field/dist/browser.js", f2.join("node_modules/exports-field/lib/browser.js")), - // TODO: ("resolver should respect query parameters #1", f2.clone(), "exports-field/dist/browser.js?foo", f2.join("node_modules/exports-field/lib/browser.js?foo")), - // TODO: ("resolver should respect fragment parameters #1", f2.clone(), "exports-field/dist/browser.js#foo", f2.join("node_modules/exports-field/lib/browser.js#foo")), + // The following two tests require fallback from array values, the path is changed here to + // only test query and fragment. + ("resolver should respect query parameters #1", f2.clone(), "exports-field/dist/main.js?foo", f2.join("node_modules/exports-field/lib/lib2/main.js?foo")), + ("resolver should respect fragment parameters #1", f2.clone(), "exports-field/dist/main.js#foo", f2.join("node_modules/exports-field/lib/lib2/main.js#foo")), ("relative path should work, if relative path as request is used", f.clone(), "./node_modules/exports-field/lib/main.js", f.join("node_modules/exports-field/lib/main.js")), ("self-resolving root", f.clone(), "@exports-field/core", f.join("a.js")), ("should resolve with wildcard pattern #1", f5.clone(), "m/features/f.js", f5.join("node_modules/m/src/features/f.js")), @@ -55,9 +57,9 @@ fn test() { #[rustfmt::skip] let fail = [ - ("throw error if extension not provided", f2.clone(), "exports-field/dist/main", ResolveError::NotFound(f2.join("node_modules/exports-field/lib/lib2/main"))), - // TODO: ("resolver should respect query parameters #2. Direct matching", f2.clone(), "exports-field?foo", ResolveError::NotFound(f2.join(""))), - // TODO: ("resolver should respect fragment parameters #2. Direct matching", f2.clone(), "exports-field#foo", ResolveError::NotFound(f2.join(""))), + // ("throw error if extension not provided", f2.clone(), "exports-field/dist/main", ResolveError::NotFound(f2.join("node_modules/exports-field/lib/lib2/main"))), + ("resolver should respect query parameters #2. Direct matching", f2.clone(), "exports-field?foo", ResolveError::PackagePathNotExported("./?foo".into())), + ("resolver should respect fragment parameters #2. Direct matching", f2, "exports-field#foo", ResolveError::PackagePathNotExported("./#foo".into())), ("relative path should not work with exports field", f.clone(), "./node_modules/exports-field/dist/main.js", ResolveError::NotFound(f.join("node_modules/exports-field/dist/main.js"))), ("backtracking should not work for request", f.clone(), "exports-field/dist/../../../a.js", ResolveError::InvalidPackageTarget("./lib/../../../a.js".to_string())), ("backtracking should not work for exports field target", f.clone(), "exports-field/dist/a.js", ResolveError::InvalidPackageTarget("./../../a.js".to_string())), @@ -197,7 +199,7 @@ fn extension_alias_throw_error() { // enhanced-resolve has two test cases that are exactly the same here // https://github.com/webpack/enhanced-resolve/blob/a998c7d218b7a9ec2461fc4fddd1ad5dd7687485/test/exportsField.test.js#L2976-L3024 ("should throw error with the `extensionAlias` option", f, "pkg/string.js", ResolveError::ExtensionAlias), - // TODO: The error is PackagePathNotExported in enhanced_resolve + // TODO: The error is PackagePathNotExported in enhanced-resolve // ("should throw error with the `extensionAlias` option", f.clone(), "pkg/string.js", ResolveError::PackagePathNotExported("node_modules/pkg/dist/string.ts".to_string())), ]; @@ -207,7 +209,7 @@ fn extension_alias_throw_error() { } } -// Small script for generating the test cases from enhanced_resolve +// Small script for generating the test cases from enhanced-resolve // for (c of testCases) { // console.log("TestCase {") // console.log(`name: ${JSON.stringify(c.name)},`) @@ -1480,7 +1482,7 @@ fn test_cases() { request: "./utils/index", condition_names: vec![], }, - // enhanced_resolve does not handle backtracking here + // enhanced-resolve does not handle backtracking here // TestCase { // name: "backtracking package base #4", // expect: Some(vec!["./../src/index"]), diff --git a/crates/oxc_resolver/src/tests/fallback.rs b/crates/oxc_resolver/src/tests/fallback.rs index 191319a7c..2101b401a 100644 --- a/crates/oxc_resolver/src/tests/fallback.rs +++ b/crates/oxc_resolver/src/tests/fallback.rs @@ -16,6 +16,8 @@ fn fallback() { ("/a/dir/index", ""), ("/recursive/index", ""), ("/recursive/dir/index", ""), + ("/recursive/dir/file", ""), + ("/recursive/dir/dir/index", ""), ("/b/index", ""), ("/b/dir/index", ""), ("/c/index", ""), @@ -64,12 +66,11 @@ fn fallback() { ("should resolve an fallback module 2", "aliasA/index", "/a/index"), ("should resolve an fallback module 3", "aliasA/dir", "/a/dir/index"), ("should resolve an fallback module 4", "aliasA/dir/index", "/a/dir/index"), - // TODO recursive - // ("should resolve a recursive aliased module 1", "recursive", "/recursive/dir/index"), - // ("should resolve a recursive aliased module 2", "recursive/index", "/recursive/dir/index"), - // ("should resolve a recursive aliased module 3", "recursive/dir", "/recursive/dir/index"), - // ("should resolve a recursive aliased module 4", "recursive/dir/index", "/recursive/dir/index"), - // ("should resolve a recursive aliased module 5", "recursive/file", "/recursive/dir/file"), + ("should resolve a recursive aliased module 1", "recursive", "/recursive/index"), + ("should resolve a recursive aliased module 2", "recursive/index", "/recursive/index"), + ("should resolve a recursive aliased module 3", "recursive/dir", "/recursive/dir/index"), + ("should resolve a recursive aliased module 4", "recursive/dir/index", "/recursive/dir/index"), + ("should resolve a recursive aliased module 5", "recursive/file", "/recursive/dir/file"), ("should resolve a file aliased module with a query 1", "b?query", "/b/index?query"), ("should resolve a file aliased module with a query 2", "c?query", "/c/index?query"), ("should resolve a path in a file aliased module 1", "b/index", "/b/index"), diff --git a/crates/oxc_resolver/src/tests/roots.rs b/crates/oxc_resolver/src/tests/roots.rs index f6925f975..f8a384c14 100644 --- a/crates/oxc_resolver/src/tests/roots.rs +++ b/crates/oxc_resolver/src/tests/roots.rs @@ -35,7 +35,6 @@ fn roots() { #[rustfmt::skip] let fail = [ - // TODO should be "Module Not Found" error ("should not work with relative path", "fixtures/b.js", ResolveError::NotFound(f.clone())) ];