mix babel and node semantics for "default" (#532)

CHANGELOG.md

@@ -2,15 +2,17 @@
 
 ## Unreleased
 
-* Align with node's `default` import behavior for CommonJS ([#532](https://github.com/evanw/esbuild/issues/532))
+* Align more closely with node's `default` import behavior for CommonJS ([#532](https://github.com/evanw/esbuild/issues/532))
 
     _Note: This could be considered a breaking change or a bug fix depending on your point of view._
 
     Importing a CommonJS file into an ESM file does not behave the same everywhere. Historically people compiled their ESM code into CommonJS using Babel before ESM was supported natively. More recently, node has made it possible to use ESM syntax natively but to still import CommonJS files into ESM. These behave differently in many ways but one of the most unfortunate differences is how the `default` export is handled.
 
     When you import a normal CommonJS file, both Babel and node agree that the value of `module.exports` should be stored in the ESM import named `default`. However, if the CommonJS file used to be an ESM file but was compiled into a CommonJS file, Babel will set the ESM import named `default` to the value of the original ESM export named `default` while node will continue to set the ESM import named `default` to the value of `module.exports`. Babel detects if a CommonJS file used to be an ESM file by the presence of the `exports.__esModule = true` marker.
 
-    This is unfortunate because it means there is no general way to make code work with both ecosystems. With Babel you can access the original `default` export with `require().default` but with node you need to use `require().default.default` to access the original `default` export. Previously esbuild followed Babel's approach but starting with this release, esbuild will now follow node's approach. Now that node has native ESM support, people will be writing more and more native ESM code and will expect bundlers to follow node semantics. Using ESM via Babel is now the legacy code path that will become less and less relevant over time.
+    This is unfortunate because it means there is no general way to make code work with both ecosystems. With Babel the code `import * as someFile from './some-file'` can access the original `default` export with `someFile.default` but with node you need to use `someFile.default.default` instead. Previously esbuild followed Babel's approach but starting with this release, esbuild will now try to use a blend between the Babel and node approaches.
+
+    This is the new behavior: importing a CommonJS file will set the `default` import to `module.exports` in all cases except when `module.exports.__esModule && "default" in module.exports`, in which case it will fall through to `module.exports.default`. In other words: in cases where the default import was previously `undefined` for CommonJS files when `exports.__esModule === true`, the default import will now be `module.exports`. This should hopefully keep Babel cross-compiled ESM code mostly working but at the same time now enable some node-oriented code to start working.
 
     If you are authoring a library using ESM but shipping it as CommonJS, the best way to avoid this mess is to just never use `default` exports in ESM. Only use named exports with names other than `default`.
 

internal/bundler/linker.go

@@ -197,10 +197,10 @@ type fileMeta struct {
 	// the other one.
 	//
 	// We need to behave like Babel when we cross-compile ESM to CommonJS but
-	// we need to behave like node for compatibility with existing libraries
-	// on npm. So we deliberately skip calling "__toModule" only for ESM files
-	// that we ourselves have converted to CommonJS during the build so that we
-	// at least don't break ourselves.
+	// we need to behave like a mix of Babel and node for compatibility with
+	// existing libraries on npm. So we deliberately skip calling "__toModule"
+	// only for ESM files that we ourselves have converted to CommonJS during
+	// the build so that we at least don't break ourselves.
 	skipCallingToModule bool
 
 	// If true, the "__export(exports, { ... })" call will be force-included even

internal/runtime/runtime.go

@@ -111,9 +111,20 @@ func code(isES6 bool) string {
 
 		// Converts the module from CommonJS to ES6 if necessary
 		export var __toModule = module => {
-			return __exportStar(
-				__markAsModule(__defProp(module != null ? __create(__getProtoOf(module)) : {}, 'default', { value: module, enumerable: true })),
-				module)
+			return __exportStar(__markAsModule(
+				__defProp(
+					module != null ? __create(__getProtoOf(module)) : {},
+					'default',
+
+					// If this is an ESM file that has been converted to a CommonJS file
+					// using a Babel-compatible transform (i.e. "__esModule" has been set)
+					// and there is already a "default" property, then forward "default"
+					// to that property. Otherwise set "default" to "module.exports" for
+					// node compatibility.
+					module && module.__esModule && 'default' in module
+						? { get: () => module.default, enumerable: true }
+						: { value: module, enumerable: true })
+			), module)
 		}
 
 		// For TypeScript decorators

scripts/end-to-end-tests.js

@@ -556,6 +556,26 @@
     }
   }
 
+  // Test "default" exports in ESM-to-CommonJS conversion scenarios
+  tests.push(
+    test(['in.js', '--outfile=node.js', '--format=cjs'], {
+      'in.js': `import def from './foo'; if (def !== 123) throw 'fail'`,
+      'foo.js': `exports.__esModule = true; exports.default = 123`,
+    }),
+    test(['in.js', '--outfile=node.js', '--format=cjs'], {
+      'in.js': `import * as ns from './foo'; if (ns.default !== 123) throw 'fail'`,
+      'foo.js': `exports.__esModule = true; exports.default = 123`,
+    }),
+    test(['in.js', '--outfile=node.js', '--format=cjs'], {
+      'in.js': `import def from './foo'; if (!def || def.foo !== 123) throw 'fail'`,
+      'foo.js': `exports.__esModule = true; exports.foo = 123`,
+    }),
+    test(['in.js', '--outfile=node.js', '--format=cjs'], {
+      'in.js': `import * as ns from './foo'; if (!ns.default || ns.default.foo !== 123) throw 'fail'`,
+      'foo.js': `exports.__esModule = true; exports.foo = 123`,
+    }),
+  )
+
   // Test external CommonJS export
   tests.push(
     test(['--bundle', 'foo.js', '--outfile=out.js', '--format=cjs'], {