From 48a4ae517f6e5d8d051ea1c1e24b1c7bdd71f121 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sat, 3 Dec 2022 17:44:41 -0500 Subject: [PATCH] fix #2439: webpack magic comments in `new Worker` --- CHANGELOG.md | 4 ++++ internal/js_ast/js_ast.go | 8 ++++++-- internal/js_parser/js_parser.go | 23 ++++++++++++++--------- internal/js_printer/js_printer.go | 19 ++++++++++++++++--- internal/js_printer/js_printer_test.go | 17 ++++++++++++++++- 5 files changed, 56 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e717f1fb859..e480bfb8def 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ TypeScript code that does this should now be parsed correctly. +* Preserve Webpack comments inside constructor calls ([#2439](https://github.com/evanw/esbuild/issues/2439)) + + This improves the use of esbuild as a faster TypeScript-to-JavaScript frontend for Webpack, which has special [magic comments](https://webpack.js.org/api/module-methods/#magic-comments) inside `new Worker()` expressions that affect Webpack's behavior. + ## 0.15.16 * Add a package alias feature ([#2191](https://github.com/evanw/esbuild/issues/2191)) diff --git a/internal/js_ast/js_ast.go b/internal/js_ast/js_ast.go index 628e9aed64b..da05c6d00f2 100644 --- a/internal/js_ast/js_ast.go +++ b/internal/js_ast/js_ast.go @@ -492,8 +492,12 @@ var SEmptyShared = &SEmpty{} var SDebuggerShared = &SDebugger{} type ENew struct { - Target Expr - Args []Expr + Target Expr + Args []Expr + + // See this for more context: https://github.com/evanw/esbuild/issues/2439 + WebpackComments []Comment + CloseParenLoc logger.Loc IsMultiLine bool diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 9a48489af21..688e68c6e05 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -3341,19 +3341,21 @@ func (p *parser) parsePrefix(level js_ast.L, errors *deferredErrors, flags exprF } target := p.parseExprWithFlags(js_ast.LMember, flags) + var webpackComments []js_ast.Comment args := []js_ast.Expr{} var closeParenLoc logger.Loc var isMultiLine bool if p.lexer.Token == js_lexer.TOpenParen { - args, closeParenLoc, isMultiLine = p.parseCallArgs() + webpackComments, args, closeParenLoc, isMultiLine = p.parseCallArgs() } return js_ast.Expr{Loc: loc, Data: &js_ast.ENew{ - Target: target, - Args: args, - CloseParenLoc: closeParenLoc, - IsMultiLine: isMultiLine, + Target: target, + Args: args, + WebpackComments: webpackComments, + CloseParenLoc: closeParenLoc, + IsMultiLine: isMultiLine, }} case js_lexer.TOpenBracket: @@ -3839,7 +3841,7 @@ func (p *parser) parseSuffix(left js_ast.Expr, level js_ast.L, errors *deferredE if level >= js_ast.LCall { return left } - args, closeParenLoc, isMultiLine := p.parseCallArgs() + _, args, closeParenLoc, isMultiLine := p.parseCallArgs() left = js_ast.Expr{Loc: left.Loc, Data: &js_ast.ECall{ Target: left, Args: args, @@ -3861,7 +3863,7 @@ func (p *parser) parseSuffix(left js_ast.Expr, level js_ast.L, errors *deferredE if level >= js_ast.LCall { return left } - args, closeParenLoc, isMultiLine := p.parseCallArgs() + _, args, closeParenLoc, isMultiLine := p.parseCallArgs() left = js_ast.Expr{Loc: left.Loc, Data: &js_ast.ECall{ Target: left, Args: args, @@ -3963,7 +3965,7 @@ func (p *parser) parseSuffix(left js_ast.Expr, level js_ast.L, errors *deferredE if level >= js_ast.LCall { return left } - args, closeParenLoc, isMultiLine := p.parseCallArgs() + _, args, closeParenLoc, isMultiLine := p.parseCallArgs() left = js_ast.Expr{Loc: left.Loc, Data: &js_ast.ECall{ Target: left, Args: args, @@ -4474,11 +4476,13 @@ func (p *parser) parseExprOrLetStmt(opts parseStmtOpts) (js_ast.Expr, js_ast.Stm return p.parseSuffix(expr, js_ast.LLowest, nil, 0), js_ast.Stmt{}, nil } -func (p *parser) parseCallArgs() (args []js_ast.Expr, closeParenLoc logger.Loc, isMultiLine bool) { +func (p *parser) parseCallArgs() (webpackComments []js_ast.Comment, args []js_ast.Expr, closeParenLoc logger.Loc, isMultiLine bool) { // Allow "in" inside call arguments oldAllowIn := p.allowIn p.allowIn = true + oldWebpackComments := p.lexer.WebpackComments + p.lexer.WebpackComments = &webpackComments p.lexer.Expect(js_lexer.TOpenParen) for p.lexer.Token != js_lexer.TCloseParen { @@ -4509,6 +4513,7 @@ func (p *parser) parseCallArgs() (args []js_ast.Expr, closeParenLoc logger.Loc, isMultiLine = true } closeParenLoc = p.lexer.Loc() + p.lexer.WebpackComments = oldWebpackComments p.lexer.Expect(js_lexer.TCloseParen) p.allowIn = oldAllowIn return diff --git a/internal/js_printer/js_printer.go b/internal/js_printer/js_printer.go index 9119f80a808..689aa7c78da 100644 --- a/internal/js_printer/js_printer.go +++ b/internal/js_printer/js_printer.go @@ -1743,27 +1743,40 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla // Omit the "()" when minifying, but only when safe to do so if !p.options.MinifyWhitespace || len(e.Args) > 0 || level >= js_ast.LPostfix { - isMultiLine := e.IsMultiLine && len(e.Args) > 0 && !p.options.MinifyWhitespace + isMultiLine := !p.options.MinifyWhitespace && ((e.IsMultiLine && len(e.Args) > 0 && !p.options.MinifyWhitespace) || len(e.WebpackComments) > 0) + needsNewline := true p.print("(") if isMultiLine { p.options.Indent++ + if len(e.WebpackComments) > 0 { + p.printNewline() + needsNewline = false + for _, comment := range e.WebpackComments { + p.printIndentedComment(comment.Text) + } + } } for i, arg := range e.Args { if isMultiLine { if i != 0 { p.print(",") } - p.printNewline() + if needsNewline { + p.printNewline() + } p.printIndent() } else if i != 0 { p.print(",") p.printSpace() } p.printExpr(arg, js_ast.LComma, 0) + needsNewline = true } if isMultiLine { p.options.Indent-- - p.printNewline() + if needsNewline { + p.printNewline() + } p.printIndent() } if e.CloseParenLoc.Start > expr.Loc.Start { diff --git a/internal/js_printer/js_printer_test.go b/internal/js_printer/js_printer_test.go index 093102320a0..2728b0765a5 100644 --- a/internal/js_printer/js_printer_test.go +++ b/internal/js_printer/js_printer_test.go @@ -319,6 +319,20 @@ func TestNew(t *testing.T) { expectPrintedMinify(t, "new x().y", "new x().y;") expectPrintedMinify(t, "new x() + y", "new x+y;") expectPrintedMinify(t, "new x() ** 2", "new x**2;") + + // Test preservation of Webpack-specific comments + expectPrinted(t, "new Worker(// webpackFoo: 1\n // webpackBar: 2\n 'path');", "new Worker(\n // webpackFoo: 1\n // webpackBar: 2\n \"path\"\n);\n") + expectPrinted(t, "new Worker(/* webpackFoo: 1 */ /* webpackBar: 2 */ 'path');", "new Worker(\n /* webpackFoo: 1 */\n /* webpackBar: 2 */\n \"path\"\n);\n") + expectPrinted(t, "new Worker(\n /* multi\n * line\n * webpackBar: */ 'path');", "new Worker(\n /* multi\n * line\n * webpackBar: */\n \"path\"\n);\n") + expectPrinted(t, "new Worker(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */);", "new Worker(\n /* webpackFoo: 1 */\n /* webpackBar:2 */\n \"path\"\n);\n") + expectPrinted(t, "new Worker(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */ ,);", "new Worker(\n /* webpackFoo: 1 */\n /* webpackBar:2 */\n \"path\"\n);\n") + expectPrinted(t, "new Worker(/* webpackFoo: 1 */ 'path', /* webpackBar:2 */ );", "new Worker(\n /* webpackFoo: 1 */\n /* webpackBar:2 */\n \"path\"\n);\n") + expectPrinted(t, "new Worker(new URL('path', /* webpackFoo: these can go anywhere */ import.meta.url))", + "new Worker(new URL(\n /* webpackFoo: these can go anywhere */\n \"path\",\n import.meta.url\n));\n") + + // Other comments should not be preserved + expectPrinted(t, "new Worker(// comment 1\n // comment 2\n 'path');", "new Worker(\n \"path\"\n);\n") + expectPrinted(t, "new Worker(/* comment 1 */ /* comment 2 */ 'path');", "new Worker(\"path\");\n") } func TestCall(t *testing.T) { @@ -677,7 +691,8 @@ func TestImport(t *testing.T) { expectPrinted(t, "import(/* webpackFoo: 1 */ 'path' /* webpackBar:2 */ ,);", "import(\n /* webpackFoo: 1 */\n /* webpackBar:2 */\n \"path\"\n);\n") expectPrinted(t, "import(/* webpackFoo: 1 */ 'path', /* webpackBar:2 */ );", "import(\n /* webpackFoo: 1 */\n /* webpackBar:2 */\n \"path\"\n);\n") expectPrinted(t, "import(/* webpackFoo: 1 */ 'path', { type: 'module' } /* webpackBar:2 */ );", "import(\n /* webpackFoo: 1 */\n /* webpackBar:2 */\n \"path\",\n { type: \"module\" }\n);\n") - expectPrinted(t, "import(new URL('path', /* webpackFoo: these can go anywhere */ import.meta.url))", "import(\n /* webpackFoo: these can go anywhere */\n new URL(\"path\", import.meta.url)\n);\n") + expectPrinted(t, "import(new URL('path', /* webpackFoo: these can go anywhere */ import.meta.url))", + "import(new URL(\n /* webpackFoo: these can go anywhere */\n \"path\",\n import.meta.url\n));\n") // Other comments should not be preserved expectPrinted(t, "import(// comment 1\n // comment 2\n 'path');", "import(\"path\");\n")