fix #2050: define now matches index strings

CHANGELOG.md

@@ -2,6 +2,30 @@
 
 ## Unreleased
 
+* Match `define` to strings in index expressions ([#2050](https://github.com/evanw/esbuild/issues/2050))
+
+    With this release, configuring `--define:foo.bar=baz` now matches and replaces both `foo.bar` and `foo['bar']` expressions in the original source code. This is necessary for people who have enabled TypeScript's `noPropertyAccessFromIndexSignature` feature, which prevents you from using normal property access syntax on a type with an index signature such as in the following code:
+
+    ```ts
+    declare let foo: { [key: string]: any }
+    foo.bar // This is a type error if noPropertyAccessFromIndexSignature is enabled
+    foo['bar']
+    ```
+
+    Previously esbuild would generate the following output with `--define:foo.bar=baz`:
+
+    ```js
+    baz;
+    foo["bar"];
+    ```
+
+    Now esbuild will generate the following output instead:
+
+    ```js
+    baz;
+    baz;
+    ```
+
 * Parse and discard TypeScript `export as namespace` statements ([#2070](https://github.com/evanw/esbuild/issues/2070))
 
     TypeScript `.d.ts` type declaration files can sometimes contain statements of the form `export as namespace foo;`. I believe these serve to declare that the module adds a property of that name to the global object. You aren't supposed to feed `.d.ts` files to esbuild so this normally doesn't matter, but sometimes esbuild can end up having to parse them. One such case is if you import a type-only package who's `main` field in `package.json` is a `.d.ts` file.

internal/js_ast/js_ast.go

@@ -547,10 +547,21 @@ type EIndex struct {
 	Target        Expr
 	Index         Expr
 	OptionalChain OptionalChain
+
+	// If true, this property access is known to be free of side-effects. That
+	// means it can be removed if the resulting value isn't used.
+	CanBeRemovedIfUnused bool
+
+	// If true, this property access is a function that, when called, can be
+	// unwrapped if the resulting value is unused. Unwrapping means discarding
+	// the call target but keeping any arguments with side effects.
+	CallCanBeUnwrappedIfUnused bool
 }
 
 func (a *EIndex) HasSameFlagsAs(b *EIndex) bool {
-	return a.OptionalChain == b.OptionalChain
+	return a.OptionalChain == b.OptionalChain &&
+		a.CanBeRemovedIfUnused == b.CanBeRemovedIfUnused &&
+		a.CallCanBeUnwrappedIfUnused == b.CallCanBeUnwrappedIfUnused
 }
 
 type EArrow struct {

internal/js_parser/js_parser.go

@@ -10553,14 +10553,24 @@ func (p *parser) visitArgs(args []js_ast.Arg, opts visitArgsOpts) {
 	}
 }
 
-func (p *parser) isDotDefineMatch(expr js_ast.Expr, parts []string) bool {
+func (p *parser) isDotOrIndexDefineMatch(expr js_ast.Expr, parts []string) bool {
 	switch e := expr.Data.(type) {
 	case *js_ast.EDot:
 		if len(parts) > 1 {
 			// Intermediates must be dot expressions
 			last := len(parts) - 1
-			return parts[last] == e.Name && e.OptionalChain == js_ast.OptionalChainNone &&
-				p.isDotDefineMatch(e.Target, parts[:last])
+			return e.OptionalChain == js_ast.OptionalChainNone && parts[last] == e.Name &&
+				p.isDotOrIndexDefineMatch(e.Target, parts[:last])
+		}
+
+	case *js_ast.EIndex:
+		if len(parts) > 1 {
+			if str, ok := e.Index.Data.(*js_ast.EString); ok {
+				// Intermediates must be dot expressions
+				last := len(parts) - 1
+				return e.OptionalChain == js_ast.OptionalChainNone && parts[last] == helpers.UTF16ToString(str.Value) &&
+					p.isDotOrIndexDefineMatch(e.Target, parts[:last])
+			}
 		}
 
 	case *js_ast.EThis:
@@ -11506,7 +11516,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 		// Check both user-specified defines and known globals
 		if defines, ok := p.options.defines.DotDefines["meta"]; ok {
 			for _, define := range defines {
-				if p.isDotDefineMatch(expr, define.Parts) {
+				if p.isDotOrIndexDefineMatch(expr, define.Parts) {
 					// Substitute user-specified defines
 					if define.Data.DefineFunc != nil {
 						return p.valueForDefine(expr.Loc, define.Data.DefineFunc, identifierOpts{
@@ -12325,7 +12335,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 		// Check both user-specified defines and known globals
 		if defines, ok := p.options.defines.DotDefines[e.Name]; ok {
 			for _, define := range defines {
-				if p.isDotDefineMatch(expr, define.Parts) {
+				if p.isDotOrIndexDefineMatch(expr, define.Parts) {
 					// Substitute user-specified defines
 					if define.Data.DefineFunc != nil {
 						return p.valueForDefine(expr.Loc, define.Data.DefineFunc, identifierOpts{
@@ -12407,6 +12417,33 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 		isTemplateTag := e == p.templateTag
 		isDeleteTarget := e == p.deleteTarget
 
+		// Check both user-specified defines and known globals
+		if str, ok := e.Index.Data.(*js_ast.EString); ok {
+			if defines, ok := p.options.defines.DotDefines[helpers.UTF16ToString(str.Value)]; ok {
+				for _, define := range defines {
+					if p.isDotOrIndexDefineMatch(expr, define.Parts) {
+						// Substitute user-specified defines
+						if define.Data.DefineFunc != nil {
+							return p.valueForDefine(expr.Loc, define.Data.DefineFunc, identifierOpts{
+								assignTarget:   in.assignTarget,
+								isCallTarget:   isCallTarget,
+								isDeleteTarget: isDeleteTarget,
+							}), exprOut{}
+						}
+
+						// Copy the side effect flags over in case this expression is unused
+						if define.Data.CanBeRemovedIfUnused {
+							e.CanBeRemovedIfUnused = true
+						}
+						if define.Data.CallCanBeUnwrappedIfUnused && !p.options.ignoreDCEAnnotations {
+							e.CallCanBeUnwrappedIfUnused = true
+						}
+						break
+					}
+				}
+			}
+		}
+
 		// "a['b']" => "a.b"
 		if p.options.minifySyntax {
 			if str, ok := e.Index.Data.(*js_ast.EString); ok && js_lexer.IsIdentifierUTF16(str.Value) {
@@ -13345,6 +13382,12 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 			if t.CallCanBeUnwrappedIfUnused {
 				e.CanBeUnwrappedIfUnused = true
 			}
+
+		case *js_ast.EIndex:
+			// Copy the call side effect flag over if this is a known target
+			if t.CallCanBeUnwrappedIfUnused {
+				e.CanBeUnwrappedIfUnused = true
+			}
 		}
 
 		// Handle parenthesized optional chains

scripts/js-api-tests.js

@@ -3838,8 +3838,18 @@ let transformTests = {
 
   async define({ esbuild }) {
     const define = { 'process.env.NODE_ENV': '"production"' }
-    const { code } = await esbuild.transform(`console.log(process.env.NODE_ENV)`, { define })
-    assert.strictEqual(code, `console.log("production");\n`)
+
+    const { code: code1 } = await esbuild.transform(`console.log(process.env.NODE_ENV)`, { define })
+    assert.strictEqual(code1, `console.log("production");\n`)
+
+    const { code: code2 } = await esbuild.transform(`console.log(process.env['NODE_ENV'])`, { define })
+    assert.strictEqual(code2, `console.log("production");\n`)
+
+    const { code: code3 } = await esbuild.transform(`console.log(process['env'].NODE_ENV)`, { define })
+    assert.strictEqual(code3, `console.log("production");\n`)
+
+    const { code: code4 } = await esbuild.transform(`console.log(process['env']['NODE_ENV'])`, { define })
+    assert.strictEqual(code4, `console.log("production");\n`)
   },
 
   async defineBuiltInConstants({ esbuild }) {