🐛 BUG: esbuild will incorrectly disregard exports if browser/module/main are defined.

[balupton]

Ran into this against the daet and start-of-week packages.

I've setup a proof of failure repo at:

https://github.com/balupton/astro-cfpages-esbuild-resolution-bug

> npm install
> npx wrangler pages dev public/
🚧 'wrangler pages <command>' is a beta command. Please report any issues to https://github.com/cloudflare/workers-sdk/issues/new/choose
▲ [WARNING] No compatibility_date was specified. Using today's date: 2023-02-27.

  Pass it in your terminal:
  --compatibility-date=2023-02-27
  See https://developers.cloudflare.com/workers/platform/compatibility-dates/ for more information.


Compiling worker to "/var/folders/3v/fjmy4fyx1p9c28v31vzm8j9r0000gn/T/functionsWorker-0.773551694099083.mjs"...
✨ Compiled Worker successfully
╭─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ [b] open a browser, [d] open Devtools, [l] turn off local mode, [c] clear console, [x] to exit                                                                                  │
╰─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
Debugger listening on ws://127.0.0.1:55956/5cee4c70-78a6-4b16-be7e-69b1d6b359fd
For help, see: https://nodejs.org/en/docs/inspector
Debugger attached.
[pages:wrn] Service bindings are experimental. There may be breaking changes in the future.
[pages:err] ReferenceError: window is not defined
    at browser (/private/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/conditional-resolution-package/browser.mjs:2:2)
    at /private/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/functions/[[catchall]].tsx:3:16
    at SourceTextModule.evaluate (node:internal/vm/module:225:23)
    at VMScriptRunner.runAsModule (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/runner-vm/src/index.ts:40:18)
    at VMScriptRunner.run (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/runner-vm/src/index.ts:86:17)
    at Miniflare.#reload (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/core/src/index.ts:790:13)
    at Miniflare.getPlugins (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/core/src/index.ts:1033:5)
    at createServer (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/http-server/src/index.ts:362:19)
    at startServer (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/http-server/src/index.ts:469:18)
    at main (file:///Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/wrangler/miniflare-dist/index.mjs:6300:22)

/private/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/conditional-resolution-package/browser.mjs:2
	if (
 ^
ReferenceError: window is not defined
    at browser (/private/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/conditional-resolution-package/browser.mjs:2:2)
    at /private/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/functions/[[catchall]].tsx:3:16
    at SourceTextModule.evaluate (node:internal/vm/module:225:23)
    at VMScriptRunner.runAsModule (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/runner-vm/src/index.ts:40:18)
    at VMScriptRunner.run (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/runner-vm/src/index.ts:86:17)
    at Miniflare.#reload (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/core/src/index.ts:790:13)
    at Miniflare.getPlugins (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/core/src/index.ts:1033:5)
    at createServer (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/http-server/src/index.ts:362:19)
    at startServer (/Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/@miniflare/http-server/src/index.ts:469:18)
    at main (file:///Users/balupton/Projects/websites/astro-cfpages-esbuild-resolution-bug/node_modules/wrangler/miniflare-dist/index.mjs:6300:22)
Waiting for the debugger to disconnect...                           

Which is using:

  "exports": {
    "workerd": {
      "import": "./workerd.mjs",
      "require": "./workerd.cjs"
    },
    "node": {
      "import": "./node.mjs",
      "require": "./node.cjs"
    },
    "browser": {
      "import": "./browser.mjs",
      "require": "./browser.cjs"
    },
    "default": {
      "import": "./default.mjs",
      "require": "./default.cjs"
    },
    "import": "./default.mjs",
    "require": "./default.cjs"
  },
  "type": "module",
  "module": "default.mjs",
  "browser": "browser.mjs",
  "main": "default.cjs",

The only way to get it to correctly use the workerd import is to change it to:

  "exports": {
    "workerd": {
      "import": "./workerd.mjs",
      "require": "./workerd.cjs"
    },
    "node": {
      "import": "./node.mjs",
      "require": "./node.cjs"
    },
    "browser": {
      "import": "./browser.mjs",
      "require": "./browser.cjs"
    },
    "default": {
      "import": "./default.mjs",
      "require": "./default.cjs"
    },
    "import": "./default.mjs",
    "require": "./default.cjs"
  },
  "type": "module",

Which is a removal of these:

  "module": "default.mjs",
  "browser": "browser.mjs",
  "main": "default.cjs",

Which would drop support for things like browserify and older node versions - which @bevry has been supporting successfully for its hundreds of packages (half a billion installs a month) for a very long time, by using boundation and editions to ensure the correct environment always loads the best edition for it.

The solution is to prefer exports over the legacy fields, which is what Node.js does. The ideal would be to support editions, however since @bevry's proposal and successful implementation in 2016, the industry has continued doing its own thing, so my hopes for wider adoption of Editions has been wishful thinking. The current behaviour (preferring legacy fields over exports) means the @bevry suite of packages do not function as intended for esbuild users, which is a significant impact, and having @bevry conform to the esbuild behaviour would have a far more significant negative impact as it would break our seamless support for legacy node and other bundlers.

Cross-posted at cloudflare/workers-sdk#2805

[evanw]

I believe node's new module resolution algorithm that handles exports is supposed to only work for packages inside of node_modules directories. Specifically:

PACKAGE_RESOLVE(packageSpecifier, parentURL)

...
4. If packageSpecifier does not start with "@", then
    1. Set packageName to the substring of packageSpecifier until the first
      "/" separator or the end of the string.
...
7. Let packageSubpath be "." concatenated with the substring of
    packageSpecifier from the position at the length of packageName.
...
11. While parentURL is not the file system root,
    1. Let packageURL be the URL resolution of "node_modules/"
        concatenated with packageSpecifier, relative to parentURL.
    2. Set parentURL to the parent folder URL of parentURL.
    3. If the folder at packageURL does not exist, then
        1. Continue the next loop iteration.
    4. Let pjson be the result of READ_PACKAGE_JSON(packageURL).
    5. If pjson is not null and pjson.exports is not null or
        undefined, then
        1. Return the result of PACKAGE_EXPORTS_RESOLVE(packageURL,
          packageSubpath, pjson.exports, defaultConditions).
...

The problem is that you have a tsconfig.json file with "baseUrl": "." that tells esbuild to pretend that the import of conditional-resolution-package is a relative path instead (e.g. ./conditional-resolution-package). But then node's exports field isn't applied in that case (which is intentionally part of node's design for the exports feature).

You should remove the "baseUrl": "." field in your tsconfig.json file so that esbuild loads conditional-resolution-package from node_modules/conditional-resolution-package instead.

[balupton]

@evanw okay, will try that now

[balupton]

@evanw so brilliant news, that fixed it. What strange behaviour.

I've updated my actual project to change baseUrl to the source directory instead, which also resolves the issue.