Skip to content

Commit

Permalink
fix #2139: dangling else minify label edge case
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Mar 29, 2022
1 parent 2244c09 commit d8cbaa0
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 0 deletions.
20 changes: 20 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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.
Expand Down
10 changes: 10 additions & 0 deletions internal/js_parser/js_parser_test.go
Expand Up @@ -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")
Expand Down
3 changes: 3 additions & 0 deletions internal/js_printer/js_printer.go
Expand Up @@ -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
}
Expand Down

0 comments on commit d8cbaa0

Please sign in to comment.