fix: alias value should try fragment as path (#172)

Author: ahabhgk

fixtures/enhanced_resolve/test/fixtures/#/a.js

@@ -15,7 +15,8 @@
     "./xyz.js": "./lib/xyz.js",
     "./lib/non-existent.js": "./lib/non-existent.js",
     ".": false,
-    "./number": 1
+    "./number": 1,
+    "./foo": "./lib/replaced.js?query"
   },
   "innerBrowser1": {
     "field": {

fixtures/enhanced_resolve/test/fixtures/browser-module/package.json

@@ -225,17 +225,8 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         ctx: &mut Ctx,
     ) -> Result<Resolution, ResolveError> {
         ctx.with_fully_specified(self.options.fully_specified);
-        let specifier = Specifier::parse(specifier).map_err(ResolveError::Specifier)?;
-        ctx.with_query_fragment(specifier.query, specifier.fragment);
         let cached_path = self.cache.value(path);
-        let cached_path = self.require(&cached_path, specifier.path(), ctx).or_else(|err| {
-            if err.is_ignore() {
-                return Err(err);
-            }
-            // enhanced-resolve: try fallback
-            self.load_alias(&cached_path, specifier.path(), &self.options.fallback, ctx)
-                .and_then(|value| value.ok_or(err))
-        })?;
+        let cached_path = self.require(&cached_path, specifier, ctx)?;
         let path = self.load_realpath(&cached_path)?;
         // enhanced-resolve: restrictions
         self.check_restrictions(&path)?;
@@ -266,11 +257,21 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
     ) -> Result<CachedPath, ResolveError> {
         ctx.test_for_infinite_recursion()?;
 
-        // enhanced-resolve: try fragment as path
-        if let Some(path) = self.try_fragment_as_path(cached_path, specifier, ctx) {
+        // enhanced-resolve: parse
+        let (parsed, try_fragment_as_path) = self.load_parse(cached_path, specifier, ctx)?;
+        if let Some(path) = try_fragment_as_path {
             return Ok(path);
         }
 
+        self.require_without_parse(cached_path, parsed.path(), ctx)
+    }
+
+    fn require_without_parse(
+        &self,
+        cached_path: &CachedPath,
+        specifier: &str,
+        ctx: &mut Ctx,
+    ) -> Result<CachedPath, ResolveError> {
         // tsconfig-paths
         if let Some(path) = self.load_tsconfig_paths(cached_path, specifier, &mut Ctx::default())? {
             return Ok(path);
@@ -281,7 +282,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
             return Ok(path);
         }
 
-        match Path::new(specifier).components().next() {
+        let result = match Path::new(specifier).components().next() {
             // 3. If X begins with './' or '/' or '../'
             Some(Component::RootDir | Component::Prefix(_)) => {
                 self.require_absolute(cached_path, specifier, ctx)
@@ -305,7 +306,16 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
                 // Set resolved the result of PACKAGE_RESOLVE(specifier, parentURL).
                 self.require_bare(cached_path, specifier, ctx)
             }
-        }
+        };
+
+        result.or_else(|err| {
+            if err.is_ignore() {
+                return Err(err);
+            }
+            // enhanced-resolve: try fallback
+            self.load_alias(cached_path, specifier, &self.options.fallback, ctx)
+                .and_then(|value| value.ok_or(err))
+        })
     }
 
     // PACKAGE_RESOLVE(packageSpecifier, parentURL)
@@ -409,29 +419,34 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         self.load_package_self_or_node_modules(cached_path, specifier, ctx)
     }
 
-    /// Try fragment as part of the path
+    /// enhanced-resolve: ParsePlugin.
     ///
     /// It's allowed to escape # as \0# to avoid parsing it as fragment.
     /// enhanced-resolve will try to resolve requests containing `#` as path and as fragment,
     /// so it will automatically figure out if `./some#thing` means `.../some.js#thing` or `.../some#thing.js`.
     /// When a # is resolved as path it will be escaped in the result. Here: `.../some\0#thing.js`.
     ///
     /// <https://github.com/webpack/enhanced-resolve#escaping>
-    fn try_fragment_as_path(
+    fn load_parse<'s>(
         &self,
         cached_path: &CachedPath,
-        specifier: &str,
+        specifier: &'s str,
         ctx: &mut Ctx,
-    ) -> Option<CachedPath> {
+    ) -> Result<(Specifier<'s>, Option<CachedPath>), ResolveError> {
+        let parsed = Specifier::parse(specifier).map_err(ResolveError::Specifier)?;
+        ctx.with_query_fragment(parsed.query, parsed.fragment);
+
+        // There is an edge-case where a request with # can be a path or a fragment -> try both
         if ctx.fragment.is_some() && ctx.query.is_none() {
+            let specifier = parsed.path();
             let fragment = ctx.fragment.take().unwrap();
             let path = format!("{specifier}{fragment}");
-            if let Ok(path) = self.require(cached_path, &path, ctx) {
-                return Some(path);
+            if let Ok(path) = self.require_without_parse(cached_path, &path, ctx) {
+                return Ok((parsed, Some(path)));
             }
             ctx.fragment.replace(fragment);
         }
-        None
+        Ok((parsed, None))
     }
 
     fn load_package_self_or_node_modules(
@@ -850,12 +865,10 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
             }
             return Err(ResolveError::Recursion);
         }
-        let specifier = Specifier::parse(new_specifier).map_err(ResolveError::Specifier)?;
-        ctx.with_query_fragment(specifier.query, specifier.fragment);
-        ctx.with_resolving_alias(specifier.path().to_string());
+        ctx.with_resolving_alias(new_specifier.to_string());
         ctx.with_fully_specified(false);
         let cached_path = self.cache.value(package_json.directory());
-        self.require(&cached_path, specifier.path(), ctx).map(Some)
+        self.require(&cached_path, new_specifier, ctx).map(Some)
     }
 
     /// enhanced-resolve: AliasPlugin for [ResolveOptions::alias] and [ResolveOptions::fallback].
