From d8cbaa0497d9f91b1ab07407efcf8050a07bd926 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Tue, 29 Mar 2022 13:18:18 -0400 Subject: [PATCH] fix #2139: dangling else minify label edge case --- CHANGELOG.md | 20 ++++++++++++++++++++ internal/js_parser/js_parser_test.go | 10 ++++++++++ internal/js_printer/js_printer.go | 3 +++ 3 files changed, 33 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ddfbaf25f0..3d5e5511ec5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,26 @@ ## Unreleased +* Fix a minification bug with a double-nested `if` inside a label followed by `else` ([#2139](https://github.com/evanw/esbuild/issues/2139)) + + This fixes a minification bug that affects the edge case where `if` is followed by `else` and the `if` contains a label that contains a nested `if`. Normally esbuild's AST printer automatically wraps the body of a single-statement `if` in braces to avoid the "dangling else" `if`/`else` ambiguity common to C-like languages (where the `else` accidentally becomes associated with the inner `if` instead of the outer `if`). However, I was missing automatic wrapping of label statements, which did not have test coverage because they are a rarely-used feature. This release fixes the bug: + + ```js + // Original code + if (a) + b: { + if (c) break b + } + else if (d) + e() + + // Old output (with --minify) + if(a)e:if(c)break e;else d&&e(); + + // New output (with --minify) + if(a){e:if(c)break e}else d&&e(); + ``` + * Fix edge case regarding `baseUrl` and `paths` in `tsconfig.json` ([#2119](https://github.com/evanw/esbuild/issues/2119)) In `tsconfig.json`, TypeScript forbids non-relative values inside `paths` if `baseUrl` is not present, and esbuild does too. However, TypeScript checked this after the entire `tsconfig.json` hierarchy was parsed while esbuild incorrectly checked this immediately when parsing the file containing the `paths` map. This caused incorrect warnings to be generated for `tsconfig.json` files that specify a `baseUrl` value and that inherit a `paths` value from an `extends` clause. Now esbuild will only check for non-relative `paths` values after the entire hierarchy has been parsed to avoid generating incorrect warnings. diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go index edb4b35166e..9376dab38f5 100644 --- a/internal/js_parser/js_parser_test.go +++ b/internal/js_parser/js_parser_test.go @@ -3192,6 +3192,16 @@ func TestMangleIf(t *testing.T) { expectPrintedMangle(t, "(x ? 1 : y) || foo();", "x || y || foo();\n") } +func TestMangleWrapToAvoidAmbiguousElse(t *testing.T) { + expectPrintedMangle(t, "if (a) { if (b) return c } else return d", "if (a) {\n if (b)\n return c;\n} else\n return d;\n") + expectPrintedMangle(t, "if (a) while (1) { if (b) return c } else return d", "if (a) {\n for (; ; )\n if (b)\n return c;\n} else\n return d;\n") + expectPrintedMangle(t, "if (a) for (;;) { if (b) return c } else return d", "if (a) {\n for (; ; )\n if (b)\n return c;\n} else\n return d;\n") + expectPrintedMangle(t, "if (a) for (x in y) { if (b) return c } else return d", "if (a) {\n for (x in y)\n if (b)\n return c;\n} else\n return d;\n") + expectPrintedMangle(t, "if (a) for (x of y) { if (b) return c } else return d", "if (a) {\n for (x of y)\n if (b)\n return c;\n} else\n return d;\n") + expectPrintedMangle(t, "if (a) with (x) { if (b) return c } else return d", "if (a) {\n with (x)\n if (b)\n return c;\n} else\n return d;\n") + expectPrintedMangle(t, "if (a) x: { if (b) return c } else return d", "if (a) {\n x:\n if (b)\n return c;\n} else\n return d;\n") +} + func TestMangleOptionalChain(t *testing.T) { expectPrintedMangle(t, "let a; return a != null ? a.b : undefined", "let a;\nreturn a?.b;\n") expectPrintedMangle(t, "let a; return a != null ? a[b] : undefined", "let a;\nreturn a?.[b];\n") diff --git a/internal/js_printer/js_printer.go b/internal/js_printer/js_printer.go index 9c0de4a273b..ef90a55b3bd 100644 --- a/internal/js_printer/js_printer.go +++ b/internal/js_printer/js_printer.go @@ -2901,6 +2901,9 @@ func wrapToAvoidAmbiguousElse(s js_ast.S) bool { case *js_ast.SWith: s = current.Body.Data + case *js_ast.SLabel: + s = current.Stmt.Data + default: return false }