fix #4509: pick minimum version for target env

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Handle target collisions ([#4509](https://github.com/evanw/esbuild/issues/4509))
+
+    It's possible to specify the same target engine multiple times, such as with `--target=chrome1,chrome99`. This edge case wasn't anticipated and previously took the last version for the duplicated target engine instead of the minimum version (so `chrome99` in this case instead of `chrome1`). With this release, esbuild will now pick the minimum version between all duplicated target engines.
+
 * Force `.mp3` files to use the `audio/mpeg` MIME type ([#4485](https://github.com/evanw/esbuild/issues/4485))
 
     MIME type detection for esbuild's data URLs uses Go's built-in MIME type detection, which is based on the [MIME sniffing standard](https://mimesniff.spec.whatwg.org/). This works correctly for MP3 files that start with the byte sequence `ID3`, which is commonly the case. However, it's possible to construct valid MP3 files that do not start with `ID3`, and that perhaps Go's built-in MIME type detection doesn't implement the "Signature for MP3 without ID3" part of the algorithm. This results in some `.mp3` files incorrectly using the `application/octet-stream` MIME type instead of `audio/mpeg`. With this release, esbuild will now always use the `audio/mpeg` MIME type for files ending in `.mp3`.

internal/compat/compat.go

@@ -57,6 +57,92 @@ func compareVersions(a v, b Semver) int {
 	return diff
 }
 
+func CompareSemver(a Semver, b Semver) int {
+	an := len(a.Parts)
+	bn := len(b.Parts)
+
+	n := an
+	if bn > n {
+		n = bn
+	}
+
+	for i := 0; i < n; i++ {
+		ai := 0
+		bi := 0
+		if i < an {
+			ai = a.Parts[i]
+		}
+		if i < bn {
+			bi = b.Parts[i]
+		}
+		if ai != bi {
+			return ai - bi
+		}
+	}
+
+	if (a.PreRelease != "") != (b.PreRelease != "") {
+		return len(b.PreRelease) - len(a.PreRelease)
+	}
+
+	a_tail := strings.TrimPrefix(a.PreRelease, "-")
+	b_tail := strings.TrimPrefix(b.PreRelease, "-")
+
+	for a_tail != "" && b_tail != "" {
+		var a_head, b_head string
+		a_head, a_tail = splitOffNextPreReleasePart(a_tail)
+		b_head, b_tail = splitOffNextPreReleasePart(b_tail)
+		if a_head == b_head {
+			continue
+		}
+
+		// Check for numbers
+		a_num, a_isnum := preReleasePartToNumber(a_head)
+		b_num, b_isnum := preReleasePartToNumber(b_head)
+		if !a_isnum && !b_isnum {
+			// Lexicographic comparison for non-numbers
+			if a_head < b_head {
+				return -1
+			} else {
+				return 1
+			}
+		} else if a_isnum && b_isnum {
+			// Numeric comparison for numbers
+			if a_num != b_num {
+				return a_num - b_num
+			} else {
+				// Compare lengths for different text but equal numbers (e.g. "0" vs. "00")
+				return len(a_head) - len(b_head)
+			}
+		} else {
+			// One is a number and the other isn't
+			if a_isnum {
+				return -1
+			} else {
+				return 1
+			}
+		}
+	}
+
+	return len(a_tail) - len(b_tail)
+}
+
+func splitOffNextPreReleasePart(text string) (head string, tail string) {
+	if i := strings.IndexByte(text, '.'); i != -1 {
+		return text[:i], text[i+1:]
+	}
+	return text, ""
+}
+
+func preReleasePartToNumber(text string) (int, bool) {
+	for _, c := range text {
+		if c < '0' || c > '9' {
+			return 0, false
+		}
+	}
+	n, err := strconv.Atoi(text)
+	return n, err == nil
+}
+
 // The start is inclusive and the end is exclusive
 type versionRange struct {
 	start v

internal/compat/compat_test.go

@@ -61,3 +61,70 @@ func TestCompareVersions(t *testing.T) {
 	check(v{1, 2, 3}, Semver{Parts: []int{1, 2, 3}, PreRelease: "-pre"}, '>')
 	check(v{1, 2, 2}, Semver{Parts: []int{1, 2, 3}, PreRelease: "-pre"}, '<')
 }
+
+func TestCompareSemver(t *testing.T) {
+	t.Helper()
+
+	check := func(a Semver, b Semver, expected rune) {
+		t.Helper()
+
+		at := a.String()
+		bt := b.String()
+
+		t.Run(fmt.Sprintf("%q ? %q", at, bt), func(t *testing.T) {
+			observed := '='
+			if result := CompareSemver(a, b); result < 0 {
+				observed = '<'
+			} else if result > 0 {
+				observed = '>'
+			}
+			if observed != expected {
+				test.AssertEqual(t, fmt.Sprintf("%c", observed), fmt.Sprintf("%c", expected))
+			}
+		})
+
+		// Automatically generate reverse comparison cases to cut down on test count
+		if expected == '<' {
+			t.Run(fmt.Sprintf("%q ? %q", bt, at), func(t *testing.T) {
+				observed := '='
+				if result := CompareSemver(b, a); result < 0 {
+					observed = '<'
+				} else if result > 0 {
+					observed = '>'
+				}
+				if observed != '>' {
+					test.AssertEqual(t, fmt.Sprintf("%c", observed), ">")
+				}
+			})
+		} else if expected == '>' {
+			t.Fail()
+		}
+	}
+
+	check(Semver{}, Semver{}, '=')
+	check(Semver{Parts: []int{0}}, Semver{}, '=')
+	check(Semver{}, Semver{Parts: []int{0}}, '=')
+	check(Semver{Parts: []int{0}}, Semver{Parts: []int{0}}, '=')
+	check(Semver{Parts: []int{0, 0}}, Semver{Parts: []int{0}}, '=')
+	check(Semver{Parts: []int{0}}, Semver{Parts: []int{0}}, '=')
+	check(Semver{Parts: []int{0, 0}}, Semver{Parts: []int{0, 0}}, '=')
+	check(Semver{Parts: []int{1}, PreRelease: "-alpha"}, Semver{Parts: []int{1}, PreRelease: "-alpha"}, '=')
+	check(Semver{Parts: []int{1, 0}, PreRelease: "-alpha"}, Semver{Parts: []int{1}, PreRelease: "-alpha"}, '=')
+	check(Semver{Parts: []int{1, 0}, PreRelease: "-alpha"}, Semver{Parts: []int{1, 0, 0}, PreRelease: "-alpha"}, '=')
+
+	check(Semver{}, Semver{Parts: []int{1}}, '<')
+	check(Semver{}, Semver{Parts: []int{0, 1}}, '<')
+	check(Semver{Parts: []int{0}}, Semver{Parts: []int{1}}, '<')
+	check(Semver{Parts: []int{0, 1}}, Semver{Parts: []int{1}}, '<')
+	check(Semver{Parts: []int{0, 1}}, Semver{Parts: []int{1, 0}}, '<')
+	check(Semver{Parts: []int{2, 0, 1}}, Semver{Parts: []int{2, 1, 0}}, '<')
+	check(Semver{Parts: []int{2, 0, 1}}, Semver{Parts: []int{2, 0, 2}}, '<')
+	check(Semver{Parts: []int{1}, PreRelease: "-alpha"}, Semver{Parts: []int{1}}, '<')
+	check(Semver{Parts: []int{1, 0}, PreRelease: "-alpha"}, Semver{Parts: []int{1}, PreRelease: "-alpha.1"}, '<')
+	check(Semver{Parts: []int{1, 0}, PreRelease: "-alpha.1"}, Semver{Parts: []int{1}, PreRelease: "-alpha.beta"}, '<')
+	check(Semver{Parts: []int{1, 0}, PreRelease: "-alpha.1"}, Semver{Parts: []int{1}, PreRelease: "-alpha.11"}, '<')
+	check(Semver{Parts: []int{1, 0}, PreRelease: "-alpha.1"}, Semver{Parts: []int{1}, PreRelease: "-alpha.02"}, '<')
+	check(Semver{Parts: []int{1, 0}, PreRelease: "-alpha.1"}, Semver{Parts: []int{1}, PreRelease: "-alpha.01"}, '<')
+	check(Semver{Parts: []int{1, 0}, PreRelease: "-alpha.01"}, Semver{Parts: []int{1}, PreRelease: "-alpha.2"}, '<')
+	check(Semver{Parts: []int{1, 0}, PreRelease: "-alpha.08"}, Semver{Parts: []int{1}, PreRelease: "-alpha.011"}, '<')
+}

pkg/api/api_impl.go

@@ -345,10 +345,15 @@ func validateFeatures(log logger.Log, target Target, engines []Engine) (compat.J
 						parts = append(parts, patch)
 					}
 				}
-				constraints[convertEngineName(engine.Name)] = compat.Semver{
+				name := convertEngineName(engine.Name)
+				new := compat.Semver{
 					Parts:      parts,
 					PreRelease: match[4],
 				}
+				if old, ok := constraints[name]; ok && compat.CompareSemver(old, new) < 0 {
+					continue
+				}
+				constraints[name] = new
 				continue
 			}
 		}

pkg/api/api_test.go

@@ -319,3 +319,20 @@ func TestFormatMessages(t *testing.T) {
 `,
 	)
 }
+
+func TestEngines(t *testing.T) {
+	checkChromeVersions := func(a string, b string) string {
+		result := api.Transform("a ?? b", api.TransformOptions{
+			Engines: []api.Engine{
+				{Name: api.EngineChrome, Version: a},
+				{Name: api.EngineChrome, Version: b},
+			},
+		})
+		return string(result.Code)
+	}
+
+	test.AssertEqualWithDiff(t, checkChromeVersions("99", "99"), "a ?? b;\n")
+	test.AssertEqualWithDiff(t, checkChromeVersions("9", "9"), "a != null ? a : b;\n")
+	test.AssertEqualWithDiff(t, checkChromeVersions("9", "99"), "a != null ? a : b;\n")
+	test.AssertEqualWithDiff(t, checkChromeVersions("99", "9"), "a != null ? a : b;\n")
+}