Skip to content

Commit

Permalink
fix #2439: webpack magic comments in new Worker
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Dec 3, 2022
1 parent fdf184e commit 48a4ae5
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 15 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -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))
Expand Down
8 changes: 6 additions & 2 deletions internal/js_ast/js_ast.go
View file
Expand Up @@ -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

Expand Down
23 changes: 14 additions & 9 deletions internal/js_parser/js_parser.go
View file
Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
19 changes: 16 additions & 3 deletions internal/js_printer/js_printer.go
View file
Expand Up @@ -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 {
Expand Down
17 changes: 16 additions & 1 deletion internal/js_printer/js_printer_test.go
View file
Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit 48a4ae5

Please sign in to comment.