fix #2417: add the module condition by default

CHANGELOG.md

@@ -113,6 +113,16 @@
 
     The legal comments feature automatically gathers comments containing `@license` or `@preserve` and puts the comments somewhere (either in the generated code or in a separate file). People sometimes want this to happen so that the their dependencies' software licenses are retained in the generated output code. By default esbuild puts these comments at the end of the file when bundling. However, people sometimes find this confusing because these comments can be very generic and may not mention which library they come from. So with this release, esbuild will now discard legal comments by default. You now have to opt-in to preserving them if you want this behavior.
 
+* Enable the `module` condition by default ([#2417](https://github.com/evanw/esbuild/issues/2417))
+
+    Package authors want to be able to use the new [`exports`](https://nodejs.org/api/packages.html#conditional-exports) field in `package.json` to provide tree-shakable ESM code for ESM-aware bundlers while simultaneously providing fallback CommonJS code for other cases.
+
+    Node's proposed way to do this involves using the `import` and `require` export conditions so that you get the ESM code if you use an import statement and the CommonJS code if you use a require call. However, this has a major drawback: if some code in the bundle uses an import statement and other code in the bundle uses a require call, then you'll get two copies of the same package in the bundle. This is known as the [dual package hazard](https://nodejs.org/api/packages.html#dual-package-hazard) and can lead to bloated bundles or even worse to subtle logic bugs.
+
+    Webpack supports an alternate solution: an export condition called `module` that takes effect regardless of whether the package was imported using an import statement or a require call. This works because bundlers such as Webpack support importing a ESM using a require call (something node doesn't support). You could already do this with esbuild using `--conditions=module` but you previously had to explicitly enable this. Package authors are concerned that esbuild users won't know to do this and will get suboptimal output with their package, so they have requested for esbuild to do this automatically.
+
+    So with this release, esbuild will now automatically add the `module` condition when there aren't any custom `conditions` already configured. You can disable this with `--conditions=` or `conditions: []` (i.e. explicitly clearing all custom conditions).
+
 * Rename the `master` branch to `main`
 
     The primary branch for this repository was previously called `master` but is now called `main`. This change mirrors a similar change in many other projects.

pkg/api/api_impl.go

@@ -1005,7 +1005,6 @@ func rebuildImpl(
 		PackageAliases:        validateAlias(log, realFS, buildOpts.Alias),
 		TsConfigOverride:      validatePath(log, realFS, buildOpts.Tsconfig, "tsconfig path"),
 		MainFields:            buildOpts.MainFields,
-		Conditions:            append([]string{}, buildOpts.Conditions...),
 		PublicPath:            buildOpts.PublicPath,
 		KeepNames:             buildOpts.KeepNames,
 		InjectAbsPaths:        make([]string, len(buildOpts.Inject)),
@@ -1018,6 +1017,9 @@ func rebuildImpl(
 		WatchMode:             buildOpts.Watch != nil,
 		Plugins:               plugins,
 	}
+	if buildOpts.Conditions != nil {
+		options.Conditions = append([]string{}, buildOpts.Conditions...)
+	}
 	if options.MainFields != nil {
 		options.MainFields = append([]string{}, options.MainFields...)
 	}
@@ -1109,6 +1111,11 @@ func rebuildImpl(
 		options.Mode = config.ModeConvertFormat
 	}
 
+	// Automatically enable the "module" condition for better tree shaking
+	if options.Conditions == nil && options.Platform != config.PlatformNeutral {
+		options.Conditions = []string{"module"}
+	}
+
 	// Code splitting is experimental and currently only enabled for ES6 modules
 	if options.CodeSplitting && options.OutputFormat != config.FormatESModule {
 		log.AddError(nil, logger.Range{}, "Splitting currently only works with the \"esm\" format")

scripts/end-to-end-tests.js

@@ -6116,6 +6116,58 @@ for (const flags of [[], ['--bundle']]) {
         }
       }`,
     }),
+    test(['in.js', '--outfile=node.js', '--bundle', '--platform=browser'].concat(flags), {
+      'in.js': `import abc from 'pkg'; if (abc !== 'module') throw 'fail'`,
+      'node_modules/pkg/default.js': `module.exports = 'default'`,
+      'node_modules/pkg/module.js': `export default 'module'`,
+      'node_modules/pkg/package.json': `{
+        "exports": {
+          ".": {
+            "module": "./module.js",
+            "default": "./default.js"
+          }
+        }
+      }`,
+    }),
+    test(['in.js', '--outfile=node.js', '--bundle', '--platform=node'].concat(flags), {
+      'in.js': `import abc from 'pkg'; if (abc !== 'module') throw 'fail'`,
+      'node_modules/pkg/default.js': `module.exports = 'default'`,
+      'node_modules/pkg/module.js': `export default 'module'`,
+      'node_modules/pkg/package.json': `{
+        "exports": {
+          ".": {
+            "module": "./module.js",
+            "default": "./default.js"
+          }
+        }
+      }`,
+    }),
+    test(['in.js', '--outfile=node.js', '--bundle', '--platform=neutral'].concat(flags), {
+      'in.js': `import abc from 'pkg'; if (abc !== 'default') throw 'fail'`,
+      'node_modules/pkg/default.js': `module.exports = 'default'`,
+      'node_modules/pkg/module.js': `export default 'module'`,
+      'node_modules/pkg/package.json': `{
+        "exports": {
+          ".": {
+            "module": "./module.js",
+            "default": "./default.js"
+          }
+        }
+      }`,
+    }),
+    test(['in.js', '--outfile=node.js', '--bundle', '--conditions='].concat(flags), {
+      'in.js': `import abc from 'pkg'; if (abc !== 'default') throw 'fail'`,
+      'node_modules/pkg/default.js': `module.exports = 'default'`,
+      'node_modules/pkg/module.js': `export default 'module'`,
+      'node_modules/pkg/package.json': `{
+        "exports": {
+          ".": {
+            "module": "./module.js",
+            "default": "./default.js"
+          }
+        }
+      }`,
+    }),
   )
 
   // Node 17+ deliberately broke backward compatibility with packages using mappings