@@ -886,23 +899,16 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
             for r in specifiers {
                 match r {
                     AliasValue::Path(alias_value) => {
-                        let new_specifier =
-                            Specifier::parse(alias_value).map_err(ResolveError::Specifier)?;
-                        // Resolve path without query and fragment
-                        let old_query = ctx.query.clone();
-                        let old_fragment = ctx.fragment.clone();
-                        ctx.with_query_fragment(new_specifier.query, new_specifier.fragment);
                         if let Some(path) = self.load_alias_value(
                             cached_path,
                             alias_key,
-                            new_specifier.path(), // pass in parsed alias value
+                            alias_value,
                             specifier,
                             ctx,
                             &mut should_stop,
                         )? {
                             return Ok(Some(path));
                         }
-                        ctx.with_query_fragment(old_query.as_deref(), old_fragment.as_deref());
                     }
                     AliasValue::Ignore => {
                         let path = cached_path.path().normalize_with(alias_key);
@@ -1175,10 +1181,8 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
                         }
                     }
                     let subpath = format!(".{subpath}");
-                    let specifier = Specifier::parse(&subpath).map_err(ResolveError::Specifier)?;
                     ctx.with_fully_specified(false);
-                    ctx.with_query_fragment(specifier.query, specifier.fragment);
-                    return self.require(&cached_path, specifier.path(), ctx).map(Some);
+                    return self.require(&cached_path, &subpath, ctx).map(Some);
                 }
             }
         }

src/lib.rs

@@ -264,3 +264,17 @@ fn alias_fragment() {
         assert_eq!(resolved_path, Ok(expected), "{comment} {request}");
     }
 }
+
+#[test]
+fn alias_try_fragment_as_path() {
+    let f = super::fixture();
+    let resolver = Resolver::new(ResolveOptions {
+        alias: vec![(
+            "#".to_string(),
+            vec![AliasValue::Path(f.join("#").to_string_lossy().to_string())],
+        )],
+        ..ResolveOptions::default()
+    });
+    let resolution = resolver.resolve(&f, "#/a").map(|r| r.full_path());
+    assert_eq!(resolution, Ok(f.join("#").join("a.js")));
+}

src/tests/alias.rs

@@ -174,3 +174,16 @@ fn recursive() {
         assert_eq!(resolved_path, Err(ResolveError::Recursion), "{comment} {path:?} {request}");
     }
 }
+
+#[test]
+fn with_query() {
+    let f = super::fixture().join("browser-module");
+
+    let resolver = Resolver::new(ResolveOptions {
+        alias_fields: vec![vec!["browser".into()]],
+        ..ResolveOptions::default()
+    });
+
+    let resolved_path = resolver.resolve(&f, "./foo").map(|r| r.full_path());
+    assert_eq!(resolved_path, Ok(f.join("lib").join("browser.js?query")));
+}