fix #826: sometimes ignore path query part

CHANGELOG.md

@@ -10,6 +10,23 @@
 
     However, this warning is not relevant when accessing a property off of the `require` object such as `require.cache` because a property access does not result in capturing the value of `require`. Now a warning is no longer generated for `require.someProperty` when the output format is `cjs`. This allows for the use of features such as `require.cache` and `require.extensions`. This fix was contributed by [@huonw](https://github.com/huonw).
 
+* Support ignored URL parameters at the end of import paths ([#826](https://github.com/evanw/esbuild/issues/826))
+
+    If path resolution fails, ebuild will now try again with the URL query and/or fragment removed. This helps handle ancient CSS code like this that contains hacks for Internet Explorer:
+
+    ```css
+    @font-face {
+      src:
+        url("./themes/default/assets/fonts/icons.eot?#iefix") format('embedded-opentype'),
+        url("./themes/default/assets/fonts/icons.woff2") format('woff2'),
+        url("./themes/default/assets/fonts/icons.woff") format('woff'),
+        url("./themes/default/assets/fonts/icons.ttf") format('truetype'),
+        url("./themes/default/assets/fonts/icons.svg#icons") format('svg');
+    }
+    ```
+
+    Previously path resolution would fail because these files do not end with the `.eot?#iefix` or `.svg#icons` extensions. Now path resolution should succeed. The URL query and fragment are not unconditionally stripped because there is apparently [code in the wild that uses `#` as a directory name](https://github.com/medikoo/es5-ext/tree/3ddd2066b19e7c25a782869a304ae35d8188c8f1/string/%23). So esbuild will still try to resolve the full import path first and only try to reinterpret the path as a URL if that fails.
+
 ## 0.8.46
 
 * Fix minification of `.0` in CSS ([#804](https://github.com/evanw/esbuild/issues/804))

internal/bundler/bundler.go

@@ -304,7 +304,7 @@ func parseFile(args parseArgs) {
 		// but different contents from colliding
 		hash := hashForFileName([]byte(source.Contents))
 		additionalFileName := base + "." + hash + ext
-		publicPath := args.options.PublicPath + additionalFileName
+		publicPath := args.options.PublicPath + additionalFileName + source.KeyPath.IgnoredSuffix
 
 		// Determine the destination folder
 		targetFolder := args.options.AbsOutputDir

internal/bundler/bundler_default_test.go

@@ -2247,6 +2247,65 @@ func TestExternalModuleExclusionRelativePath(t *testing.T) {
 	})
 }
 
+// Webpack supports this case, so we do too. Some libraries apparently have
+// these paths: https://github.com/webpack/enhanced-resolve/issues/247
+func TestImportWithHashInPath(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				import foo from './file#foo.txt'
+				import bar from './file#bar.txt'
+				console.log(foo, bar)
+			`,
+			"/file#foo.txt": `foo`,
+			"/file#bar.txt": `bar`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:         config.ModeBundle,
+			AbsOutputDir: "/out",
+		},
+	})
+}
+
+func TestImportWithHashParameter(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				// Each of these should have a separate identity (i.e. end up in the output file twice)
+				import foo from './file.txt#foo'
+				import bar from './file.txt#bar'
+				console.log(foo, bar)
+			`,
+			"/file.txt": `This is some text`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:         config.ModeBundle,
+			AbsOutputDir: "/out",
+		},
+	})
+}
+
+func TestImportWithQueryParameter(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				// Each of these should have a separate identity (i.e. end up in the output file twice)
+				import foo from './file.txt?foo'
+				import bar from './file.txt?bar'
+				console.log(foo, bar)
+			`,
+			"/file.txt": `This is some text`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:         config.ModeBundle,
+			AbsOutputDir: "/out",
+		},
+	})
+}
+
 func TestAutoExternal(t *testing.T) {
 	default_suite.expectBundled(t, bundled{
 		files: map[string]string{

internal/bundler/bundler_loader_test.go

@@ -401,3 +401,48 @@ func TestLoaderJSONSharedWithMultipleEntriesIssue413(t *testing.T) {
 		},
 	})
 }
+
+func TestLoaderFileWithQueryParameter(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				// Each of these should have a separate identity (i.e. end up in the output file twice)
+				import foo from './file.txt?foo'
+				import bar from './file.txt?bar'
+				console.log(foo, bar)
+			`,
+			"/file.txt": `This is some text`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:         config.ModeBundle,
+			AbsOutputDir: "/out",
+			ExtensionToLoader: map[string]config.Loader{
+				".js":  config.LoaderJS,
+				".txt": config.LoaderFile,
+			},
+		},
+	})
+}
+
+func TestLoaderFromExtensionWithQueryParameter(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				import foo from './file.abc?query.xyz'
+				console.log(foo)
+			`,
+			"/file.abc": `This should not be base64 encoded`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:         config.ModeBundle,
+			AbsOutputDir: "/out",
+			ExtensionToLoader: map[string]config.Loader{
+				".js":  config.LoaderJS,
+				".abc": config.LoaderText,
+				".xyz": config.LoaderBase64,
+			},
+		},
+	})
+}

internal/bundler/snapshots/snapshots_default.txt

@@ -1004,6 +1004,42 @@ var Internal = () => /* @__PURE__ */ h(p, null, " Test 2 ");
 var App = () => /* @__PURE__ */ h(p, null, " ", /* @__PURE__ */ h(Internal, null), " T ");
 render(/* @__PURE__ */ h(App, null), document.getElementById("app"));
 
+================================================================================
+TestImportWithHashInPath
+---------- /out/entry.js ----------
+// file#foo.txt
+var file_foo_default = "foo";
+
+// file#bar.txt
+var file_bar_default = "bar";
+
+// entry.js
+console.log(file_foo_default, file_bar_default);
+
+================================================================================
+TestImportWithHashParameter
+---------- /out/entry.js ----------
+// file.txt#foo
+var file_default = "This is some text";
+
+// file.txt#bar
+var file_default2 = "This is some text";
+
+// entry.js
+console.log(file_default, file_default2);
+
+================================================================================
+TestImportWithQueryParameter
+---------- /out/entry.js ----------
+// file.txt?foo
+var file_default = "This is some text";
+
+// file.txt?bar
+var file_default2 = "This is some text";
+
+// entry.js
+console.log(file_default, file_default2);
+
 ================================================================================
 TestInject
 ---------- /out.js ----------
@@ -1123,6 +1159,29 @@ var clsExprKeep = /* @__PURE__ */ __name(class {
 }, "keep");
 new clsExprKeep();
 
+================================================================================
+TestLoaderFileWithQueryParameter
+---------- /out/file.JAWLBT6L.txt ----------
+This is some text
+---------- /out/entry.js ----------
+// file.txt?foo
+var file_default = "file.JAWLBT6L.txt?foo";
+
+// file.txt?bar
+var file_default2 = "file.JAWLBT6L.txt?bar";
+
+// entry.js
+console.log(file_default, file_default2);
+
+================================================================================
+TestLoaderFromExtensionWithQueryParameter
+---------- /out/entry.js ----------
+// file.abc?query.xyz
+var file_default = "This should not be base64 encoded";
+
+// entry.js
+console.log(file_default);
+
 ================================================================================
 TestManyEntryPoints
 ---------- /out/e00.js ----------

internal/logger/logger.go

@@ -129,7 +129,14 @@ func (a SortableMsgs) Less(i int, j int) bool {
 type Path struct {
 	Text      string
 	Namespace string
-	Flags     PathFlags
+
+	// This feature was added to support ancient CSS libraries that append things
+	// like "?#iefix" and "#icons" to some of their import paths as a hack for IE6.
+	// The intent is for these suffix parts to be ignored but passed through to
+	// the output. This is supported by other bundlers, so we also support this.
+	IgnoredSuffix string
+
+	Flags PathFlags
 }
 
 type PathFlags uint8
@@ -146,7 +153,8 @@ func (p Path) IsDisabled() bool {
 func (a Path) ComesBeforeInSortedOrder(b Path) bool {
 	return a.Namespace > b.Namespace ||
 		(a.Namespace == b.Namespace && (a.Text < b.Text ||
-			(a.Text == b.Text && a.Flags < b.Flags)))
+			(a.Text == b.Text && (a.Flags < b.Flags ||
+				(a.Flags == b.Flags && a.IgnoredSuffix < b.IgnoredSuffix)))))
 }
 
 // This has a custom implementation instead of using "filepath.Dir/Base/Ext"

internal/resolver/resolver.go

@@ -226,11 +226,24 @@ func (r *resolver) Resolve(sourceDir string, importPath string, kind ast.ImportK
 
 	result := r.resolveWithoutSymlinks(sourceDir, importPath, kind)
 	if result == nil {
-		return nil
+		// If resolution failed, try again with the URL query and/or hash removed
+		suffix := strings.IndexAny(importPath, "?#")
+		if suffix < 1 {
+			return nil
+		}
+		result = r.resolveWithoutSymlinks(sourceDir, importPath[:suffix], kind)
+		if result == nil {
+			return nil
+		}
+		result.PathPair.Primary.IgnoredSuffix = importPath[suffix:]
+		if result.PathPair.HasSecondary() {
+			result.PathPair.Secondary.IgnoredSuffix = importPath[suffix:]
+		}
 	}
 
 	// If successful, resolve symlinks using the directory info cache
-	return r.finalizeResolve(*result)
+	r.finalizeResolve(result)
+	return result
 }
 
 func (r *resolver) isExternalPattern(path string) bool {
@@ -249,7 +262,9 @@ func (r *resolver) ResolveAbs(absPath string) *ResolveResult {
 	defer r.mutex.Unlock()
 
 	// Just decorate the absolute path with information from parent directories
-	return r.finalizeResolve(ResolveResult{PathPair: PathPair{Primary: logger.Path{Text: absPath, Namespace: "file"}}})
+	result := &ResolveResult{PathPair: PathPair{Primary: logger.Path{Text: absPath, Namespace: "file"}}}
+	r.finalizeResolve(result)
+	return result
 }
 
 func (r *resolver) ProbeResolvePackageAsRelative(sourceDir string, importPath string, kind ast.ImportKind) *ResolveResult {
@@ -259,7 +274,9 @@ func (r *resolver) ProbeResolvePackageAsRelative(sourceDir string, importPath st
 	defer r.mutex.Unlock()
 
 	if pair, ok := r.loadAsFileOrDirectory(absPath, kind); ok {
-		return r.finalizeResolve(ResolveResult{PathPair: pair})
+		result := &ResolveResult{PathPair: pair}
+		r.finalizeResolve(result)
+		return result
 	}
 	return nil
 }
@@ -283,7 +300,7 @@ func IsInsideNodeModules(fs fs.FS, path string) bool {
 	}
 }
 
-func (r *resolver) finalizeResolve(result ResolveResult) *ResolveResult {
+func (r *resolver) finalizeResolve(result *ResolveResult) {
 	for _, path := range result.PathPair.iter() {
 		if path.Namespace == "file" {
 			if dirInfo := r.dirInfoCached(r.fs.Dir(path.Text)); dirInfo != nil {
@@ -346,8 +363,6 @@ func (r *resolver) finalizeResolve(result ResolveResult) *ResolveResult {
 			}
 		}
 	}
-
-	return &result
 }
 
 func (r *resolver) resolveWithoutSymlinks(sourceDir string, importPath string, kind ast.ImportKind) *ResolveResult {
@@ -527,7 +542,7 @@ func (r *resolver) PrettyPath(path logger.Path) string {
 		path.Text = "(disabled):" + path.Text
 	}
 
-	return path.Text
+	return path.Text + path.IgnoredSuffix
 }
 
 ////////////////////////////////////////////////////////////////////////////////