Skip to content

Commit

Permalink
fix #3367: ignore ESM flag for disabled modules
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Oct 17, 2023
1 parent d5f397f commit 5870809
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
32 changes: 32 additions & 0 deletions internal/bundler_tests/bundler_packagejson_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
})
}
13 changes: 13 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_packagejson.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 ----------
Expand Down
5 changes: 5 additions & 0 deletions internal/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down

0 comments on commit 5870809

Please sign in to comment.