From 58e27340da3846eb91f68dd5eceb64ec19e561e1 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Wed, 2 Mar 2022 22:22:57 +0000 Subject: [PATCH] fix #2050: `define` now matches index strings --- CHANGELOG.md | 24 +++++++++++++++ internal/js_ast/js_ast.go | 13 +++++++- internal/js_parser/js_parser.go | 53 +++++++++++++++++++++++++++++---- scripts/js-api-tests.js | 14 +++++++-- 4 files changed, 96 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a896e793d96..ffa0f1127ca 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/internal/js_ast/js_ast.go b/internal/js_ast/js_ast.go index b5bb33b8523..1335e2be9e9 100644 --- a/internal/js_ast/js_ast.go +++ b/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 { diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 8a2ba070b55..f6edc3bb754 100644 --- a/internal/js_parser/js_parser.go +++ b/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 diff --git a/scripts/js-api-tests.js b/scripts/js-api-tests.js index e8bc68bb628..8ad5764c552 100644 --- a/scripts/js-api-tests.js +++ b/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 }) {