change esm/cjs support: "type" and "default"

CHANGELOG.md

@@ -35,6 +35,16 @@
 
     This initial release only has limited support for top-level await. It is only supported with the `esm` output format, but not with the `iife` or `cjs` output formats. In addition, the compilation is not correct in that two modules that both contain top-level await and that are siblings in the import graph will be evaluated in serial instead of in parallel. Full support for top-level await will come in a future release.
 
+* Change whether certain files are interpreted as ESM or CommonJS ([#1043](https://github.com/evanw/esbuild/issues/1043))
+
+    The bundling algorithm currently doesn't contain any logic that requires flagging modules as CommonJS vs. ESM beforehand. Instead it handles a superset and then sort of decides later if the module should be treated as CommonJS vs. ESM based on whether the module uses the `module` or `exports` variables and/or the `exports` keyword.
+
+    With this release, files that follow [node's rules for module types](https://nodejs.org/api/packages.html#packages_type) will be flagged as explicitly ESM. This includes files that end in `.mjs` and files within a package containing `"type": "module"` in the enclosing `package.json` file. The CommonJS `require`, `module`, and `exports` features will be unavailable in these files. This matters most for files without any exports, since then it's otherwise ambiguous what the module type is.
+
+    In addition, files without exports should now accurately fall back to being considered CommonJS. They should now generate a `default` export of an empty object when imported using an `import` statement, since that's what happens in node when you import a CommonJS file into an ESM file in node. Previously the default export could be undefined because these export-less files were sort of treated as ESM but with missing import errors turned into warnings instead.
+
+    This is an edge case that rarely comes up in practice, since you usually never import things from a module that has no exports.
+
 ## 0.9.7
 
 * Add support for Android on ARM 64-bit ([#803](https://github.com/evanw/esbuild/issues/803))

internal/ast/ast.go

@@ -77,6 +77,10 @@ type ImportRecord struct {
 	// CommonJS wrapper or not.
 	ContainsImportStar bool
 
+	// If this is true, the import contains an import for the alias "default",
+	// either via the "import x from" or "import {default as x} from" syntax.
+	ContainsDefaultAlias bool
+
 	// If true, this "export * from 'path'" statement is evaluated at run-time by
 	// calling the "__exportStar()" helper function
 	CallsRunTimeExportStarFn bool

internal/bundler/bundler.go

@@ -1005,6 +1005,15 @@ func (s *scanner) maybeParseFile(
 		optionsClone.PreserveUnusedImportsTS = true
 	}
 
+	// Set the module type preference using node's module type rules
+	if strings.HasSuffix(path.Text, ".mjs") {
+		optionsClone.ModuleType = config.ModuleESM
+	} else if strings.HasSuffix(path.Text, ".cjs") {
+		optionsClone.ModuleType = config.ModuleCommonJS
+	} else {
+		optionsClone.ModuleType = resolveResult.ModuleType
+	}
+
 	// Enable bundling for injected files so we always do tree shaking. We
 	// never want to include unnecessary code from injected files since they
 	// are essentially bundled. However, if we do this we should skip the

internal/bundler/bundler_default_test.go

@@ -693,10 +693,8 @@ func TestImportMissingNeitherES6NorCommonJS(t *testing.T) {
 			Mode:         config.ModeBundle,
 			AbsOutputDir: "/out",
 		},
-		expectedCompileLog: `named.js: warning: Import "default" will always be undefined because the file "foo.js" has no exports
-named.js: warning: Import "x" will always be undefined because the file "foo.js" has no exports
+		expectedCompileLog: `named.js: warning: Import "x" will always be undefined because the file "foo.js" has no exports
 named.js: warning: Import "y" will always be undefined because the file "foo.js" has no exports
-star.js: warning: Import "default" will always be undefined because the file "foo.js" has no exports
 star.js: warning: Import "x" will always be undefined because the file "foo.js" has no exports
 star.js: warning: Import "y" will always be undefined because the file "foo.js" has no exports
 `,

internal/bundler/bundler_splitting_test.go

@@ -272,7 +272,7 @@ func TestSplittingMissingLazyExport(t *testing.T) {
 			OutputFormat:  config.FormatESModule,
 			AbsOutputDir:  "/out",
 		},
-		expectedCompileLog: `common.js: warning: Import "missing" will always be undefined because there is no matching export
+		expectedCompileLog: `common.js: warning: Import "missing" will always be undefined because the file "empty.js" has no exports
 `,
 	})
 }

internal/bundler/linker.go

@@ -1278,14 +1278,16 @@ func (c *linkerContext) scanImportsAndExports() {
 					// We emit a warning in this case but try to avoid turning the module
 					// into a CommonJS module if possible. This is possible with named
 					// imports (the module stays an ECMAScript module but the imports are
-					// rewritten with undefined) but is not possible with star imports:
+					// rewritten with undefined) but is not possible with star or default
+					// imports:
 					//
 					//   import * as ns from './empty-file'
-					//   console.log(ns)
+					//   import defVal from './empty-file'
+					//   console.log(ns, defVal)
 					//
 					// In that case the module *is* considered a CommonJS module because
 					// the namespace object must be created.
-					if record.ContainsImportStar && otherRepr.ast.ExportsKind == js_ast.ExportsNone && !otherRepr.ast.HasLazyExport {
+					if (record.ContainsImportStar || record.ContainsDefaultAlias) && otherRepr.ast.ExportsKind == js_ast.ExportsNone && !otherRepr.ast.HasLazyExport {
 						otherRepr.ast.ExportsKind = js_ast.ExportsCommonJS
 					}
 
@@ -2498,7 +2500,11 @@ func (c *linkerContext) advanceImportTracker(tracker importTracker) (importTrack
 
 	// Is this a named import of a file without any exports?
 	otherRepr := c.files[otherSourceIndex].repr.(*reprJS)
-	if !namedImport.AliasIsStar && otherRepr.ast.ExportsKind == js_ast.ExportsNone && !otherRepr.ast.HasLazyExport {
+	if !namedImport.AliasIsStar && !otherRepr.ast.HasLazyExport &&
+		// CommonJS exports
+		otherRepr.ast.ExportKeyword.Len == 0 && namedImport.Alias != "default" &&
+		// ESM exports
+		!otherRepr.ast.UsesExportsRef && !otherRepr.ast.UsesModuleRef {
 		// Just warn about it and replace the import with "undefined"
 		return importTracker{sourceIndex: otherSourceIndex, importRef: js_ast.InvalidRef}, importCommonJSWithoutExports, nil
 	}

internal/bundler/snapshots/snapshots_default.txt

@@ -977,17 +977,23 @@ console.log((0, import_foo.default)(import_foo.x, import_foo.y));
 TestImportMissingNeitherES6NorCommonJS
 ---------- /out/named.js ----------
 // foo.js
-console.log("no exports here");
+var require_foo = __commonJS(() => {
+  console.log("no exports here");
+});
 
 // named.js
-console.log((void 0)(void 0, void 0));
+var import_foo = __toModule(require_foo());
+console.log((0, import_foo.default)(void 0, void 0));
 
 ---------- /out/star.js ----------
 // foo.js
-console.log("no exports here");
+var require_foo = __commonJS(() => {
+  console.log("no exports here");
+});
 
 // star.js
-console.log((void 0)(void 0, void 0));
+var ns = __toModule(require_foo());
+console.log(ns.default(void 0, void 0));
 
 ---------- /out/star-capture.js ----------
 // foo.js

internal/config/config.go

@@ -160,8 +160,17 @@ const (
 	ModeBundle
 )
 
+type ModuleType uint8
+
+const (
+	ModuleUnknown ModuleType = iota
+	ModuleCommonJS
+	ModuleESM
+)
+
 type Options struct {
 	Mode              Mode
+	ModuleType        ModuleType
 	PreserveSymlinks  bool
 	RemoveWhitespace  bool
 	MinifyIdentifiers bool

internal/js_parser/js_parser.go

@@ -286,6 +286,7 @@ type optionsThatSupportStructuralEquality struct {
 	mode                           config.Mode
 	platform                       config.Platform
 	outputFormat                   config.Format
+	moduleType                     config.ModuleType
 	asciiOnly                      bool
 	keepNames                      bool
 	mangleSyntax                   bool
@@ -309,6 +310,7 @@ func OptionsFromConfig(options *config.Options) Options {
 			mode:                           options.Mode,
 			platform:                       options.Platform,
 			outputFormat:                   options.OutputFormat,
+			moduleType:                     options.ModuleType,
 			asciiOnly:                      options.ASCIIOnly,
 			keepNames:                      options.KeepNames,
 			mangleSyntax:                   options.MangleSyntax,
@@ -11705,6 +11707,7 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) (result scanForIm
 	for _, stmt := range stmts {
 		switch s := stmt.Data.(type) {
 		case *js_ast.SImport:
+			record := &p.importRecords[s.ImportRecordIndex]
 
 			// The official TypeScript compiler always removes unused imported
 			// symbols. However, we deliberately deviate from the official
@@ -11852,7 +11855,7 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) (result scanForIm
 				if p.options.ts.Parse && foundImports && isUnusedInTypeScript && !p.options.preserveUnusedImportsTS {
 					// Ignore import records with a pre-filled source index. These are
 					// for injected files and we definitely do not want to trim these.
-					if record := &p.importRecords[s.ImportRecordIndex]; !record.SourceIndex.IsValid() {
+					if !record.SourceIndex.IsValid() {
 						record.IsUnused = true
 						continue
 					}
@@ -11983,7 +11986,17 @@ func (p *parser) scanForImportsAndExports(stmts []js_ast.Stmt) (result scanForIm
 			p.importRecordsForCurrentPart = append(p.importRecordsForCurrentPart, s.ImportRecordIndex)
 
 			if s.StarNameLoc != nil {
-				p.importRecords[s.ImportRecordIndex].ContainsImportStar = true
+				record.ContainsImportStar = true
+			}
+
+			if s.DefaultName != nil {
+				record.ContainsDefaultAlias = true
+			} else if s.Items != nil {
+				for _, item := range *s.Items {
+					if item.Alias == "default" {
+						record.ContainsDefaultAlias = true
+					}
+				}
 			}
 
 		case *js_ast.SFunction:
@@ -12877,7 +12890,11 @@ func (p *parser) prepareForVisitPass() {
 
 	p.hoistSymbols(p.moduleScope)
 
-	if p.options.mode != config.ModePassThrough {
+	// Do not enable CommonJS features inside a module that has been flagged as
+	// ESM using node's module type rules.
+	forceESM := p.options.moduleType == config.ModuleESM
+
+	if !forceESM && p.options.mode != config.ModePassThrough {
 		p.requireRef = p.declareCommonJSSymbol(js_ast.SymbolUnbound, "require")
 	} else {
 		p.requireRef = p.newSymbol(js_ast.SymbolUnbound, "require")
@@ -12886,7 +12903,7 @@ func (p *parser) prepareForVisitPass() {
 	// CommonJS-style exports are only enabled if this isn't using ECMAScript-
 	// style exports. You can still use "require" in ESM, just not "module" or
 	// "exports". You can also still use "import" in CommonJS.
-	if p.options.mode != config.ModePassThrough && p.es6ExportKeyword.Len == 0 && p.topLevelAwaitKeyword.Len == 0 {
+	if !forceESM && p.options.mode != config.ModePassThrough && p.es6ExportKeyword.Len == 0 && p.topLevelAwaitKeyword.Len == 0 {
 		p.exportsRef = p.declareCommonJSSymbol(js_ast.SymbolHoisted, "exports")
 		p.moduleRef = p.declareCommonJSSymbol(js_ast.SymbolHoisted, "module")
 	} else {
@@ -13161,8 +13178,27 @@ func (p *parser) toAST(source logger.Source, parts []js_ast.Part, hashbang strin
 		exportsKind = js_ast.ExportsESM
 	} else if usesExportsRef || usesModuleRef || p.hasTopLevelReturn {
 		exportsKind = js_ast.ExportsCommonJS
-	} else if p.es6ImportKeyword.Len > 0 {
-		exportsKind = js_ast.ExportsESM
+	} else {
+		// If this module has no exports, try to determine what kind of module it
+		// is by looking at node's "type" field in "package.json" and/or whether
+		// the file extension is ".mjs" or ".cjs".
+		switch p.options.moduleType {
+		case config.ModuleCommonJS:
+			// "type: module" or ".mjs"
+			exportsKind = js_ast.ExportsCommonJS
+
+		case config.ModuleESM:
+			// "type: commonjs" or ".cjs"
+			exportsKind = js_ast.ExportsESM
+
+		case config.ModuleUnknown:
+			// Treat unknown modules containing an import statement as ESM. Otherwise
+			// the bundler will treat this file as CommonJS if it's imported and ESM
+			// if it's not imported.
+			if p.es6ImportKeyword.Len > 0 {
+				exportsKind = js_ast.ExportsESM
+			}
+		}
 	}
 
 	return js_ast.AST{

internal/resolver/package_json.go

@@ -19,6 +19,7 @@ import (
 type packageJSON struct {
 	source        logger.Source
 	absMainFields map[string]string
+	moduleType    config.ModuleType
 
 	// Present if the "browser" field is present. This field is intended to be
 	// used by bundlers and lets you redirect the paths of certain 3rd-party
@@ -110,6 +111,24 @@ func (r *resolver) parsePackageJSON(path string) *packageJSON {
 
 	packageJSON := &packageJSON{source: jsonSource}
 
+	// Read the "type" field
+	if typeJSON, _, ok := getProperty(json, "type"); ok {
+		if typeValue, ok := getString(typeJSON); ok {
+			switch typeValue {
+			case "commonjs":
+				packageJSON.moduleType = config.ModuleCommonJS
+			case "module":
+				packageJSON.moduleType = config.ModuleESM
+			default:
+				r.log.AddRangeWarning(&jsonSource, jsonSource.RangeOfString(typeJSON.Loc),
+					fmt.Sprintf("%q is not a valid value for the \"type\" field (must be either \"commonjs\" or \"module\")", typeValue))
+			}
+		} else {
+			r.log.AddWarning(&jsonSource, typeJSON.Loc,
+				"The value for \"type\" must be a string")
+		}
+	}
+
 	// Read the "main" fields
 	mainFields := r.options.MainFields
 	if mainFields == nil {

internal/resolver/resolver.go

@@ -94,20 +94,24 @@ type ResolveResult struct {
 	JSXFactory  []string // Default if empty: "React.createElement"
 	JSXFragment []string // Default if empty: "React.Fragment"
 
-	IsExternal    bool
 	DifferentCase *fs.DifferentCase
 
 	// If true, any ES6 imports to this file can be considered to have no side
 	// effects. This means they should be removed if unused.
 	IgnorePrimaryIfUnused *IgnoreIfUnusedData
 
+	IsExternal bool
+
 	// If true, the class field transform should use Object.defineProperty().
 	UseDefineForClassFieldsTS bool
 
 	// If true, unused imports are retained in TypeScript code. This matches the
 	// behavior of the "importsNotUsedAsValues" field in "tsconfig.json" when the
 	// value is not "remove".
 	PreserveUnusedImportsTS bool
+
+	// This is the "type" field from "package.json"
+	ModuleType config.ModuleType
 }
 
 type AlternativeApproach uint8
@@ -414,6 +418,9 @@ func (r *resolver) finalizeResolve(result *ResolveResult) {
 									result.IgnorePrimaryIfUnused = info.packageJSON.ignoreIfUnusedData
 								}
 							}
+
+							// Also copy over the "type" field
+							result.ModuleType = info.packageJSON.moduleType
 							break
 						}
 					}

scripts/end-to-end-tests.js

@@ -780,6 +780,63 @@
     }),
   )
 
+  // Test imports from modules without any imports
+  tests.push(
+    test(['in.js', '--outfile=node.js', '--bundle'], {
+      'in.js': `
+        import * as ns from 'pkg'
+        if (ns.default === void 0) throw 'fail'
+      `,
+      'node_modules/pkg/index.js': ``,
+    }, {}),
+    test(['in.js', '--outfile=node.js', '--bundle'], {
+      'in.js': `
+        import * as ns from 'pkg/index.cjs'
+        if (ns.default === void 0) throw 'fail'
+      `,
+      'node_modules/pkg/index.cjs': ``,
+    }),
+    test(['in.js', '--outfile=node.js', '--bundle'], {
+      'in.js': `
+        import * as ns from 'pkg/index.mjs'
+        if (ns.default !== void 0) throw 'fail'
+      `,
+      'node_modules/pkg/index.mjs': ``,
+    }, {
+      expectedStderr: ` > in.js:3:15: warning: Import "default" will always be undefined because there is no matching export
+    3 │         if (ns.default !== void 0) throw 'fail'
+      ╵                ~~~~~~~
+
+`,
+    }),
+    test(['in.js', '--outfile=node.js', '--bundle'], {
+      'in.js': `
+        import * as ns from 'pkg'
+        if (ns.default === void 0) throw 'fail'
+      `,
+      'node_modules/pkg/package.json': `{
+        "type": "commonjs"
+      }`,
+      'node_modules/pkg/index.js': ``,
+    }),
+    test(['in.js', '--outfile=node.js', '--bundle'], {
+      'in.js': `
+        import * as ns from 'pkg'
+        if (ns.default !== void 0) throw 'fail'
+      `,
+      'node_modules/pkg/package.json': `{
+        "type": "module"
+      }`,
+      'node_modules/pkg/index.js': ``,
+    }, {
+      expectedStderr: ` > in.js:3:15: warning: Import "default" will always be undefined because there is no matching export
+    3 │         if (ns.default !== void 0) throw 'fail'
+      ╵                ~~~~~~~
+
+`,
+    }),
+  )
+
   // Test imports not being able to access the namespace object
   tests.push(
     test(['in.js', '--outfile=node.js', '--bundle'], {