fix #478: add "--avoid-tdz" workaround for safari

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## Unreleased
+
+* Add the `--avoid-tdz` option for large bundles in Safari ([#478](https://github.com/evanw/esbuild/issues/478))
+
+    This is a workaround for a performance issue with certain large JavaScript files in Safari.
+
+    First, some background. In JavaScript the `var` statement is "hoisted" meaning the variable is declared immediately in the closest surrounding function, module, or global scope. Accessing one of these variables before its declaration has been evaluated results in the value `undefined`. In ES6 the `const`, `let`, and `class` statements introduce what's called a "temporal dead zone" or TDZ. This means that, unlike `var` statements, accessing one of these variable before its declaration has been evaluated results in a `ReferenceError` being thrown. It's called a "temporal dead zone" because it's a zone of time in which the variable is inaccessible.
+
+    According to [this WebKit bug](https://bugs.webkit.org/show_bug.cgi?id=199866), there's a severe performance issue with the tracking of TDZ checks in JavaScriptCore, the JavaScript JIT compiler used by WebKit. In a large private code base I have access to, the initialization phase of the bundle produced by esbuild runs 10x faster in Safari if top-level `const`, `let`, and `class` are replaced with `var`. It's a difference between a loading time of about 2sec vs. about 200ms. This transformation is not enabled by default because it changes the semantics of the code (it removes the TDZ and `const` assignment checks). However, this change in semantics may be acceptable for you given the performance trade-off. You can enable it with the `--avoid-tdz` flag.
+
 ## 0.7.18
 
 * Treat paths in CSS without a `./` or `../` prefix as relative ([#469](https://github.com/evanw/esbuild/issues/469))

README.md

@@ -405,6 +405,7 @@ Advanced options:
                             browser and "main,module" when platform is node)
   --public-path=...         Set the base URL for the "file" loader
   --color=...               Force use of color terminal escapes (true | false)
+  --avoid-tdz               An optimization for large bundles in Safari
 
 Examples:
   # Produces dist/entry_point.js and dist/entry_point.js.map

cmd/esbuild/main.go

@@ -69,6 +69,7 @@ Advanced options:
                             browser and "main,module" when platform is node)
   --public-path=...         Set the base URL for the "file" loader
   --color=...               Force use of color terminal escapes (true | false)
+  --avoid-tdz               An optimization for large bundles in Safari
 
 Examples:
   # Produces dist/entry_point.js and dist/entry_point.js.map

internal/bundler/bundler_default_test.go

@@ -3168,3 +3168,27 @@ func TestInjectImportOrder(t *testing.T) {
 		},
 	})
 }
+
+func TestAvoidTDZNoBundle(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				class Foo {
+					static foo = new Foo
+				}
+				let foo = Foo.foo
+				console.log(foo)
+				export class Bar {}
+				export let bar = 123
+			`,
+			"/foo.js": `
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModePassThrough,
+			AbsOutputFile: "/out.js",
+			AvoidTDZ:      true,
+		},
+	})
+}

internal/bundler/linker.go

