From 8b5e048786ecb72364f1ff6ab4c7bc30a0ea7e06 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Tue, 5 Jul 2022 23:21:12 -0400 Subject: [PATCH] follow-up to #2371 --- CHANGELOG.md | 34 +++++++++++++++++++++ internal/bundler/bundler_ts_test.go | 27 +++++++++++----- internal/bundler/snapshots/snapshots_ts.txt | 21 +++++++++---- internal/js_printer/js_printer.go | 2 +- 4 files changed, 70 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c354110c20..08666e5507d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,39 @@ # Changelog +## Unreleased + +* Fix generated TypeScript `enum` comments containing `*/` ([#2369](https://github.com/evanw/esbuild/issues/2369), [#2371](https://github.com/evanw/esbuild/pull/2371)) + + TypeScript `enum` values that are equal to a number or string literal are inlined (references to the enum are replaced with the literal value) and have a `/* ... */` comment after them with the original enum name to improve readability. However, this comment is omitted if the enum name contains the character sequence `*/` because that would end the comment early and cause a syntax error: + + ```ts + // Original TypeScript + enum Foo { '/*' = 1, '*/' = 2 } + console.log(Foo['/*'], Foo['*/']) + + // Generated JavaScript + console.log(1 /* /* */, 2); + ``` + + This was originally handled correctly when TypeScript `enum` inlining was initially implemented since it was only supported within a single file. However, when esbuild was later extended to support TypeScript `enum` inlining across files, this special case where the enum name contains `*/` was not handled in that new code. Starting with this release, esbuild will now handle enums with names containing `*/` correctly when they are inlined across files: + + ```ts + // foo.ts + export enum Foo { '/*' = 1, '*/' = 2 } + + // bar.ts + import { Foo } from './foo' + console.log(Foo['/*'], Foo['*/']) + + // Old output (with --bundle --format=esm) + console.log(1 /* /* */, 2 /* */ */); + + // New output (with --bundle --format=esm) + console.log(1 /* /* */, 2); + ``` + + This fix was contributed by [@magic-akari](https://github.com/magic-akari). + ## 0.14.48 * Enable using esbuild in Deno via WebAssembly ([#2323](https://github.com/evanw/esbuild/issues/2323)) diff --git a/internal/bundler/bundler_ts_test.go b/internal/bundler/bundler_ts_test.go index 2d730302ac9..eb4f5ee93bd 100644 --- a/internal/bundler/bundler_ts_test.go +++ b/internal/bundler/bundler_ts_test.go @@ -223,18 +223,31 @@ func TestTSDeclareConstEnum(t *testing.T) { }) } -func TestConstEnumComments(t *testing.T) { +func TestTSConstEnumComments(t *testing.T) { ts_suite.expectBundled(t, bundled{ files: map[string]string{ "/bar.ts": ` - export const enum PRCD { - "*/%" = 14, - } + export const enum Foo { + "%/*" = 1, + "*/%" = 2, + } `, "/foo.ts": ` - import { PRCD } from "./bar"; - - console.log(PRCD["*/%"]); + import { Foo } from "./bar"; + const enum Bar { + "%/*" = 1, + "*/%" = 2, + } + console.log({ + 'should have comments': [ + Foo["%/*"], + Bar["%/*"], + ], + 'should not have comments': [ + Foo["*/%"], + Bar["*/%"], + ], + }); `, }, entryPaths: []string{"/foo.ts"}, diff --git a/internal/bundler/snapshots/snapshots_ts.txt b/internal/bundler/snapshots/snapshots_ts.txt index 49ff10f6b79..25bf5c03fcc 100644 --- a/internal/bundler/snapshots/snapshots_ts.txt +++ b/internal/bundler/snapshots/snapshots_ts.txt @@ -1,9 +1,3 @@ -TestConstEnumComments ----------- /out.js ---------- -// foo.ts -console.log(14); - -================================================================================ TestExportTypeIssue379 ---------- /out.js ---------- // a.ts @@ -37,6 +31,21 @@ var foo4 = 123; // entry.ts console.log(a_exports, b_exports, c_exports, d_exports); +================================================================================ +TestTSConstEnumComments +---------- /out.js ---------- +// foo.ts +console.log({ + "should have comments": [ + 1 /* %/* */, + 1 /* %/* */ + ], + "should not have comments": [ + 2, + 2 + ] +}); + ================================================================================ TestTSDeclareClass ---------- /out.js ---------- diff --git a/internal/js_printer/js_printer.go b/internal/js_printer/js_printer.go index cb9e2d6f0e2..e0f77992385 100644 --- a/internal/js_printer/js_printer.go +++ b/internal/js_printer/js_printer.go @@ -1940,7 +1940,7 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla } else { p.printNumber(value.Number, level) } - if !p.options.MinifyWhitespace && !p.options.MinifyIdentifiers { + if !p.options.MinifyWhitespace && !p.options.MinifyIdentifiers && !strings.Contains(e.Name, "*/") { p.print(" /* ") p.print(e.Name) p.print(" */")