From d0b08747d05934f1c7c6f312df26e432024a2526 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Wed, 24 Nov 2021 00:38:02 -0600 Subject: [PATCH] fix #1791: mark certain "Set" and "Map" as pure --- CHANGELOG.md | 20 ++++++++ internal/js_parser/js_parser.go | 68 ++++++++++++++++++++++++++++ internal/js_parser/js_parser_test.go | 28 ++++++++++++ 3 files changed, 116 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e532d27f00..bb8332adc6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -146,6 +146,26 @@ In addition to the breaking changes above, the following changes are also includ .example{--some-var: var(--tw-empty, )}/*! Some legal comment */body{background-color:red} ``` +* Mark `Set` and `Map` with array arguments as pure ([#1791](https://github.com/evanw/esbuild/issues/1791)) + + This release introduces special behavior for references to the global `Set` and `Map` constructors that marks them as `/* @__PURE__ */` if they are known to not have any side effects. These constructors evaluate the iterator of whatever is passed to them and the iterator could have side effects, so this is only safe if whatever is passed to them is an array, since the array iterator has no side effects. + + Marking a constructor call as `/* @__PURE__ */` means it's safe to remove if the result is unused. This is an existing feature that you can trigger by manually adding a `/* @__PURE__ */` comment before a constructor call. The difference is that this release contains special behavior to automatically mark `Set` and `Map` as pure for you as long as it's safe to do so. As with all constructor calls that are marked `/* @__PURE__ */`, any internal expressions which could cause side effects are still preserved even though the constructor call itself is removed: + + ```js + // Original code + new Map([ + ['a', b()], + [c(), new Set(['d', e()])], + ]); + + // Old output (with --minify) + new Map([["a",b()],[c(),new Set(["d",e()])]]); + + // New output (with --minify) + b(),c(),e(); + ``` + ## 0.13.15 * Fix `super` in lowered `async` arrow functions ([#1777](https://github.com/evanw/esbuild/issues/1777)) diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 61ee0c9865e..11a87122c14 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -12718,6 +12718,8 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO e.Args[i] = p.visitExpr(arg) } + p.maybeMarkKnownGlobalConstructorAsPure(e) + case *js_ast.EArrow: asyncArrowNeedsToBeLowered := e.IsAsync && p.options.unsupportedJSFeatures.Has(compat.AsyncAwait) oldFnOrArrowData := p.fnOrArrowDataVisit @@ -12890,6 +12892,72 @@ func (p *parser) warnAboutImportNamespaceCall(target js_ast.Expr, kind importNam } } +func (p *parser) maybeMarkKnownGlobalConstructorAsPure(e *js_ast.ENew) { + if id, ok := e.Target.Data.(*js_ast.EIdentifier); ok { + if symbol := p.symbols[id.Ref.InnerIndex]; symbol.Kind == js_ast.SymbolUnbound { + switch symbol.OriginalName { + case "Set": + n := len(e.Args) + + if n == 0 { + // "new Set()" is pure + e.CanBeUnwrappedIfUnused = true + break + } + + if n == 1 { + switch e.Args[0].Data.(type) { + case *js_ast.EArray, *js_ast.ENull, *js_ast.EUndefined: + // "new Set([a, b, c])" is pure + // "new Set(null)" is pure + // "new Set(void 0)" is pure + e.CanBeUnwrappedIfUnused = true + + default: + // "new Set(x)" is impure because the iterator for "x" could have side effects + } + } + + case "Map": + n := len(e.Args) + + if n == 0 { + // "new Map()" is pure + e.CanBeUnwrappedIfUnused = true + break + } + + if n == 1 { + switch arg := e.Args[0].Data.(type) { + case *js_ast.ENull, *js_ast.EUndefined: + // "new Map(null)" is pure + // "new Map(void 0)" is pure + e.CanBeUnwrappedIfUnused = true + + case *js_ast.EArray: + allEntriesAreArrays := true + for _, item := range arg.Items { + if _, ok := item.Data.(*js_ast.EArray); !ok { + // "new Map([x])" is impure because "x[0]" could have side effects + allEntriesAreArrays = false + break + } + } + + // "new Map([[a, b], [c, d]])" is pure + if allEntriesAreArrays { + e.CanBeUnwrappedIfUnused = true + } + + default: + // "new Map(x)" is impure because the iterator for "x" could have side effects + } + } + } + } + } +} + func (p *parser) valueForDefine(loc logger.Loc, defineFunc config.DefineFunc, opts identifierOpts) js_ast.Expr { expr := js_ast.Expr{Loc: loc, Data: defineFunc(config.DefineArgs{ Loc: loc, diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go index a120c239c85..df4ef5d1f9d 100644 --- a/internal/js_parser/js_parser_test.go +++ b/internal/js_parser/js_parser_test.go @@ -4839,3 +4839,31 @@ func TestMangleCatch(t *testing.T) { expectPrintedMangle(t, "try { throw 1 } catch (x) { eval('x') }", "try {\n throw 1;\n} catch (x) {\n eval(\"x\");\n}\n") expectPrintedMangle(t, "if (y) try { throw 1 } catch (x) {} else eval('x')", "if (y)\n try {\n throw 1;\n } catch {\n }\nelse\n eval(\"x\");\n") } + +func TestAutoPureForSet(t *testing.T) { + expectPrinted(t, "new Set", "/* @__PURE__ */ new Set();\n") + expectPrinted(t, "new Set(null)", "/* @__PURE__ */ new Set(null);\n") + expectPrinted(t, "new Set(undefined)", "/* @__PURE__ */ new Set(void 0);\n") + expectPrinted(t, "new Set([])", "/* @__PURE__ */ new Set([]);\n") + expectPrinted(t, "new Set([x])", "/* @__PURE__ */ new Set([x]);\n") + + expectPrinted(t, "new Set(x)", "new Set(x);\n") + expectPrinted(t, "new Set(false)", "new Set(false);\n") + expectPrinted(t, "new Set({})", "new Set({});\n") + expectPrinted(t, "new Set({ x })", "new Set({ x });\n") +} + +func TestAutoPureForMap(t *testing.T) { + expectPrinted(t, "new Map", "/* @__PURE__ */ new Map();\n") + expectPrinted(t, "new Map(null)", "/* @__PURE__ */ new Map(null);\n") + expectPrinted(t, "new Map(undefined)", "/* @__PURE__ */ new Map(void 0);\n") + expectPrinted(t, "new Map([])", "/* @__PURE__ */ new Map([]);\n") + expectPrinted(t, "new Map([[]])", "/* @__PURE__ */ new Map([[]]);\n") + expectPrinted(t, "new Map([[], []])", "/* @__PURE__ */ new Map([[], []]);\n") + + expectPrinted(t, "new Map(x)", "new Map(x);\n") + expectPrinted(t, "new Map(false)", "new Map(false);\n") + expectPrinted(t, "new Map([x])", "new Map([x]);\n") + expectPrinted(t, "new Map([x, []])", "new Map([x, []]);\n") + expectPrinted(t, "new Map([[], x])", "new Map([[], x]);\n") +}