fix #3367: ignore ESM flag for disabled modules

evanw · evanw · commit 58708094af0f · 2023-10-17T00:47:41.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,6 +29,10 @@
 
     In the future, CSS `url()` tokens may contain additional stuff after the URL. This is irrelevant today as no CSS specification does this. But esbuild previously had a bug where using these tokens in an `@import` rule resulted in malformed output. This bug has been fixed.
 
+* Fix `browser` + `false` + `type: module` in `package.json` ([#3367](https://github.com/evanw/esbuild/issues/3367))
+
+    The `browser` field in `package.json` allows you to map a file to `false` to have it be treated as an empty file when bundling for the browser. However, if `package.json` contains `"type": "module"` then all `.js` files will be considered ESM, not CommonJS. Importing a named import from an empty CommonJS file gives you undefined, but importing a named export from an empty ESM file is a build error. This release changes esbuild's interpretation of these files mapped to `false` in this situation from ESM to CommonJS to avoid generating build errors for named imports.
+
 * Fix a bug in top-level await error reporting ([#3400](https://github.com/evanw/esbuild/issues/3400))
 
     Using `require()` on a file that contains [top-level await](https://v8.dev/features/top-level-await) is not allowed because `require()` must return synchronously and top-level await makes that impossible. You will get a build error if you try to bundle code that does this with esbuild. This release fixes a bug in esbuild's error reporting code for complex cases of this situation involving multiple levels of imports to get to the module containing the top-level await.
diff --git a/internal/bundler_tests/bundler_packagejson_test.go b/internal/bundler_tests/bundler_packagejson_test.go
@@ -2841,3 +2841,35 @@ NOTE: You can mark the path "msw/browser" as external to exclude it from the bun
 `,
 	})
 }
+
+// See: https://github.com/evanw/esbuild/issues/3367
+func TestPackageJsonDisabledTypeModuleIssue3367(t *testing.T) {
+	packagejson_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				import foo from 'foo'
+				foo()
+			`,
+			"/package.json": `
+				{
+					"browser": {
+						"foo": false
+					}
+				}
+			`,
+			"/node_modules/foo/package.json": `
+				{
+					"type": "module"
+				}
+			`,
+			"/node_modules/foo/index.js": `
+				export default function() {}
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+		},
+	})
+}
diff --git a/internal/bundler_tests/snapshots/snapshots_packagejson.txt b/internal/bundler_tests/snapshots/snapshots_packagejson.txt
@@ -415,6 +415,19 @@ function main_browser_esm_default() {
 // Users/user/project/src/entry.js
 console.log(main_browser_esm_default());
 
+================================================================================
+TestPackageJsonDisabledTypeModuleIssue3367
+---------- /out.js ----------
+// (disabled):node_modules/foo/index.js
+var require_foo = __commonJS({
+  "(disabled):node_modules/foo/index.js"() {
+  }
+});
+
+// entry.js
+var import_foo = __toESM(require_foo());
+(0, import_foo.default)();
+
 ================================================================================
 TestPackageJsonDualPackageHazardImportAndRequireBrowser
 ---------- /Users/user/project/out.js ----------
diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go
@@ -889,6 +889,11 @@ func (r resolverQuery) finalizeResolve(result *ResolveResult) {
 				continue
 			}
 
+			// Path attributes are not taken from disabled files
+			if path.IsDisabled() {
+				continue
+			}
+
 			// Look up this file in the "sideEffects" map in the nearest enclosing
 			// directory with a "package.json" file.
 			//
