add more special cases to ts namespace transform

CHANGELOG.md

@@ -6,6 +6,41 @@
 
     Previously esbuild would collapse `Object.is(x ? 0 : -0, -0)` into `Object.is((x, 0), -0)` during minification, which is incorrect. The IEEE floating-point value `-0` is a different bit pattern than `0` and while they both compare equal, the difference is detectable in a few scenarios such as when using `Object.is()`. The minification transformation now checks for `-0` vs. `0` and no longer has this bug. This fix was contributed by [@rtsao](https://github.com/rtsao).
 
+* Match the TypeScript compiler's output in a strange edge case ([#1158](https://github.com/evanw/esbuild/issues/1158))
+
+    With this release, esbuild's TypeScript-to-JavaScript transform will no longer omit the namespace in this case:
+
+    ```ts
+    namespace Something {
+      export declare function Print(a: string): void
+    }
+    Something.Print = function(a) {}
+    ```
+
+    This was previously omitted because TypeScript omits empty namespaces, and the namespace was considered empty because the `export declare function` statement isn't "real":
+
+    ```ts
+    namespace Something {
+      export declare function Print(a: string): void
+      setTimeout(() => Print('test'))
+    }
+    Something.Print = function(a) {}
+    ```
+
+    The TypeScript compiler compiles the above code into the following:
+
+    ```js
+    var Something;
+    (function (Something) {
+      setTimeout(() => Print('test'));
+    })(Something || (Something = {}));
+    Something.Print = function (a) { };
+    ```
+
+    Notice how `Something.Print` is never called, and what appears to be a reference to the `Print` symbol on the namespace `Something` is actually a reference to the global variable `Print`. I can only assume this is a bug in TypeScript, but it's important to replicate this behavior inside esbuild for TypeScript compatibility.
+
+    The TypeScript-to-JavaScript transform in esbuild has been updated to match the TypeScript compiler's output in both of these cases.
+
 ## 0.11.11
 
 * Initial support for Deno ([#936](https://github.com/evanw/esbuild/issues/936))

internal/bundler/bundler_ts_test.go

@@ -665,13 +665,13 @@ func TestTSExportDefaultTypeIssue316(t *testing.T) {
 				import tone_def, { bar as tone } from './remove/type-only-namespace-exported'
 
 				export default [
-					dc,
-					dl,
-					im,
-					_in,
-					tn,
-					vn,
-					vnm,
+					dc_def, dc,
+					dl_def, dl,
+					im_def, im,
+					in_def, _in,
+					tn_def, tn,
+					vn_def, vn,
+					vnm_def, vnm,
 
 					i,
 					ie,

internal/bundler/snapshots/snapshots_ts.txt

@@ -95,6 +95,7 @@ var _foo = class {
 };
 var foo2 = _foo;
 foo2.x = new _foo();
+var interface_merged_default = foo2;
 var bar3 = 123;
 
 // keep/interface-nested.ts
@@ -114,13 +115,15 @@ var foo3;
 (function(foo5) {
   foo5.num = 0;
 })(foo3 || (foo3 = {}));
+var value_namespace_default = foo3;
 var bar6 = 123;
 
 // keep/value-namespace-merged.ts
 var foo4;
 (function(foo5) {
   foo5.num = 0;
 })(foo4 || (foo4 = {}));
+var value_namespace_merged_default = foo4;
 var bar7 = 123;
 
 // remove/interface.ts
@@ -143,12 +146,19 @@ var bar13 = 123;
 
 // entry.ts
 var entry_default = [
+  declare_class_default,
   bar,
+  declare_let_default,
   bar2,
+  interface_merged_default,
   bar3,
+  interface_nested_default,
   bar4,
+  type_nested_default,
   bar5,
+  value_namespace_default,
   bar6,
+  value_namespace_merged_default,
   bar7,
   bar8,
   bar9,

internal/js_parser/js_parser.go

@@ -143,6 +143,62 @@ type parser struct {
 	moduleScope       *js_ast.Scope
 	isControlFlowDead bool
 
+	// Inside a TypeScript namespace, an "export declare" statement can be used
+	// to cause a namespace to be emitted even though it has no other observable
+	// effect. This flag is used to implement this feature.
+	//
+	// Specifically, namespaces should be generated for all of the following
+	// namespaces below except for "f", which should not be generated:
+	//
+	//   namespace a { export declare const a }
+	//   namespace b { export declare let [[b]] }
+	//   namespace c { export declare function c() }
+	//   namespace d { export declare class d {} }
+	//   namespace e { export declare enum e {} }
+	//   namespace f { export declare namespace f {} }
+	//
+	// The TypeScript compiler compiles this into the following code (notice "f"
+	// is missing):
+	//
+	//   var a; (function (a_1) {})(a || (a = {}));
+	//   var b; (function (b_1) {})(b || (b = {}));
+	//   var c; (function (c_1) {})(c || (c = {}));
+	//   var d; (function (d_1) {})(d || (d = {}));
+	//   var e; (function (e_1) {})(e || (e = {}));
+	//
+	// Note that this should not be implemented by declaring symbols for "export
+	// declare" statements because the TypeScript compiler doesn't generate any
+	// code for these statements, so these statements are actually references to
+	// global variables. There is one exception, which is that local variables
+	// *should* be declared as symbols because they are replaced with. This seems
+	// like very arbitrary behavior but it's what the TypeScript compiler does,
+	// so we try to match it.
+	//
+	// Specifically, in the following code below "a" and "b" should be declared
+	// and should be substituted with "ns.a" and "ns.b" but the other symbols
+	// shouldn't. References to the other symbols actually refer to global
+	// variables instead of to symbols that are exported from the namespace.
+	// This is the case as of TypeScript 4.3. I assume this is a TypeScript bug:
+	//
+	//   namespace ns {
+	//     export declare const a
+	//     export declare let [[b]]
+	//     export declare function c()
+	//     export declare class d { }
+	//     export declare enum e { }
+	//     console.log(a, b, c, d, e)
+	//   }
+	//
+	// The TypeScript compiler compiles this into the following code:
+	//
+	//   var ns;
+	//   (function (ns) {
+	//       console.log(ns.a, ns.b, c, d, e);
+	//   })(ns || (ns = {}));
+	//
+	// Relevant issue: https://github.com/evanw/esbuild/issues/1158
+	hasNonLocalExportDeclareInsideNamespace bool
+
 	// This helps recognize the "await import()" pattern. When this is present,
 	// warnings about non-string import paths will be omitted inside try blocks.
 	awaitTarget js_ast.E
@@ -4708,11 +4764,18 @@ func (p *parser) parseClassStmt(loc logger.Loc, opts parseStmtOpts) js_ast.Stmt
 	}
 	scopeIndex := p.pushScopeForParsePass(js_ast.ScopeClassName, loc)
 	class := p.parseClass(classKeyword, name, classOpts)
-	if classOpts.isTypeScriptDeclare {
+
+	if opts.isTypeScriptDeclare {
 		p.popAndDiscardScope(scopeIndex)
-	} else {
-		p.popScope()
+
+		if opts.isNamespaceScope && opts.isExport {
+			p.hasNonLocalExportDeclareInsideNamespace = true
+		}
+
+		return js_ast.Stmt{Loc: loc, Data: &js_ast.STypeScript{}}
 	}
+
+	p.popScope()
 	return js_ast.Stmt{Loc: loc, Data: &js_ast.SClass{Class: class, IsExport: opts.isExport}}
 }
 
@@ -4908,6 +4971,10 @@ func (p *parser) parseFnStmt(loc logger.Loc, opts parseStmtOpts, isAsync bool, a
 			p.popAndDiscardScope(ifStmtScopeIndex)
 		}
 
+		if opts.isTypeScriptDeclare && opts.isNamespaceScope && opts.isExport {
+			p.hasNonLocalExportDeclareInsideNamespace = true
+		}
+
 		return js_ast.Stmt{Loc: loc, Data: &js_ast.STypeScript{}}
 	}
 

internal/js_parser/ts_parser.go

@@ -867,6 +867,15 @@ func (p *parser) parseTypeScriptEnumStmt(loc logger.Loc, opts parseStmtOpts) js_
 	}
 
 	p.lexer.Expect(js_lexer.TCloseBrace)
+
+	if opts.isTypeScriptDeclare {
+		if opts.isNamespaceScope && opts.isExport {
+			p.hasNonLocalExportDeclareInsideNamespace = true
+		}
+
+		return js_ast.Stmt{Loc: loc, Data: &js_ast.STypeScript{}}
+	}
+
 	return js_ast.Stmt{Loc: loc, Data: &js_ast.SEnum{
 		Name:     name,
 		Arg:      argRef,
@@ -932,6 +941,8 @@ func (p *parser) parseTypeScriptNamespaceStmt(loc logger.Loc, opts parseStmtOpts
 	name := js_ast.LocRef{Loc: nameLoc, Ref: js_ast.InvalidRef}
 	scopeIndex := p.pushScopeForParsePass(js_ast.ScopeEntry, loc)
 
+	oldHasNonLocalExportDeclareInsideNamespace := p.hasNonLocalExportDeclareInsideNamespace
+	p.hasNonLocalExportDeclareInsideNamespace = false
 	var stmts []js_ast.Stmt
 	if p.lexer.Token == js_lexer.TDot {
 		dotLoc := p.lexer.Loc()
@@ -951,6 +962,8 @@ func (p *parser) parseTypeScriptNamespaceStmt(loc logger.Loc, opts parseStmtOpts
 		})
 		p.lexer.Next()
 	}
+	hasNonLocalExportDeclareInsideNamespace := p.hasNonLocalExportDeclareInsideNamespace
+	p.hasNonLocalExportDeclareInsideNamespace = oldHasNonLocalExportDeclareInsideNamespace
 
 	// Import assignments may be only used in type expressions, not value
 	// expressions. If this is the case, the TypeScript compiler removes
@@ -968,7 +981,12 @@ func (p *parser) parseTypeScriptNamespaceStmt(loc logger.Loc, opts parseStmtOpts
 	// allowed to be exported, but can also only be used in type
 	// expressions when imported. So we shouldn't count them as a
 	// real export either.
-	if len(stmts) == importEqualsCount || opts.isTypeScriptDeclare {
+	//
+	// TypeScript also strangely counts namespaces containing only
+	// "export declare" statements as non-empty even though "declare"
+	// statements are only type annotations. We cannot omit the namespace
+	// in that case. See https://github.com/evanw/esbuild/issues/1158.
+	if (len(stmts) == importEqualsCount && !hasNonLocalExportDeclareInsideNamespace) || opts.isTypeScriptDeclare {
 		p.popAndDiscardScope(scopeIndex)
 		if opts.isModuleScope {
 			p.localTypeNames[nameText] = true

internal/js_parser/ts_parser_test.go

@@ -881,6 +881,30 @@ var f;
   _f.f = f;
   log(f);
 })(f || (f = {}));
+`)
+
+	expectPrintedTS(t, `
+		namespace a { export declare var a }
+		namespace b { export declare let b }
+		namespace c { export declare enum c {} }
+		namespace d { export declare class d {} }
+		namespace e { export declare namespace e {} }
+		namespace f { export declare function f() {} }
+	`, `var a;
+(function(_a) {
+})(a || (a = {}));
+var b;
+(function(_b) {
+})(b || (b = {}));
+var c;
+(function(c) {
+})(c || (c = {}));
+var d;
+(function(d) {
+})(d || (d = {}));
+var f;
+(function(f) {
+})(f || (f = {}));
 `)
 }