diff --git a/CHANGELOG.md b/CHANGELOG.md index b50c40c86fb..689e00b55d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,37 @@ # Changelog +## Unreleased + +* Special-case `const` inlining at the top of a scope ([#1317](https://github.com/evanw/esbuild/issues/1317), [#1981](https://github.com/evanw/esbuild/issues/1981)) + + The minifier now inlines `const` variables (even across modules during bundling) if a certain set of specific requirements are met: + + * All `const` variables to be inlined are at the top of their scope + * That scope doesn't contain any `import` or `export` statements with paths + * All constants to be inlined are `null`, `undefined`, `true`, `false`, an integer, or a short real number + * Any expression outside of a small list of allowed ones stops constant identification + + Practically speaking this basically means that you can trigger this optimization by just putting the constants you want inlined into a separate file (e.g. `constants.js`) and bundling everything together. + + These specific conditions are present to avoid esbuild unintentionally causing any behavior changes by inlining constants when the variable reference could potentially be evaluated before being declared. It's possible to identify more cases where constants can be inlined but doing so may require complex call graph analysis so it has not been implemented. Although these specific heuristics may change over time, this general approach to constant inlining should continue to work going forward. + + Here's an example: + + ```js + // Original code + const bold = 1 << 0; + const italic = 2 << 1; + const underline = 3 << 2; + const font = bold | italic | underline; + console.log(font); + + // Old output (with --minify --bundle) + (()=>{var o=1<<0,n=2<<1,c=3<<2,t=o|n|c;console.log(t);})(); + + // New output (with --minify --bundle) + (()=>{console.log(13);})(); + ``` + ## 0.14.18 * Add the `--mangle-cache=` feature ([#1977](https://github.com/evanw/esbuild/issues/1977)) diff --git a/internal/bundler/bundler_dce_test.go b/internal/bundler/bundler_dce_test.go index 5598ddd2c39..d514fd89ba4 100644 --- a/internal/bundler/bundler_dce_test.go +++ b/internal/bundler/bundler_dce_test.go @@ -1,6 +1,7 @@ package bundler import ( + "regexp" "testing" "github.com/evanw/esbuild/internal/compat" @@ -2553,3 +2554,309 @@ func TestInlineFunctionCallForInitDecl(t *testing.T) { }, }) } + +func TestConstValueInliningNoBundle(t *testing.T) { + dce_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/top-level.js": ` + // These should be kept because they are top-level and tree shaking is not enabled + const n_keep = null + const u_keep = undefined + const i_keep = 1234567 + const f_keep = 123.456 + const s_keep = '' + + // Values should still be inlined + console.log( + // These are doubled to avoid the "inline const/let into next statement if used once" optimization + n_keep, n_keep, + u_keep, u_keep, + i_keep, i_keep, + f_keep, f_keep, + s_keep, s_keep, + ) + `, + "/nested-block.js": ` + { + const REMOVE_n = null + const REMOVE_u = undefined + const REMOVE_i = 1234567 + const REMOVE_f = 123.456 + const s_keep = '' // String inlining is intentionally not supported right now + console.log( + // These are doubled to avoid the "inline const/let into next statement if used once" optimization + REMOVE_n, REMOVE_n, + REMOVE_u, REMOVE_u, + REMOVE_i, REMOVE_i, + REMOVE_f, REMOVE_f, + s_keep, s_keep, + ) + } + `, + "/nested-function.js": ` + function nested() { + const REMOVE_n = null + const REMOVE_u = undefined + const REMOVE_i = 1234567 + const REMOVE_f = 123.456 + const s_keep = '' // String inlining is intentionally not supported right now + console.log( + // These are doubled to avoid the "inline const/let into next statement if used once" optimization + REMOVE_n, REMOVE_n, + REMOVE_u, REMOVE_u, + REMOVE_i, REMOVE_i, + REMOVE_f, REMOVE_f, + s_keep, s_keep, + ) + } + `, + "/namespace-export.ts": ` + namespace ns { + const x_REMOVE = 1 + export const y_keep = 2 + console.log( + x_REMOVE, x_REMOVE, + y_keep, y_keep, + ) + } + `, + + "/comment-before.js": ` + { + //! comment + const REMOVE = 1 + x = [REMOVE, REMOVE] + } + `, + "/directive-before.js": ` + function nested() { + 'directive' + const REMOVE = 1 + x = [REMOVE, REMOVE] + } + `, + "/semicolon-before.js": ` + { + ; + const REMOVE = 1 + x = [REMOVE, REMOVE] + } + `, + "/debugger-before.js": ` + { + debugger + const REMOVE = 1 + x = [REMOVE, REMOVE] + } + `, + "/type-before.ts": ` + { + declare let x + const REMOVE = 1 + x = [REMOVE, REMOVE] + } + `, + "/exprs-before.js": ` + function nested() { + const x = [, '', {}, 0n, /./, function() {}, () => {}] + const y_REMOVE = 1 + function foo() { + return y_REMOVE + } + } + `, + + "/disabled-tdz.js": ` + foo() + const x_keep = 1 + function foo() { + return x_keep + } + `, + "/backwards-reference-top-level.js": ` + const x = y + const y = 1 + console.log( + x, x, + y, y, + ) + `, + "/backwards-reference-nested-function.js": ` + function foo() { + const x = y + const y = 1 + console.log( + x, x, + y, y, + ) + } + `, + }, + entryPaths: []string{ + "/top-level.js", + "/nested-block.js", + "/nested-function.js", + "/namespace-export.ts", + + "/comment-before.js", + "/directive-before.js", + "/semicolon-before.js", + "/debugger-before.js", + "/type-before.ts", + "/exprs-before.js", + + "/disabled-tdz.js", + "/backwards-reference-top-level.js", + "/backwards-reference-nested-function.js", + }, + options: config.Options{ + Mode: config.ModePassThrough, + AbsOutputDir: "/out", + MinifySyntax: true, + }, + }) +} + +func TestConstValueInliningBundle(t *testing.T) { + dce_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/exported-entry.js": ` + const x_REMOVE = 1 + export const y_keep = 2 + console.log( + x_REMOVE, + y_keep, + ) + `, + + "/re-exported-entry.js": ` + import { x_REMOVE, y_keep } from './re-exported-constants' + console.log(x_REMOVE, y_keep) + export { y_keep } + `, + "/re-exported-constants.js": ` + export const x_REMOVE = 1 + export const y_keep = 2 + `, + + "/re-exported-2-entry.js": ` + export { y_keep } from './re-exported-2-constants' + `, + "/re-exported-2-constants.js": ` + export const x_REMOVE = 1 + export const y_keep = 2 + `, + + "/re-exported-star-entry.js": ` + export * from './re-exported-star-constants' + `, + "/re-exported-star-constants.js": ` + export const x_keep = 1 + export const y_keep = 2 + `, + + "/cross-module-entry.js": ` + import { x_REMOVE, y_keep } from './cross-module-constants' + console.log(x_REMOVE, y_keep) + `, + "/cross-module-constants.js": ` + export const x_REMOVE = 1 + foo() + export const y_keep = 1 + export function foo() { + return [x_REMOVE, y_keep] + } + `, + + "/print-shorthand-entry.js": ` + import { foo, _bar } from './print-shorthand-constants' + // The inlined constants must still be present in the output! We don't + // want the printer to use the shorthand syntax here to refer to the + // name of the constant itself because the constant declaration is omitted. + console.log({ foo, _bar }) + `, + "/print-shorthand-constants.js": ` + export const foo = 123 + export const _bar = -321 + `, + + "/circular-import-entry.js": ` + import './circular-import-constants' + `, + "/circular-import-constants.js": ` + export const foo = 123 // Inlining should be prevented by the cycle + export function bar() { + return foo + } + import './circular-import-cycle' + `, + "/circular-import-cycle.js": ` + import { bar } from './circular-import-constants' + console.log(bar()) // This accesses "foo" before it's initialized + `, + + "/circular-re-export-entry.js": ` + import { baz } from './circular-re-export-constants' + console.log(baz) + `, + "/circular-re-export-constants.js": ` + export const foo = 123 // Inlining should be prevented by the cycle + export function bar() { + return foo + } + export { baz } from './circular-re-export-cycle' + `, + "/circular-re-export-cycle.js": ` + export const baz = 0 + import { bar } from './circular-re-export-constants' + console.log(bar()) // This accesses "foo" before it's initialized + `, + + "/circular-re-export-star-entry.js": ` + import './circular-re-export-star-constants' + `, + "/circular-re-export-star-constants.js": ` + export const foo = 123 // Inlining should be prevented by the cycle + export function bar() { + return foo + } + export * from './circular-re-export-star-cycle' + `, + "/circular-re-export-star-cycle.js": ` + import { bar } from './circular-re-export-star-constants' + console.log(bar()) // This accesses "foo" before it's initialized + `, + + "/non-circular-export-entry.js": ` + import { foo, bar } from './non-circular-export-constants' + console.log(foo, bar()) + `, + "/non-circular-export-constants.js": ` + const foo = 123 // Inlining should be prevented by the cycle + function bar() { + return foo + } + export { foo, bar } + `, + }, + entryPaths: []string{ + "/exported-entry.js", + "/re-exported-entry.js", + "/re-exported-2-entry.js", + "/re-exported-star-entry.js", + "/cross-module-entry.js", + "/print-shorthand-entry.js", + "/circular-import-entry.js", + "/circular-re-export-entry.js", + "/circular-re-export-star-entry.js", + "/non-circular-export-entry.js", + }, + options: config.Options{ + Mode: config.ModeBundle, + OutputFormat: config.FormatESModule, + AbsOutputDir: "/out", + MinifySyntax: true, + MangleProps: regexp.MustCompile("^_"), + }, + }) +} diff --git a/internal/bundler/bundler_default_test.go b/internal/bundler/bundler_default_test.go index 8da5b24abf2..17a61082150 100644 --- a/internal/bundler/bundler_default_test.go +++ b/internal/bundler/bundler_default_test.go @@ -4464,7 +4464,7 @@ func TestConstWithLet(t *testing.T) { "/entry.js": ` const a = 1; console.log(a) if (true) { const b = 2; console.log(b) } - if (true) { const b = 2; unknownFn(b) } + if (true) { const b = 3; unknownFn(b) } for (const c = x;;) console.log(c) for (const d in x) console.log(d) for (const e of x) console.log(e) @@ -4485,7 +4485,7 @@ func TestConstWithLetNoBundle(t *testing.T) { "/entry.js": ` const a = 1; console.log(a) if (true) { const b = 2; console.log(b) } - if (true) { const b = 2; unknownFn(b) } + if (true) { const b = 3; unknownFn(b) } for (const c = x;;) console.log(c) for (const d in x) console.log(d) for (const e of x) console.log(e) diff --git a/internal/bundler/linker.go b/internal/bundler/linker.go index a09f7bf2659..4137b06cb53 100644 --- a/internal/bundler/linker.go +++ b/internal/bundler/linker.go @@ -1449,6 +1449,7 @@ func (c *linkerContext) scanImportsAndExports() { localDependencies := make(map[uint32]uint32) parts := repr.AST.Parts namedImports := repr.AST.NamedImports + graph := c.graph for partIndex := range parts { part := &parts[partIndex] @@ -1460,8 +1461,8 @@ func (c *linkerContext) scanImportsAndExports() { // Rare path: this import is a TypeScript enum if importData, ok := repr.Meta.ImportsToBind[ref]; ok { - if symbol := c.graph.Symbols.Get(importData.Ref); symbol.Kind == js_ast.SymbolTSEnum { - if enum, ok := c.graph.TSEnums[importData.Ref]; ok { + if symbol := graph.Symbols.Get(importData.Ref); symbol.Kind == js_ast.SymbolTSEnum { + if enum, ok := graph.TSEnums[importData.Ref]; ok { foundNonInlinedEnum := false for name, propertyUse := range properties { if _, ok := enum[name]; !ok { @@ -1491,10 +1492,10 @@ func (c *linkerContext) scanImportsAndExports() { use := part.SymbolUses[ref] // Find the symbol that was called - symbol := c.graph.Symbols.Get(ref) + symbol := graph.Symbols.Get(ref) if symbol.Kind == js_ast.SymbolImport { if importData, ok := repr.Meta.ImportsToBind[ref]; ok { - symbol = c.graph.Symbols.Get(importData.Ref) + symbol = graph.Symbols.Get(importData.Ref) } } flags := symbol.Flags @@ -1518,6 +1519,17 @@ func (c *linkerContext) scanImportsAndExports() { // Now that we know this, we can determine cross-part dependencies for ref := range part.SymbolUses { + + // Rare path: this import is an inlined const value + if graph.ConstValues != nil { + if importData, ok := repr.Meta.ImportsToBind[ref]; ok { + if _, isConstValue := graph.ConstValues[importData.Ref]; isConstValue { + delete(part.SymbolUses, importData.Ref) + continue + } + } + } + for _, otherPartIndex := range repr.TopLevelSymbolToParts(ref) { if oldPartIndex, ok := localDependencies[otherPartIndex]; !ok || oldPartIndex != uint32(partIndex) { localDependencies[otherPartIndex] = uint32(partIndex) @@ -3994,6 +4006,7 @@ func (c *linkerContext) generateCodeForFileInChunkJS( ToESMRef: toESMRef, RuntimeRequireRef: runtimeRequireRef, TSEnums: c.graph.TSEnums, + ConstValues: c.graph.ConstValues, LegalComments: c.options.LegalComments, UnsupportedFeatures: c.options.UnsupportedJSFeatures, AddSourceMappings: addSourceMappings, diff --git a/internal/bundler/snapshots/snapshots_dce.txt b/internal/bundler/snapshots/snapshots_dce.txt index 4d5dfe75fcc..cc6ecc3d091 100644 --- a/internal/bundler/snapshots/snapshots_dce.txt +++ b/internal/bundler/snapshots/snapshots_dce.txt @@ -3,6 +3,171 @@ TestBase64LoaderRemoveUnused // entry.js console.log("unused import"); +================================================================================ +TestConstValueInliningBundle +---------- /out/exported-entry.js ---------- +// exported-entry.js +var y_keep = 2; +console.log(1, 2); +export { + y_keep +}; + +---------- /out/re-exported-entry.js ---------- +// re-exported-constants.js +var y_keep = 2; + +// re-exported-entry.js +console.log(1, 2); +export { + y_keep +}; + +---------- /out/re-exported-2-entry.js ---------- +// re-exported-2-constants.js +var y_keep = 2; +export { + y_keep +}; + +---------- /out/re-exported-star-entry.js ---------- +// re-exported-star-constants.js +var x_keep = 1, y_keep = 2; +export { + x_keep, + y_keep +}; + +---------- /out/cross-module-entry.js ---------- +// cross-module-constants.js +foo(); +var y_keep = 1; +function foo() { + return [1, y_keep]; +} + +// cross-module-entry.js +console.log(1, y_keep); + +---------- /out/print-shorthand-entry.js ---------- +// print-shorthand-entry.js +console.log({ foo: 123, a: -321 }); + +---------- /out/circular-import-entry.js ---------- +// circular-import-cycle.js +console.log(bar()); + +// circular-import-constants.js +var foo = 123; +function bar() { + return foo; +} + +---------- /out/circular-re-export-entry.js ---------- +// circular-re-export-cycle.js +var baz = 0; +console.log(bar()); + +// circular-re-export-constants.js +var foo = 123; +function bar() { + return foo; +} + +// circular-re-export-entry.js +console.log(baz); + +---------- /out/circular-re-export-star-entry.js ---------- +// circular-re-export-star-cycle.js +console.log(bar()); + +// circular-re-export-star-constants.js +var foo = 123; +function bar() { + return foo; +} + +---------- /out/non-circular-export-entry.js ---------- +// non-circular-export-constants.js +function bar() { + return 123; +} + +// non-circular-export-entry.js +console.log(123, bar()); + +================================================================================ +TestConstValueInliningNoBundle +---------- /out/top-level.js ---------- +const n_keep = null, u_keep = void 0, i_keep = 1234567, f_keep = 123.456, s_keep = ""; +console.log(null, null, void 0, void 0, 1234567, 1234567, 123.456, 123.456, s_keep, s_keep); + +---------- /out/nested-block.js ---------- +{ + const s_keep = ""; + console.log(null, null, void 0, void 0, 1234567, 1234567, 123.456, 123.456, s_keep, s_keep); +} + +---------- /out/nested-function.js ---------- +function nested() { + const s_keep = ""; + console.log(null, null, void 0, void 0, 1234567, 1234567, 123.456, 123.456, s_keep, s_keep); +} + +---------- /out/namespace-export.js ---------- +var ns; +((ns2) => (ns2.y_keep = 2, console.log(1, 1, 2, 2)))(ns ||= {}); + +---------- /out/comment-before.js ---------- +{ + //! comment + x = [1, 1]; +} + +---------- /out/directive-before.js ---------- +function nested() { + x = [1, 1]; +} + +---------- /out/semicolon-before.js ---------- +x = [1, 1]; + +---------- /out/debugger-before.js ---------- +{ + debugger; + x = [1, 1]; +} + +---------- /out/type-before.js ---------- +x = [1, 1]; + +---------- /out/exprs-before.js ---------- +function nested() { + const x = [, "", {}, 0n, /./, function() { + }, () => { + }]; + function foo() { + return 1; + } +} + +---------- /out/disabled-tdz.js ---------- +foo(); +const x_keep = 1; +function foo() { + return x_keep; +} + +---------- /out/backwards-reference-top-level.js ---------- +const x = y, y = 1; +console.log(x, x, y, y); + +---------- /out/backwards-reference-nested-function.js ---------- +function foo() { + const x = y, y = 1; + console.log(x, x, y, y); +} + ================================================================================ TestDCEClassStaticBlocks ---------- /out.js ---------- diff --git a/internal/bundler/snapshots/snapshots_default.txt b/internal/bundler/snapshots/snapshots_default.txt index bda2997d2f6..5ed6ae7bbb9 100644 --- a/internal/bundler/snapshots/snapshots_default.txt +++ b/internal/bundler/snapshots/snapshots_default.txt @@ -356,13 +356,9 @@ x ? y ? require.resolve("a") : require.resolve("b") : require.resolve(c); TestConstWithLet ---------- /out.js ---------- // entry.js -var a = 1; -console.log(a); +console.log(1); console.log(2); -{ - let b = 2; - unknownFn(b); -} +unknownFn(3); for (let c = x; ; ) console.log(c); for (let d in x) @@ -374,11 +370,7 @@ for (let e of x) TestConstWithLetNoBundle ---------- /out.js ---------- const a = 1; -console.log(a), console.log(2); -{ - const b = 2; - unknownFn(b); -} +console.log(1), console.log(2), unknownFn(3); for (const c = x; ; ) console.log(c); for (const d in x) diff --git a/internal/graph/graph.go b/internal/graph/graph.go index 2a32d41967e..371de7110a3 100644 --- a/internal/graph/graph.go +++ b/internal/graph/graph.go @@ -105,6 +105,9 @@ type LinkerGraph struct { // This is for cross-module inlining of TypeScript enum constants TSEnums map[js_ast.Ref]map[string]js_ast.TSEnumValue + // This is for cross-module inlining of detected inlinable constants + ConstValues map[js_ast.Ref]js_ast.ConstValue + // We should avoid traversing all files in the bundle, because the linker // should be able to run a linking operation on a large bundle where only // a few files are needed (e.g. an incremental compilation scenario). This @@ -256,6 +259,7 @@ func CloneLinkerGraph( // Do a final quick pass over all files var tsEnums map[js_ast.Ref]map[string]js_ast.TSEnumValue + var constValues map[js_ast.Ref]js_ast.ConstValue bitCount := uint(len(entryPoints)) for _, sourceIndex := range reachableFiles { file := &files[sourceIndex] @@ -274,11 +278,22 @@ func CloneLinkerGraph( tsEnums[ref] = enum } } + + // Also merge const values into one big map as well + if repr, ok := file.InputFile.Repr.(*JSRepr); ok && repr.AST.ConstValues != nil { + if constValues == nil { + constValues = make(map[js_ast.Ref]js_ast.ConstValue) + } + for ref, value := range repr.AST.ConstValues { + constValues[ref] = value + } + } } return LinkerGraph{ Symbols: symbols, TSEnums: tsEnums, + ConstValues: constValues, entryPoints: entryPoints, Files: files, ReachableFiles: reachableFiles, diff --git a/internal/js_ast/js_ast.go b/internal/js_ast/js_ast.go index e274d0fde0d..4df1ead3f25 100644 --- a/internal/js_ast/js_ast.go +++ b/internal/js_ast/js_ast.go @@ -3,6 +3,7 @@ package js_ast import ( "math" "sort" + "strconv" "github.com/evanw/esbuild/internal/ast" "github.com/evanw/esbuild/internal/compat" @@ -1786,6 +1787,11 @@ type Scope struct { // This is to help forbid "arguments" inside class body scopes ForbidArguments bool + // As a special case, we enable constant propagation for any chain of "const" + // declarations at the start of a statement list. This special case doesn't + // have any TDZ considerations because no other statements come before it. + IsAfterConstLocalPrefix bool + StrictMode StrictModeKind Kind ScopeKind } @@ -2057,6 +2063,10 @@ type AST struct { // to enable cross-module inlining of constant enums. TSEnums map[Ref]map[string]TSEnumValue + // This contains the values of all detected inlinable constants. It exists + // to enable cross-module inlining of these constants. + ConstValues map[Ref]ConstValue + // Properties in here are represented as symbols instead of strings, which // allows them to be renamed to smaller names. MangledProps map[string]Ref @@ -2105,6 +2115,78 @@ type TSEnumValue struct { Number float64 // Use this if "String" is nil } +type ConstValueKind uint8 + +const ( + ConstValueNone ConstValueKind = iota + ConstValueNull + ConstValueUndefined + ConstValueTrue + ConstValueFalse + ConstValueNumber +) + +type ConstValue struct { + Number float64 // Use this for "ConstValueNumber" + Kind ConstValueKind +} + +func ExprToConstValue(expr Expr) ConstValue { + switch v := expr.Data.(type) { + case *ENull: + return ConstValue{Kind: ConstValueNull} + + case *EUndefined: + return ConstValue{Kind: ConstValueUndefined} + + case *EBoolean: + if v.Value { + return ConstValue{Kind: ConstValueTrue} + } else { + return ConstValue{Kind: ConstValueFalse} + } + + case *ENumber: + // Inline integers and other small numbers. Don't inline large + // real numbers because people may not want them to be inlined + // as it will increase the minified code size by too much. + if asInt := int64(v.Value); v.Value == float64(asInt) || len(strconv.FormatFloat(v.Value, 'g', -1, 64)) <= 8 { + return ConstValue{Kind: ConstValueNumber, Number: v.Value} + } + + case *EString: + // I'm deliberately not inlining strings here. It seems more likely that + // people won't want them to be inlined since they can be arbitrarily long. + + case *EBigInt: + // I'm deliberately not inlining bigints here for the same reason (they can + // be arbitrarily long). + } + + return ConstValue{} +} + +func ConstValueToExpr(loc logger.Loc, value ConstValue) Expr { + switch value.Kind { + case ConstValueNull: + return Expr{Loc: loc, Data: ENullShared} + + case ConstValueUndefined: + return Expr{Loc: loc, Data: EUndefinedShared} + + case ConstValueTrue: + return Expr{Loc: loc, Data: &EBoolean{Value: true}} + + case ConstValueFalse: + return Expr{Loc: loc, Data: &EBoolean{Value: false}} + + case ConstValueNumber: + return Expr{Loc: loc, Data: &ENumber{Value: value.Number}} + } + + panic("Internal error: invalid constant value") +} + // This is a histogram of character frequencies for minification type CharFreq [64]int32 diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 03ffaac37b7..eaf40890005 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -91,6 +91,7 @@ type parser struct { isExportedInsideNamespace map[js_ast.Ref]js_ast.Ref localTypeNames map[string]bool tsEnums map[js_ast.Ref]map[string]js_ast.TSEnumValue + constValues map[js_ast.Ref]js_ast.ConstValue // This is the reference to the generated function argument for the namespace, // which is different than the reference to the namespace itself: @@ -6166,6 +6167,10 @@ func (p *parser) parseStmt(opts parseStmtOpts) js_ast.Stmt { } importRecordIndex := p.addImportRecord(ast.ImportStmt, pathLoc, pathText, assertions) + // Export-star statements anywhere in the file disable top-level const + // local prefix because import cycles can be used to trigger TDZ + p.currentScope.IsAfterConstLocalPrefix = true + p.lexer.ExpectOrInsertSemicolon() return js_ast.Stmt{Loc: loc, Data: &js_ast.SExportStar{ NamespaceRef: namespaceRef, @@ -6186,6 +6191,11 @@ func (p *parser) parseStmt(opts parseStmtOpts) js_ast.Stmt { importRecordIndex := p.addImportRecord(ast.ImportStmt, pathLoc, pathText, assertions) name := "import_" + js_ast.GenerateNonUniqueNameFromPath(pathText) namespaceRef := p.storeNameInRef(name) + + // Export clause statements anywhere in the file disable top-level const + // local prefix because import cycles can be used to trigger TDZ + p.currentScope.IsAfterConstLocalPrefix = true + p.lexer.ExpectOrInsertSemicolon() return js_ast.Stmt{Loc: loc, Data: &js_ast.SExportFrom{ Items: items, @@ -6194,6 +6204,7 @@ func (p *parser) parseStmt(opts parseStmtOpts) js_ast.Stmt { IsSingleLine: isSingleLine, }} } + p.lexer.ExpectOrInsertSemicolon() return js_ast.Stmt{Loc: loc, Data: &js_ast.SExportClause{Items: items, IsSingleLine: isSingleLine}} @@ -6805,6 +6816,9 @@ func (p *parser) parseStmt(opts parseStmtOpts) js_ast.Stmt { // Track the items for this namespace p.importItemsForNamespace[stmt.NamespaceRef] = itemRefs + // Import statements anywhere in the file disable top-level const + // local prefix because import cycles can be used to trigger TDZ + p.currentScope.IsAfterConstLocalPrefix = true return js_ast.Stmt{Loc: loc, Data: &stmt} case js_lexer.TBreak: @@ -8966,20 +8980,34 @@ func (p *parser) keepStmtSymbolName(loc logger.Loc, ref js_ast.Ref, name string) } func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ast.Stmt { + // By default any statement ends the const local prefix + wasAfterAfterConstLocalPrefix := p.currentScope.IsAfterConstLocalPrefix + p.currentScope.IsAfterConstLocalPrefix = true + switch s := stmt.Data.(type) { case *js_ast.SEmpty, *js_ast.SComment: - // These don't contain anything to traverse + // Comments do not end the const local prefix + p.currentScope.IsAfterConstLocalPrefix = wasAfterAfterConstLocalPrefix case *js_ast.SDebugger: + // Debugger statements do not end the const local prefix + p.currentScope.IsAfterConstLocalPrefix = wasAfterAfterConstLocalPrefix + if p.options.dropDebugger { return stmts } case *js_ast.STypeScript: + // Type annotations do not end the const local prefix + p.currentScope.IsAfterConstLocalPrefix = wasAfterAfterConstLocalPrefix + // Erase TypeScript constructs from the output completely return stmts case *js_ast.SDirective: + // Directives do not end the const local prefix + p.currentScope.IsAfterConstLocalPrefix = wasAfterAfterConstLocalPrefix + if p.isStrictMode() && s.LegacyOctalLoc.Start > 0 { p.markStrictModeFeature(legacyOctalEscape, p.source.RangeOfLegacyOctalEscape(s.LegacyOctalLoc), "") } @@ -9204,13 +9232,26 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ p.popScope() case *js_ast.SLocal: + // Local statements do not end the const local prefix + p.currentScope.IsAfterConstLocalPrefix = wasAfterAfterConstLocalPrefix + + end := 0 for i := range s.Decls { - d := &s.Decls[i] + d := s.Decls[i] p.visitBinding(d.Binding, bindingOpts{}) + + // Visit the initializer if d.ValueOrNil.Data != nil { wasAnonymousNamedExpr := p.isAnonymousNamedExpr(d.ValueOrNil) + + // Fold numeric constants in the initializer + oldShouldFoldNumericConstants := p.shouldFoldNumericConstants + p.shouldFoldNumericConstants = p.options.minifySyntax && !p.currentScope.IsAfterConstLocalPrefix + d.ValueOrNil = p.visitExpr(d.ValueOrNil) + p.shouldFoldNumericConstants = oldShouldFoldNumericConstants + // Optionally preserve the name if id, ok := d.Binding.Data.(*js_ast.BIdentifier); ok { d.ValueOrNil = p.maybeKeepExprSymbolName( @@ -9238,8 +9279,49 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ } } } + + // Attempt to continue the const local prefix + if p.options.minifySyntax && !p.currentScope.IsAfterConstLocalPrefix { + if id, ok := d.Binding.Data.(*js_ast.BIdentifier); ok { + if s.Kind == js_ast.LocalConst && d.ValueOrNil.Data != nil { + if value := js_ast.ExprToConstValue(d.ValueOrNil); value.Kind != js_ast.ConstValueNone { + if p.constValues == nil { + p.constValues = make(map[js_ast.Ref]js_ast.ConstValue) + } + p.constValues[id.Ref] = value + + // Only keep this declaration if it's top-level or exported (which + // could be in a nested TypeScript namespace), otherwise erase it + if p.currentScope.Parent == nil || s.IsExport { + s.Decls[end] = d + end++ + } + continue + } + } + + if d.ValueOrNil.Data != nil && !isSafeForConstLocalPrefix(d.ValueOrNil) { + p.currentScope.IsAfterConstLocalPrefix = true + } + } else { + // A non-identifier binding ends the const local prefix + p.currentScope.IsAfterConstLocalPrefix = true + } + } + + // Keep the declaration if we didn't continue the loop above + s.Decls[end] = d + end++ + } + + // Remove this declaration entirely if all symbols are inlined constants + if end == 0 { + return stmts } + // Trim the removed declarations + s.Decls = s.Decls[:end] + // Handle being exported inside a namespace if s.IsExport && p.enclosingNamespaceArgRef != nil { wrapIdentifier := func(loc logger.Loc, ref js_ast.Ref) js_ast.Expr { @@ -9845,6 +9927,47 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ return stmts } +// If we encounter a variable initializer that could possibly trigger access to +// a constant declared later on, then we need to end the const local prefix. +// We want to avoid situations like this: +// +// const x = y; // This is supposed to throw due to TDZ +// const y = 1; +// +// or this: +// +// const x = 1; +// const y = foo(); // This is supposed to throw due to TDZ +// const z = 2; +// const foo = () => z; +// +// But a situation like this is ok: +// +// const x = 1; +// const y = [() => z]; +// const z = 2; +// +func isSafeForConstLocalPrefix(expr js_ast.Expr) bool { + switch e := expr.Data.(type) { + case *js_ast.EMissing, *js_ast.EString, *js_ast.ERegExp, *js_ast.EBigInt, *js_ast.EFunction, *js_ast.EArrow: + return true + + case *js_ast.EArray: + for _, item := range e.Items { + if !isSafeForConstLocalPrefix(item) { + return false + } + } + return true + + case *js_ast.EObject: + // For now just allow "{}" and forbid everything else + return len(e.Properties) == 0 + } + + return false +} + type relocateVarsMode uint8 const ( @@ -11443,8 +11566,10 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO fmt.Sprintf("The symbol %q was declared a constant here:", name))} // Make this an error when bundling because we may need to convert this - // "const" into a "var" during bundling. - if p.options.mode == config.ModeBundle { + // "const" into a "var" during bundling. Also make this an error when + // the constant is inlined because we will otherwise generate code with + // a syntax error. + if _, isInlinedConstant := p.constValues[result.ref]; isInlinedConstant || p.options.mode == config.ModeBundle { p.log.AddWithNotes(logger.Error, &p.tracker, r, fmt.Sprintf("Cannot assign to %q because it is a constant", name), notes) } else { @@ -11992,21 +12117,24 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO } case js_ast.BinOpBitwiseAnd: - if p.shouldFoldNumericConstants { + // Minification folds bitwise operations since they are unlikely to result in larger output + if p.shouldFoldNumericConstants || p.options.minifySyntax { if left, right, ok := extractNumericValues(e.Left, e.Right); ok { return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ENumber{Value: float64(toInt32(left) & toInt32(right))}}, exprOut{} } } case js_ast.BinOpBitwiseOr: - if p.shouldFoldNumericConstants { + // Minification folds bitwise operations since they are unlikely to result in larger output + if p.shouldFoldNumericConstants || p.options.minifySyntax { if left, right, ok := extractNumericValues(e.Left, e.Right); ok { return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ENumber{Value: float64(toInt32(left) | toInt32(right))}}, exprOut{} } } case js_ast.BinOpBitwiseXor: - if p.shouldFoldNumericConstants { + // Minification folds bitwise operations since they are unlikely to result in larger output + if p.shouldFoldNumericConstants || p.options.minifySyntax { if left, right, ok := extractNumericValues(e.Left, e.Right); ok { return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ENumber{Value: float64(toInt32(left) ^ toInt32(right))}}, exprOut{} } @@ -13636,6 +13764,14 @@ type identifierOpts struct { func (p *parser) handleIdentifier(loc logger.Loc, e *js_ast.EIdentifier, opts identifierOpts) js_ast.Expr { ref := e.Ref + // Substitute inlined constants + if p.options.minifySyntax { + if value, ok := p.constValues[ref]; ok { + p.ignoreUsage(ref) + return js_ast.ConstValueToExpr(loc, value) + } + } + // Capture the "arguments" variable if necessary if p.fnOnlyDataVisit.argumentsRef != nil && ref == *p.fnOnlyDataVisit.argumentsRef { isInsideUnsupportedArrow := p.fnOrArrowDataVisit.isArrow && p.options.unsupportedJSFeatures.Has(compat.Arrow) @@ -15402,6 +15538,7 @@ func (p *parser) toAST(parts []js_ast.Part, hashbang string, directive string) j NamedImports: p.namedImports, NamedExports: p.namedExports, TSEnums: p.tsEnums, + ConstValues: p.constValues, NestedScopeSlotCounts: nestedScopeSlotCounts, TopLevelSymbolToPartsFromParser: p.topLevelSymbolToParts, ExportStarImportRecords: p.exportStarImportRecords, diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go index 9a90d8105f9..9c8195e9d73 100644 --- a/internal/js_parser/js_parser_test.go +++ b/internal/js_parser/js_parser_test.go @@ -2886,7 +2886,7 @@ func TestMangleIndex(t *testing.T) { func TestMangleBlock(t *testing.T) { expectPrintedMangle(t, "while(1) { while (1) {} }", "for (; ; )\n for (; ; )\n ;\n") - expectPrintedMangle(t, "while(1) { const x = 0; }", "for (; ; ) {\n const x = 0;\n}\n") + expectPrintedMangle(t, "while(1) { const x = y; }", "for (; ; ) {\n const x = y;\n}\n") expectPrintedMangle(t, "while(1) { let x; }", "for (; ; ) {\n let x;\n}\n") expectPrintedMangle(t, "while(1) { var x; }", "for (; ; )\n var x;\n") expectPrintedMangle(t, "while(1) { class X {} }", "for (; ; ) {\n class X {\n }\n}\n") @@ -3645,6 +3645,39 @@ func TestMangleBinaryInsideComma(t *testing.T) { expectPrintedMangle(t, "a + (b, c)", "a + (b, c);\n") } +func TestMangleBinaryConstantFolding(t *testing.T) { + expectPrintedMangle(t, "x = 3 + 6", "x = 3 + 6;\n") + expectPrintedMangle(t, "x = 3 - 6", "x = 3 - 6;\n") + expectPrintedMangle(t, "x = 3 * 6", "x = 3 * 6;\n") + expectPrintedMangle(t, "x = 3 / 6", "x = 3 / 6;\n") + expectPrintedMangle(t, "x = 3 % 6", "x = 3 % 6;\n") + expectPrintedMangle(t, "x = 3 ** 6", "x = 3 ** 6;\n") + + expectPrintedMangle(t, "x = 3 < 6", "x = 3 < 6;\n") + expectPrintedMangle(t, "x = 3 > 6", "x = 3 > 6;\n") + expectPrintedMangle(t, "x = 3 <= 6", "x = 3 <= 6;\n") + expectPrintedMangle(t, "x = 3 >= 6", "x = 3 >= 6;\n") + expectPrintedMangle(t, "x = 3 == 6", "x = false;\n") + expectPrintedMangle(t, "x = 3 != 6", "x = true;\n") + expectPrintedMangle(t, "x = 3 === 6", "x = false;\n") + expectPrintedMangle(t, "x = 3 !== 6", "x = true;\n") + + expectPrintedMangle(t, "x = 3 in 6", "x = 3 in 6;\n") + expectPrintedMangle(t, "x = 3 instanceof 6", "x = 3 instanceof 6;\n") + expectPrintedMangle(t, "x = (3, 6)", "x = 6;\n") + + expectPrintedMangle(t, "x = 3 << 6", "x = 3 << 6;\n") + expectPrintedMangle(t, "x = 3 >> 6", "x = 3 >> 6;\n") + expectPrintedMangle(t, "x = 3 >>> 6", "x = 3 >>> 6;\n") + expectPrintedMangle(t, "x = 3 & 6", "x = 2;\n") + expectPrintedMangle(t, "x = 3 | 6", "x = 7;\n") + expectPrintedMangle(t, "x = 3 ^ 6", "x = 5;\n") + + expectPrintedMangle(t, "x = 3 && 6", "x = 6;\n") + expectPrintedMangle(t, "x = 3 || 6", "x = 3;\n") + expectPrintedMangle(t, "x = 3 ?? 6", "x = 3;\n") +} + func TestMangleNestedLogical(t *testing.T) { expectPrintedMangle(t, "(a && b) && c", "a && b && c;\n") expectPrintedMangle(t, "a && (b && c)", "a && b && c;\n") diff --git a/internal/js_printer/js_printer.go b/internal/js_printer/js_printer.go index abc970b3697..69016a27928 100644 --- a/internal/js_printer/js_printer.go +++ b/internal/js_printer/js_printer.go @@ -1007,8 +1007,8 @@ func (p *printer) printProperty(item js_ast.Property) { case *js_ast.EImportIdentifier: // Make sure we're not using a property access instead of an identifier ref := js_ast.FollowSymbols(p.symbols, e.Ref) - symbol := p.symbols.Get(ref) - if symbol.NamespaceAlias == nil && name == p.renamer.NameForSymbol(e.Ref) { + if symbol := p.symbols.Get(ref); symbol.NamespaceAlias == nil && name == p.renamer.NameForSymbol(ref) && + p.options.ConstValues[ref].Kind == js_ast.ConstValueNone { if item.InitializerOrNil.Data != nil { p.printSpace() p.print("=") @@ -1046,8 +1046,8 @@ func (p *printer) printProperty(item js_ast.Property) { case *js_ast.EImportIdentifier: // Make sure we're not using a property access instead of an identifier ref := js_ast.FollowSymbols(p.symbols, e.Ref) - symbol := p.symbols.Get(ref) - if symbol.NamespaceAlias == nil && js_lexer.UTF16EqualsString(key.Value, p.renamer.NameForSymbol(e.Ref)) { + if symbol := p.symbols.Get(ref); symbol.NamespaceAlias == nil && js_lexer.UTF16EqualsString(key.Value, p.renamer.NameForSymbol(ref)) && + p.options.ConstValues[ref].Kind == js_ast.ConstValueNone { if item.InitializerOrNil.Data != nil { p.printSpace() p.print("=") @@ -2234,6 +2234,9 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla if wrap { p.print(")") } + } else if value := p.options.ConstValues[ref]; value.Kind != js_ast.ConstValueNone { + // Handle inlined constants + p.printExpr(js_ast.ConstValueToExpr(expr.Loc, value), level, flags) } else { p.printSymbol(e.Ref) } @@ -3490,6 +3493,9 @@ type Options struct { // Cross-module inlining of TypeScript enums is actually done during printing TSEnums map[js_ast.Ref]map[string]js_ast.TSEnumValue + // Cross-module inlining of detected inlinable constants is also done during printing + ConstValues map[js_ast.Ref]js_ast.ConstValue + // This will be present if the input file had a source map. In that case we // want to map all the way back to the original input file(s). InputSourceMap *sourcemap.SourceMap