fix #3477: forbid --keep-names if not supported

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+* Forbid `--keep-names` when targeting old browsers ([#3477](https://github.com/evanw/esbuild/issues/3477))
+
+    The `--keep-names` setting needs to be able to assign to the `name` property on functions and classes. However, before ES6 this property was non-configurable, and attempting to assign to it would throw an error. So with this release, esbuild will no longer allow you to enable this setting and also target a really old browser.
+
 ## 0.19.6
 
 * Fix a constant folding bug with bigint equality

compat-table/src/index.ts

@@ -53,6 +53,7 @@ export const jsFeatures = {
   ExportStarAs: true,
   ForAwait: true,
   ForOf: true,
+  FunctionNameConfigurable: true,
   FunctionOrClassPropertyAccess: true,
   Generator: true,
   Hashbang: true,
@@ -332,6 +333,7 @@ import('./kangax').then(kangax => {
   js.Destructuring.ES = { 2015: { force: true } }
   js.DynamicImport.ES = { 2015: { force: true } }
   js.ForOf.ES = { 2015: { force: true } }
+  js.FunctionNameConfigurable.ES = { 2015: { force: true } }
   js.Generator.ES = { 2015: { force: true } }
   js.NewTarget.ES = { 2015: { force: true } }
   js.ObjectExtensions.ES = { 2015: { force: true } }

compat-table/src/kangax.ts

@@ -31,6 +31,7 @@ const features: Record<string, JSFeature> = {
   'destructuring, declarations': 'Destructuring',
   'destructuring, parameters': 'Destructuring',
   'for..of loops': 'ForOf',
+  'function "name" property: isn\'t writable, is configurable': 'FunctionNameConfigurable',
   'generators': 'Generator',
   'let': 'ConstAndLet',
   'new.target': 'NewTarget',

internal/compat/js_table.go

@@ -86,6 +86,7 @@ const (
 	ExportStarAs
 	ForAwait
 	ForOf
+	FunctionNameConfigurable
 	FunctionOrClassPropertyAccess
 	Generator
 	Hashbang
@@ -146,6 +147,7 @@ var StringToJSFeature = map[string]JSFeature{
 	"export-star-as":                    ExportStarAs,
 	"for-await":                         ForAwait,
 	"for-of":                            ForOf,
+	"function-name-configurable":        FunctionNameConfigurable,
 	"function-or-class-property-access": FunctionOrClassPropertyAccess,
 	"generator":                         Generator,
 	"hashbang":                          Hashbang,
@@ -521,6 +523,20 @@ var jsTable = map[JSFeature]map[Engine][]versionRange{
 		Opera:   {{start: v{38, 0, 0}}},
 		Safari:  {{start: v{10, 0, 0}}},
 	},
+	FunctionNameConfigurable: {
+		// Note: The latest version of "IE" failed this test: function "name" property: isn't writable, is configurable
+		// Note: The latest version of "Rhino" failed this test: function "name" property: isn't writable, is configurable
+		Chrome:  {{start: v{43, 0, 0}}},
+		Deno:    {{start: v{1, 0, 0}}},
+		Edge:    {{start: v{12, 0, 0}}},
+		ES:      {{start: v{2015, 0, 0}}},
+		Firefox: {{start: v{38, 0, 0}}},
+		Hermes:  {{start: v{0, 7, 0}}},
+		IOS:     {{start: v{10, 0, 0}}},
+		Node:    {{start: v{4, 0, 0}}},
+		Opera:   {{start: v{30, 0, 0}}},
+		Safari:  {{start: v{10, 0, 0}}},
+	},
 	FunctionOrClassPropertyAccess: {
 		Chrome:  {{start: v{0, 0, 0}}},
 		Deno:    {{start: v{0, 0, 0}}},

internal/config/config.go

@@ -813,3 +813,27 @@ type OnLoadResult struct {
 
 	Loader Loader
 }
+
+func PrettyPrintTargetEnvironment(originalTargetEnv string, unsupportedJSFeatureOverridesMask compat.JSFeature) (where string) {
+	where = "the configured target environment"
+	overrides := ""
+	if unsupportedJSFeatureOverridesMask != 0 {
+		count := 0
+		mask := unsupportedJSFeatureOverridesMask
+		for mask != 0 {
+			if (mask & 1) != 0 {
+				count++
+			}
+			mask >>= 1
+		}
+		s := "s"
+		if count == 1 {
+			s = ""
+		}
+		overrides = fmt.Sprintf(" + %d override%s", count, s)
+	}
+	if originalTargetEnv != "" {
+		where = fmt.Sprintf("%s (%s%s)", where, originalTargetEnv, overrides)
+	}
+	return
+}

internal/js_parser/js_parser.go

@@ -11906,10 +11906,10 @@ func (p *parser) checkForUnrepresentableIdentifier(loc logger.Loc, name string)
 		}
 		if !p.unrepresentableIdentifiers[name] {
 			p.unrepresentableIdentifiers[name] = true
-			where, notes := p.prettyPrintTargetEnvironment(compat.UnicodeEscapes)
+			where := config.PrettyPrintTargetEnvironment(p.options.originalTargetEnv, p.options.unsupportedJSFeatureOverridesMask)
 			r := js_lexer.RangeOfIdentifier(p.source, loc)
-			p.log.AddErrorWithNotes(&p.tracker, r, fmt.Sprintf("%q cannot be escaped in %s but you "+
-				"can set the charset to \"utf8\" to allow unescaped Unicode characters", name, where), notes)
+			p.log.AddError(&p.tracker, r, fmt.Sprintf("%q cannot be escaped in %s but you "+
+				"can set the charset to \"utf8\" to allow unescaped Unicode characters", name, where))
 		}
 	}
 }
@@ -12489,7 +12489,6 @@ func containsClosingScriptTag(text string) bool {
 }
 
 func (p *parser) isUnsupportedRegularExpression(loc logger.Loc, value string) (pattern string, flags string, isUnsupported bool) {
-	var feature compat.JSFeature
 	var what string
 	var r logger.Range
 
@@ -12531,7 +12530,6 @@ pattern:
 
 			if strings.HasPrefix(tail, "?<=") || strings.HasPrefix(tail, "?<!") {
 				if p.options.unsupportedJSFeatures.Has(compat.RegexpLookbehindAssertions) {
-					feature = compat.RegexpLookbehindAssertions
 					what = "Lookbehind assertions in regular expressions are not available"
 					r = logger.Range{Loc: logger.Loc{Start: loc.Start + int32(i) + 1}, Len: 3}
 					isUnsupported = true
@@ -12540,7 +12538,6 @@ pattern:
 			} else if strings.HasPrefix(tail, "?<") {
 				if p.options.unsupportedJSFeatures.Has(compat.RegexpNamedCaptureGroups) {
 					if end := strings.IndexByte(tail, '>'); end >= 0 {
-						feature = compat.RegexpNamedCaptureGroups
 						what = "Named capture groups in regular expressions are not available"
 						r = logger.Range{Loc: logger.Loc{Start: loc.Start + int32(i) + 1}, Len: int32(end) + 1}
 						isUnsupported = true
@@ -12566,7 +12563,6 @@ pattern:
 			if isUnicode && (strings.HasPrefix(tail, "p{") || strings.HasPrefix(tail, "P{")) {
 				if p.options.unsupportedJSFeatures.Has(compat.RegexpUnicodePropertyEscapes) {
 					if end := strings.IndexByte(tail, '}'); end >= 0 {
-						feature = compat.RegexpUnicodePropertyEscapes
 						what = "Unicode property escapes in regular expressions are not available"
 						r = logger.Range{Loc: logger.Loc{Start: loc.Start + int32(i)}, Len: int32(end) + 2}
 						isUnsupported = true
@@ -12589,25 +12585,21 @@ pattern:
 				if !p.options.unsupportedJSFeatures.Has(compat.RegexpDotAllFlag) {
 					continue // This is part of ES2018
 				}
-				feature = compat.RegexpDotAllFlag
 
 			case 'y', 'u':
 				if !p.options.unsupportedJSFeatures.Has(compat.RegexpStickyAndUnicodeFlags) {
 					continue // These are part of ES2018
 				}
-				feature = compat.RegexpStickyAndUnicodeFlags
 
 			case 'd':
 				if !p.options.unsupportedJSFeatures.Has(compat.RegexpMatchIndices) {
 					continue // This is part of ES2022
 				}
-				feature = compat.RegexpMatchIndices
 
 			case 'v':
 				if !p.options.unsupportedJSFeatures.Has(compat.RegexpSetNotation) {
 					continue // This is from a proposal: https://github.com/tc39/proposal-regexp-v-flag
 				}
-				feature = compat.RegexpSetNotation
 
 			default:
 				// Unknown flags are never supported
@@ -12621,11 +12613,11 @@ pattern:
 	}
 
 	if isUnsupported {
-		where, notes := p.prettyPrintTargetEnvironment(feature)
-		p.log.AddIDWithNotes(logger.MsgID_JS_UnsupportedRegExp, logger.Debug, &p.tracker, r, fmt.Sprintf("%s in %s", what, where), append(notes, logger.MsgData{
+		where := config.PrettyPrintTargetEnvironment(p.options.originalTargetEnv, p.options.unsupportedJSFeatureOverridesMask)
+		p.log.AddIDWithNotes(logger.MsgID_JS_UnsupportedRegExp, logger.Debug, &p.tracker, r, fmt.Sprintf("%s in %s", what, where), []logger.MsgData{{
 			Text: "This regular expression literal has been converted to a \"new RegExp()\" constructor " +
 				"to avoid generating code with a syntax error. However, you will need to include a " +
-				"polyfill for \"RegExp\" for your code to have the correct behavior at run-time."}))
+				"polyfill for \"RegExp\" for your code to have the correct behavior at run-time."}})
 	}
 
 	return

internal/js_parser/js_parser_lower.go

@@ -15,30 +15,6 @@ import (
 	"github.com/evanw/esbuild/internal/logger"
 )
 
-func (p *parser) prettyPrintTargetEnvironment(feature compat.JSFeature) (where string, notes []logger.MsgData) {
-	where = "the configured target environment"
-	overrides := ""
-	if p.options.unsupportedJSFeatureOverridesMask != 0 {
-		count := 0
-		mask := p.options.unsupportedJSFeatureOverridesMask
-		for mask != 0 {
-			if (mask & 1) != 0 {
-				count++
-			}
-			mask >>= 1
-		}
-		s := "s"
-		if count == 1 {
-			s = ""
-		}
-		overrides = fmt.Sprintf(" + %d override%s", count, s)
-	}
-	if p.options.originalTargetEnv != "" {
-		where = fmt.Sprintf("%s (%s%s)", where, p.options.originalTargetEnv, overrides)
-	}
-	return
-}
-
 func (p *parser) markSyntaxFeature(feature compat.JSFeature, r logger.Range) (didGenerateError bool) {
 	didGenerateError = true
 
@@ -54,7 +30,7 @@ func (p *parser) markSyntaxFeature(feature compat.JSFeature, r logger.Range) (di
 	}
 
 	var name string
-	where, notes := p.prettyPrintTargetEnvironment(feature)
+	where := config.PrettyPrintTargetEnvironment(p.options.originalTargetEnv, p.options.unsupportedJSFeatureOverridesMask)
 
 	switch feature {
 	case compat.DefaultArgument:
@@ -106,24 +82,24 @@ func (p *parser) markSyntaxFeature(feature compat.JSFeature, r logger.Range) (di
 		name = "JavaScript decorators"
 
 	case compat.ImportAttributes:
-		p.log.AddErrorWithNotes(&p.tracker, r, fmt.Sprintf(
-			"Using an arbitrary value as the second argument to \"import()\" is not possible in %s", where), notes)
+		p.log.AddError(&p.tracker, r, fmt.Sprintf(
+			"Using an arbitrary value as the second argument to \"import()\" is not possible in %s", where))
 		return
 
 	case compat.TopLevelAwait:
-		p.log.AddErrorWithNotes(&p.tracker, r, fmt.Sprintf(
-			"Top-level await is not available in %s", where), notes)
+		p.log.AddError(&p.tracker, r, fmt.Sprintf(
+			"Top-level await is not available in %s", where))
 		return
 
 	case compat.ArbitraryModuleNamespaceNames:
-		p.log.AddErrorWithNotes(&p.tracker, r, fmt.Sprintf(
-			"Using a string as a module namespace identifier name is not supported in %s", where), notes)
+		p.log.AddError(&p.tracker, r, fmt.Sprintf(
+			"Using a string as a module namespace identifier name is not supported in %s", where))
 		return
 
 	case compat.Bigint:
 		// Transforming these will never be supported
-		p.log.AddErrorWithNotes(&p.tracker, r, fmt.Sprintf(
-			"Big integer literals are not available in %s", where), notes)
+		p.log.AddError(&p.tracker, r, fmt.Sprintf(
+			"Big integer literals are not available in %s", where))
 		return
 
 	case compat.ImportMeta:
@@ -132,18 +108,18 @@ func (p *parser) markSyntaxFeature(feature compat.JSFeature, r logger.Range) (di
 		if p.suppressWarningsAboutWeirdCode || p.fnOrArrowDataVisit.tryBodyCount > 0 {
 			kind = logger.Debug
 		}
-		p.log.AddIDWithNotes(logger.MsgID_JS_EmptyImportMeta, kind, &p.tracker, r, fmt.Sprintf(
-			"\"import.meta\" is not available in %s and will be empty", where), notes)
+		p.log.AddID(logger.MsgID_JS_EmptyImportMeta, kind, &p.tracker, r, fmt.Sprintf(
+			"\"import.meta\" is not available in %s and will be empty", where))
 		return
 
 	default:
-		p.log.AddErrorWithNotes(&p.tracker, r, fmt.Sprintf(
-			"This feature is not available in %s", where), notes)
+		p.log.AddError(&p.tracker, r, fmt.Sprintf(
+			"This feature is not available in %s", where))
 		return
 	}
 
-	p.log.AddErrorWithNotes(&p.tracker, r, fmt.Sprintf(
-		"Transforming %s to %s is not supported yet", name, where), notes)
+	p.log.AddError(&p.tracker, r, fmt.Sprintf(
+		"Transforming %s to %s is not supported yet", name, where))
 	return
 }
 

pkg/api/api_impl.go

@@ -771,6 +771,15 @@ func validateBannerOrFooter(log logger.Log, name string, values map[string]strin
 	return
 }
 
+func validateKeepNames(log logger.Log, options *config.Options) {
+	if options.KeepNames && options.UnsupportedJSFeatures.Has(compat.FunctionNameConfigurable) {
+		where := config.PrettyPrintTargetEnvironment(options.OriginalTargetEnv, options.UnsupportedJSFeatureOverridesMask)
+		log.AddErrorWithNotes(nil, logger.Range{}, fmt.Sprintf("The \"keep names\" setting cannot be used with %s", where), []logger.MsgData{{
+			Text: "In this environment, the \"Function.prototype.name\" property is not configurable and assigning to it will throw an error. " +
+				"Either use a newer target environment or disable the \"keep names\" setting."}})
+	}
+}
+
 func convertLocationToPublic(loc *logger.MsgLocation) *Location {
 	if loc != nil {
 		return &Location{
@@ -1321,6 +1330,7 @@ func validateBuildOptions(
 		CSSFooter:             footerCSS,
 		PreserveSymlinks:      buildOpts.PreserveSymlinks,
 	}
+	validateKeepNames(log, &options)
 	if buildOpts.Conditions != nil {
 		options.Conditions = append([]string{}, buildOpts.Conditions...)
 	}
@@ -1755,6 +1765,7 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult
 			SourceFile: transformOpts.Sourcefile,
 		},
 	}
+	validateKeepNames(log, &options)
 	if options.Stdin.Loader.IsCSS() {
 		options.CSSBanner = transformOpts.Banner
 		options.CSSFooter = transformOpts.Footer

scripts/js-api-tests.js

@@ -6611,6 +6611,26 @@ class Foo {
     assert.strictEqual((await esbuild.transform(`class Foo { static { x } }`, { supported: { 'class-static-blocks': false } })).code, `class Foo {\n}\nx;\n`)
   },
 
+  async keepNamesUnsupported({ esbuild }) {
+    try {
+      await esbuild.transform(``, { keepNames: true, target: 'chrome36' })
+      throw new Error('Expected an error to be thrown')
+    } catch (e) {
+      assert.strictEqual(e.errors[0].text,
+        'The "keep names" setting cannot be used with the configured target environment ("chrome36")')
+    }
+
+    try {
+      await esbuild.transform(``, { keepNames: true, target: 'chrome46', supported: { 'function-name-configurable': false } })
+      throw new Error('Expected an error to be thrown')
+    } catch (e) {
+      assert.strictEqual(e.errors[0].text,
+        'The "keep names" setting cannot be used with the configured target environment ("chrome46" + 1 override)')
+    }
+
+    await esbuild.transform(``, { keepNames: true, target: 'chrome46' })
+  },
+
   async inlineScript({ esbuild }) {
     let p
     assert.strictEqual((await esbuild.transform(`x = '</script>'`, {})).code, `x = "<\\/script>";\n`)