esbuild bundles CJS where ESM is available

[tpluscode]

Please see this gist: https://gist.github.com/tpluscode/95fa409164edbe5b6655847e7c787887

The issue I'm having is that esbuild apparently chooses CJS builds of certain dependencies where they are bundled with ESM too

[tpluscode]

The exact problem is with the module node_modules/@react-dnd/asap/dist/cjs/node/raw.js which tries to require('domain') and then node_modules/@react-dnd/asap/dist/cjs/node/asap.js breaks at runtime on process.domain

This should actually be built from node_modules/@react-dnd/asap/dist/esm/browser/* but I'm clueless why it is not

From it package.json:

{
  "browser": {
    "./asap": "./dist/esm/browser/asap.js",
    "./asap.js": "./dist/esm/browser/asap.js",
    "./raw": "./dist/esm/browser/raw.js",
    "./raw.js": "./dist/esm/browser/raw.js"
  }
}

[kzc]

$ esbuild -h | grep main
  --main-fields=...         Override the main file order in package.json
                            (default "browser,module,main" when platform is
                            browser and "main,module" when platform is node)

[tpluscode]

I don't see how you comment helps @kzc.

browser is the default platform. Thus, it should first select the browser field. I also found other packages in that bundle. where main was selected before module.

[kzc]

Dude, go easy with the negative comments and thumbs down for people who are trying to help you for free.

Compare:

$ echo 'console.log(require("react-dnd"))' | esbuild --bundle --format=esm | node --input-type=module
✘ [ERROR] Could not resolve "domain"

    node_modules/@react-dnd/asap/dist/cjs/node/raw.js:88:23:
      88 │       domain = require('domain');
         ╵                        ~~~~~~~~

  The package "domain" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "--platform=node" to do that, which will remove this error.

1 error

to:

$ echo 'console.log(require("react-dnd"))' | esbuild --bundle --format=esm --main-fields=module,browser,main | node --input-type=module
{
  DndContext: [Getter],
  DndProvider: [Getter],
  DragLayer: [Getter],
  DragPreviewImage: [Getter],
  DragSource: [Getter],
  DropTarget: [Getter],
  useDrag: [Getter],
  useDragDropManager: [Getter],
  useDragLayer: [Getter],
  useDrop: [Getter]
}

As far as the browser field goes, either there's a bug in the library packaging or in esbuild.

[kzc]

It appears that esbuild is correctly following the browser field specification and @react-dnd/asap/package.json's "browser" field is misconfigured. I think the maintainers of that module may have been mistakenly using the incompatible semantics of subpath exports within the "browser" field.

Either of the following two patches will fix the "browser" packaging in question. This behavior was verified against browserify. The second simpler patch is preferable.

--- a/node_modules/@react-dnd/asap/package.json
+++ b/node_modules/@react-dnd/asap/package.json
@@ -26,6 +26,6 @@
   "browser": {
-    "./asap": "./dist/esm/browser/asap.js",
-    "./asap.js": "./dist/esm/browser/asap.js",
-    "./raw": "./dist/esm/browser/raw.js",
-    "./raw.js": "./dist/esm/browser/raw.js"
+    "./dist/cjs/node/asap": "./dist/esm/browser/asap.js",
+    "./dist/cjs/node/asap.js": "./dist/esm/browser/asap.js",
+    "./dist/cjs/node/raw": "./dist/esm/browser/raw.js",
+    "./dist/cjs/node/raw.js": "./dist/esm/browser/raw.js"
   },

--- a/node_modules/@react-dnd/asap/package.json
+++ b/node_modules/@react-dnd/asap/package.json
@@ -26,6 +26 @@
-  "browser": {
-    "./asap": "./dist/esm/browser/asap.js",
-    "./asap.js": "./dist/esm/browser/asap.js",
-    "./raw": "./dist/esm/browser/raw.js",
-    "./raw.js": "./dist/esm/browser/raw.js"
-  },
+  "browser": "dist/esm/browser/index.js",

Either patch will allow esbuild to bundle successfully with the previously failing default --main-fields and produce code that will run on the browser platform.

But using --main-fields=module,browser,main is still a valid workaround. This is similar to rollup's node-resolve plugin whose mainFields option has a default of ["module", "main"].

[tpluscode]

All right, I understand @kzc. Thank you for a thorough explanation. And sorry about my initial negative reaction. I interpreted your comment as being passive-aggressively laconic.

I will report to @react-dnd then

[kzc]

https://github.com/kriskowal/asap/blob/b3f1c12cbd2f20047e24f5dbb42196adb7c97745/package.json#L15-L22 is correct.

react-dnd uses @react-dnd/asap:

https://github.com/react-dnd/react-dnd/search?q=asap.

https://github.com/react-dnd/asap/blob/b6bebeb73428474aefa8250ac79cf0f81fd146d9/package.json#L21-L26