@@ -1485,7 +1485,7 @@ func (c *linkerContext) createExportsForFile(sourceIndex uint32) {
 				//   };
 				//
 				kind := js_ast.LocalConst
-				if c.options.UnsupportedJSFeatures.Has(compat.Const) {
+				if c.options.AvoidTDZ || c.options.UnsupportedJSFeatures.Has(compat.Const) {
 					kind = js_ast.LocalVar
 				}
 				entryPointExportStmts = append(entryPointExportStmts, js_ast.Stmt{Data: &js_ast.SLocal{
@@ -1569,7 +1569,7 @@ func (c *linkerContext) createExportsForFile(sourceIndex uint32) {
 	var nsExportStmts []js_ast.Stmt
 	if !repr.meta.cjsStyleExports {
 		kind := js_ast.LocalConst
-		if c.options.UnsupportedJSFeatures.Has(compat.Const) {
+		if c.options.AvoidTDZ || c.options.UnsupportedJSFeatures.Has(compat.Const) {
 			kind = js_ast.LocalVar
 		}
 		nsExportStmts = append(nsExportStmts, js_ast.Stmt{Data: &js_ast.SLocal{Kind: kind, Decls: []js_ast.Decl{{
@@ -2658,7 +2658,7 @@ func (c *linkerContext) shouldRemoveImportExportStmt(
 
 	// Replace the statement with a call to "require()"
 	kind := js_ast.LocalConst
-	if c.options.UnsupportedJSFeatures.Has(compat.Const) {
+	if c.options.AvoidTDZ || c.options.UnsupportedJSFeatures.Has(compat.Const) {
 		kind = js_ast.LocalVar
 	}
 	stmtList.prefixStmts = append(stmtList.prefixStmts, js_ast.Stmt{
@@ -2816,20 +2816,33 @@ func (c *linkerContext) convertStmtsForChunk(sourceIndex uint32, stmtList *stmtL
 			}
 
 		case *js_ast.SClass:
-			// Strip the "export" keyword while bundling
-			if shouldStripExports && s.IsExport {
+			if c.options.AvoidTDZ {
+				stmt.Data = &js_ast.SLocal{
+					IsExport: s.IsExport && !shouldStripExports,
+					Decls: []js_ast.Decl{{
+						Binding: js_ast.Binding{Loc: s.Class.Name.Loc, Data: &js_ast.BIdentifier{Ref: s.Class.Name.Ref}},
+						Value:   &js_ast.Expr{Loc: stmt.Loc, Data: &js_ast.EClass{Class: s.Class}},
+					}},
+				}
+			} else if shouldStripExports && s.IsExport {
 				// Be careful to not modify the original statement
 				clone := *s
 				clone.IsExport = false
 				stmt.Data = &clone
 			}
 
 		case *js_ast.SLocal:
-			// Strip the "export" keyword while bundling
-			if shouldStripExports && s.IsExport {
+			stripExport := shouldStripExports && s.IsExport
+			avoidTDZ := c.options.AvoidTDZ && s.Kind != js_ast.LocalVar
+			if stripExport || avoidTDZ {
 				// Be careful to not modify the original statement
 				clone := *s
-				clone.IsExport = false
+				if stripExport {
+					clone.IsExport = false
+				}
+				if avoidTDZ {
+					clone.Kind = js_ast.LocalVar
+				}
 				stmt.Data = &clone
 			}
 

internal/bundler/snapshots/snapshots_default.txt

@@ -145,6 +145,18 @@ import "https://example.com/code.js";
 import "//example.com/code.js";
 import "data:application/javascript;base64,ZXhwb3J0IGRlZmF1bHQgMTIz";
 
+================================================================================
+TestAvoidTDZNoBundle
+---------- /out.js ----------
+var Foo = class Foo {
+  static foo = new Foo();
+};
+var foo = Foo.foo;
+console.log(foo);
+export var Bar = class Bar {
+};
+export var bar = 123;
+
 ================================================================================
 TestCommonJSFromES6
 ---------- /out.js ----------

internal/config/config.go

@@ -187,6 +187,7 @@ type Options struct {
 
 	OmitRuntimeForTests     bool
 	PreserveUnusedImportsTS bool
+	AvoidTDZ                bool
 
 	Strict   StrictOptions
 	Defines  *ProcessedDefines

lib/common.ts

@@ -78,6 +78,7 @@ function pushCommonFlags(flags: string[], options: CommonOptions, keys: OptionKe
   let jsxFragment = getFlag(options, keys, 'jsxFragment', mustBeString);
   let define = getFlag(options, keys, 'define', mustBeObject);
   let pure = getFlag(options, keys, 'pure', mustBeArray);
+  let avoidTDZ = getFlag(options, keys, 'avoidTDZ', mustBeBoolean);
 
   if (target) {
     if (Array.isArray(target)) flags.push(`--target=${Array.from(target).map(validateTarget).join(',')}`)
@@ -102,6 +103,7 @@ function pushCommonFlags(flags: string[], options: CommonOptions, keys: OptionKe
     }
   }
   if (pure) for (let fn of pure) flags.push(`--pure:${fn}`);
+  if (avoidTDZ) flags.push(`--avoid-tdz`);
 }
 
 function flagsForBuildOptions(options: types.BuildOptions, isTTY: boolean, logLevelDefault: types.LogLevel): [string[], boolean, string | null, string | null] {

lib/types.ts

@@ -20,6 +20,7 @@ interface CommonOptions {
   jsxFragment?: string;
   define?: { [key: string]: string };
   pure?: string[];
+  avoidTDZ?: boolean;
 
   color?: boolean;
   logLevel?: LogLevel;

pkg/api/api.go

@@ -205,6 +205,7 @@ type BuildOptions struct {
 
 	Defines       map[string]string
 	PureFunctions []string
+	AvoidTDZ      bool
 
 	GlobalName        string
 	Bundle            bool
@@ -275,6 +276,7 @@ type TransformOptions struct {
 
 	Defines       map[string]string
 	PureFunctions []string
+	AvoidTDZ      bool
 
 	Sourcefile string
 	Loader     Loader

pkg/api/api_impl.go

@@ -434,6 +434,7 @@ func buildImpl(buildOpts BuildOptions) BuildResult {
 		TsConfigOverride:  validatePath(log, realFS, buildOpts.Tsconfig),
 		MainFields:        buildOpts.MainFields,
 		PublicPath:        buildOpts.PublicPath,
+		AvoidTDZ:          buildOpts.AvoidTDZ,
 		InjectAbsPaths:    make([]string, len(buildOpts.Inject)),
 	}
 	for i, path := range buildOpts.Inject {
@@ -622,6 +623,7 @@ func transformImpl(input string, transformOpts TransformOptions) TransformResult
 		RemoveWhitespace:  transformOpts.MinifyWhitespace,
 		MinifyIdentifiers: transformOpts.MinifyIdentifiers,
 		AbsOutputFile:     transformOpts.Sourcefile + "-out",
+		AvoidTDZ:          transformOpts.AvoidTDZ,
 		Stdin: &config.StdinInfo{
 			Loader:     validateLoader(transformOpts.Loader),
 			Contents:   input,

pkg/cli/cli_impl.go

@@ -69,6 +69,13 @@ func parseOptionsImpl(osArgs []string, buildOpts *api.BuildOptions, transformOpt
 				transformOpts.MinifyIdentifiers = true
 			}
 
+		case arg == "--avoid-tdz":
+			if buildOpts != nil {
+				buildOpts.AvoidTDZ = true
+			} else {
+				transformOpts.AvoidTDZ = true
+			}
+
 		case arg == "--sourcemap":
 			if buildOpts != nil {
 				buildOpts.Sourcemap = api.SourceMapLinked

scripts/js-api-tests.js

@@ -645,6 +645,49 @@ let transformTests = {
     }
   },
 
+  async avoidTDZ({ service }) {
+    for (const avoidTDZ of [false, true]) {
+      var { js } = await service.transform(`
+        class Foo {
+          // The above line will be transformed into "var". However, the
+          // symbol "Foo" must still be defined before the class body ends.
+          static foo = new Foo
+        }
+        if (!(Foo.foo instanceof Foo))
+          throw 'fail: avoidTDZ=${avoidTDZ}'
+      `, {
+        avoidTDZ,
+      })
+      new Function(js)()
+    }
+  },
+
+  async tsAvoidTDZ({ service }) {
+    for (const avoidTDZ of [false, true]) {
+      var { js } = await service.transform(`
+        class Bar {}
+        var oldFoo
+        function swap(target) {
+          oldFoo = target
+          return Bar
+        }
+        @swap
+        class Foo {
+          bar() { return new Foo }
+          static foo = new Foo
+        }
+        if (!(oldFoo.foo instanceof oldFoo))
+          throw 'fail: foo, avoidTDZ=${avoidTDZ}'
+        if (!(oldFoo.foo.bar() instanceof Bar))
+          throw 'fail: bar, avoidTDZ=${avoidTDZ}'
+      `, {
+        avoidTDZ,
+        loader: 'ts',
+      })
+      new Function(js)()
+    }
+  },
+
   async cjs_require({ service }) {
     const { js } = await service.transform(`const {foo} = require('path')`, {})
     assert.strictEqual(js, `const {foo} = require("path");\n`)