fix #1628: "export {}" with "--tree-shaking=true"

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+* Fix `export {}` statements with `--tree-shaking=true` ([#1628](https://github.com/evanw/esbuild/issues/1628))
+
+    The new `--tree-shaking=true` option allows you to force-enable tree shaking in cases where it wasn't previously possible. One such case is when bundling is disabled and there is no output format configured, in which case esbuild just preserves the format of whatever format the input code is in. Enabling tree shaking in this context caused a bug where `export {}` statements were stripped. This release fixes the bug so `export {}` statements should now be preserved when you pass `--tree-shaking=true`. This bug only affected this new functionality and didn't affect existing scenarios.
+
 ## 0.13.1
 
 * Fix the `esbuild` package in yarn 2+

internal/bundler/linker.go

@@ -4291,7 +4291,7 @@ func (c *linkerContext) renameSymbolsInChunk(chunk *chunkInfo, filesInOrder []ui
 		//
 		//   // foo.js
 		//   var foo, foo_exports = {};
-		//   __exports(foo_exports, {
+		//   __export(foo_exports, {
 		//     foo: () => foo
 		//   });
 		//   let init_foo = __esm(() => {

internal/graph/meta.go

@@ -30,7 +30,7 @@ const (
 	//
 	//   // foo.ts
 	//   var foo, foo_exports = {};
-	//   __exports(foo_exports, {
+	//   __export(foo_exports, {
 	//     foo: () => foo
 	//   });
 	//   let init_foo = __esm(() => {

internal/js_parser/js_parser.go

@@ -13121,9 +13121,9 @@ func (p *parser) appendPart(parts []js_ast.Part, stmts []js_ast.Stmt) []js_ast.P
 	p.declaredSymbols = nil
 	p.importRecordsForCurrentPart = nil
 	p.scopesForCurrentPart = nil
-	part := js_ast.Part{
-		Stmts: p.visitStmtsAndPrependTempRefs(stmts, prependTempRefsOpts{}),
 
+	part := js_ast.Part{
+		Stmts:      p.visitStmtsAndPrependTempRefs(stmts, prependTempRefsOpts{}),
 		SymbolUses: p.symbolUses,
 	}
 
@@ -13201,9 +13201,17 @@ func (p *parser) stmtsCanBeRemovedIfUnused(stmts []js_ast.Stmt) bool {
 				}
 			}
 
-		case *js_ast.SExportClause, *js_ast.SExportFrom:
+		case *js_ast.SExportFrom:
 			// Exports are tracked separately, so this isn't necessary
 
+		case *js_ast.SExportClause:
+			// Exports are tracked separately, so this isn't necessary. Except we
+			// should keep all of these statements if we're not doing any format
+			// conversion, because exports are not re-emitted in that case.
+			if p.options.mode == config.ModePassThrough {
+				return false
+			}
+
 		case *js_ast.SExportDefault:
 			switch s2 := s.Value.Data.(type) {
 			case *js_ast.SExpr:
@@ -13922,10 +13930,22 @@ func Parse(log logger.Log, source logger.Source, options Options) (result js_ast
 				}
 
 			case *js_ast.SImport, *js_ast.SExportFrom, *js_ast.SExportStar:
-				// Move imports (and import-like exports) to the top of the file to
-				// ensure that if they are converted to a require() call, the effects
-				// will take place before any other statements are evaluated.
-				before = p.appendPart(before, []js_ast.Stmt{stmt})
+				if p.options.mode != config.ModePassThrough {
+					// Move imports (and import-like exports) to the top of the file to
+					// ensure that if they are converted to a require() call, the effects
+					// will take place before any other statements are evaluated.
+					before = p.appendPart(before, []js_ast.Stmt{stmt})
+				} else {
+					// If we aren't doing any format conversion, just keep these statements
+					// inline where they were. Exports are sorted so order doesn't matter:
+					// https://262.ecma-international.org/6.0/#sec-module-namespace-exotic-objects.
+					// However, this is likely an aesthetic issue that some people will
+					// complain about. In addition, there are code transformation tools
+					// such as TypeScript and Babel with bugs where the order of exports
+					// in the file is incorrectly preserved instead of sorted, so preserving
+					// the order of exports ourselves here may be preferable.
+					parts = p.appendPart(parts, []js_ast.Stmt{stmt})
+				}
 
 			case *js_ast.SExportEquals:
 				// TypeScript "export = value;" becomes "module.exports = value;". This

scripts/js-api-tests.js

@@ -3207,30 +3207,42 @@ let transformTests = {
 
   // Note: tree shaking is disabled when the output format isn't IIFE
   async treeShakingDefault({ esbuild }) {
-    const { code } = await esbuild.transform(`var unused = 123`, {
+    const { code } = await esbuild.transform(`
+      var unused = 123
+      var used = 234
+      export { used }
+    `, {
       loader: 'jsx',
       format: 'esm',
       treeShaking: undefined,
     })
-    assert.strictEqual(code, `var unused = 123;\n`)
+    assert.strictEqual(code, `var unused = 123;\nvar used = 234;\nexport {\n  used\n};\n`)
   },
 
   async treeShakingFalse({ esbuild }) {
-    const { code } = await esbuild.transform(`var unused = 123`, {
+    const { code } = await esbuild.transform(`
+      var unused = 123
+      var used = 234
+      export { used }
+    `, {
       loader: 'jsx',
       format: 'esm',
       treeShaking: false,
     })
-    assert.strictEqual(code, `var unused = 123;\n`)
+    assert.strictEqual(code, `var unused = 123;\nvar used = 234;\nexport {\n  used\n};\n`)
   },
 
   async treeShakingTrue({ esbuild }) {
-    const { code } = await esbuild.transform(`var unused = 123`, {
+    const { code } = await esbuild.transform(`
+      var unused = 123
+      var used = 234
+      export { used }
+    `, {
       loader: 'jsx',
       format: 'esm',
       treeShaking: true,
     })
-    assert.strictEqual(code, ``)
+    assert.strictEqual(code, `var used = 234;\nexport {\n  used\n};\n`)
   },
 
   // Note: tree shaking is enabled when the output format is IIFE