diff --git a/CHANGELOG.md b/CHANGELOG.md index a8e605baa30..4f5c7316291 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Fix the `alias` feature to always prefer the longest match ([#2963](https://github.com/evanw/esbuild/issues/2963)) + + It's possible to configure conflicting aliases such as `--alias:a=b` and `--alias:a/c=d`, which is ambiguous for the import path `a/c/x` (since it could map to either `b/c/x` or `d/x`). Previously esbuild would pick the first matching `alias`, which would non-deterministically pick between one of the possible matches. This release fixes esbuild to always deterministically pick the longest possible match. + * Minify calls to some global primitive constructors ([#2962](https://github.com/evanw/esbuild/issues/2962)) With this release, esbuild's minifier now replaces calls to `Boolean`/`Number`/`String`/`BigInt` with equivalent shorter code when relevant: diff --git a/internal/bundler_tests/bundler_default_test.go b/internal/bundler_tests/bundler_default_test.go index de58f6698ad..82b2a44e6e5 100644 --- a/internal/bundler_tests/bundler_default_test.go +++ b/internal/bundler_tests/bundler_default_test.go @@ -7551,6 +7551,36 @@ func TestPackageAlias(t *testing.T) { }) } +func TestPackageAliasMatchLongest(t *testing.T) { + default_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + import "pkg" + import "pkg/foo" + import "pkg/foo/bar" + import "pkg/foo/bar/baz" + import "pkg/bar/baz" + import "pkg/baz" + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + PackageAliases: map[string]string{ + "pkg": "alias/pkg", + "pkg/foo": "alias/pkg_foo", + "pkg/foo/bar": "alias/pkg_foo_bar", + }, + ExternalSettings: config.ExternalSettings{ + PreResolve: config.ExternalMatchers{ + Patterns: []config.WildcardPattern{{Prefix: "alias/"}}, + }, + }, + }, + }) +} + func TestErrorsForAssertTypeJSON(t *testing.T) { default_suite.expectBundled(t, bundled{ files: map[string]string{ diff --git a/internal/bundler_tests/snapshots/snapshots_default.txt b/internal/bundler_tests/snapshots/snapshots_default.txt index 43acf9900d8..2d315a0584a 100644 --- a/internal/bundler_tests/snapshots/snapshots_default.txt +++ b/internal/bundler_tests/snapshots/snapshots_default.txt @@ -4697,6 +4697,17 @@ console.log(10); // node_modules/@scope/prefix-foo/index.js console.log(11); +================================================================================ +TestPackageAliasMatchLongest +---------- /out.js ---------- +// entry.js +import "alias/pkg"; +import "alias/pkg_foo"; +import "alias/pkg_foo_bar"; +import "alias/pkg_foo_bar/baz"; +import "alias/pkg/bar/baz"; +import "alias/pkg/baz"; + ================================================================================ TestQuotedProperty ---------- /out/entry.js ---------- diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 8cdda2955c4..c228a219201 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -293,42 +293,49 @@ func (res *Resolver) Resolve(sourceDir string, importPath string, kind ast.Impor if r.debugLogs != nil { r.debugLogs.addNote("Checking for package alias matches") } - foundMatch := false + longestKey := "" + longestValue := "" + for key, value := range r.options.PackageAliases { - if strings.HasPrefix(importPath, key) && (len(importPath) == len(key) || importPath[len(key)] == '/') { - // Resolve the package using the current path instead of the original - // path. This is trying to resolve the substitute in the top-level - // package instead of the nested package, which lets the top-level - // package control the version of the substitution. It's also critical - // when using Yarn PnP because Yarn PnP doesn't allow nested packages - // to "reach outside" of their normal dependency lists. - sourceDir = r.fs.Cwd() - if tail := importPath[len(key):]; tail != "/" { - // Don't include the trailing characters if they are equal to a - // single slash. This comes up because you can abuse this quirk of - // node's path resolution to force node to load the package from the - // file system instead of as a built-in module. For example, "util" - // is node's built-in module while "util/" is one on the file system. - // Leaving the trailing slash in place causes problems for people: - // https://github.com/evanw/esbuild/issues/2730. It should be ok to - // always strip the trailing slash even when using the alias feature - // to swap one package for another (except when you swap a reference - // to one built-in node module with another but really why would you - // do that). - value += tail - } - debugMeta.ModifiedImportPath = value - if r.debugLogs != nil { - r.debugLogs.addNote(fmt.Sprintf(" Matched with alias from %q to %q", key, value)) - r.debugLogs.addNote(fmt.Sprintf(" Modified import path from %q to %q", importPath, debugMeta.ModifiedImportPath)) - r.debugLogs.addNote(fmt.Sprintf(" Changed resolve directory to %q", sourceDir)) - } - importPath = debugMeta.ModifiedImportPath - foundMatch = true - break + if len(key) > len(longestKey) && strings.HasPrefix(importPath, key) && (len(importPath) == len(key) || importPath[len(key)] == '/') { + longestKey = key + longestValue = value } } - if r.debugLogs != nil && !foundMatch { + + if longestKey != "" { + debugMeta.ModifiedImportPath = longestValue + if tail := importPath[len(longestKey):]; tail != "/" { + // Don't include the trailing characters if they are equal to a + // single slash. This comes up because you can abuse this quirk of + // node's path resolution to force node to load the package from the + // file system instead of as a built-in module. For example, "util" + // is node's built-in module while "util/" is one on the file system. + // Leaving the trailing slash in place causes problems for people: + // https://github.com/evanw/esbuild/issues/2730. It should be ok to + // always strip the trailing slash even when using the alias feature + // to swap one package for another (except when you swap a reference + // to one built-in node module with another but really why would you + // do that). + debugMeta.ModifiedImportPath += tail + } + if r.debugLogs != nil { + r.debugLogs.addNote(fmt.Sprintf(" Matched with alias from %q to %q", longestKey, longestValue)) + r.debugLogs.addNote(fmt.Sprintf(" Modified import path from %q to %q", importPath, debugMeta.ModifiedImportPath)) + } + importPath = debugMeta.ModifiedImportPath + + // Resolve the package using the current path instead of the original + // path. This is trying to resolve the substitute in the top-level + // package instead of the nested package, which lets the top-level + // package control the version of the substitution. It's also critical + // when using Yarn PnP because Yarn PnP doesn't allow nested packages + // to "reach outside" of their normal dependency lists. + sourceDir = r.fs.Cwd() + if r.debugLogs != nil { + r.debugLogs.addNote(fmt.Sprintf(" Changed resolve directory to %q", sourceDir)) + } + } else if r.debugLogs != nil { r.debugLogs.addNote(" Failed to find any package alias matches") } }