uuidjs/uuid

[BUG] CommonJS file with ESM syntax is exported as "module"

cdauth opened this issue · 5 comments

Before you begin...

  • I have searched the existing issues
  • I am not using version 13.x of node (if so, please upgrade)

Description of the problem

The exports in package.json currently contains this:

  "main": "./dist/index.js",
  "exports": {
    ".": {
      "node": {
        "module": "./dist/esm-node/index.js",
        "require": "./dist/index.js",
        "import": "./wrapper.mjs"
      },
      "browser": {
        "import": "./dist/esm-browser/index.js",
        "require": "./dist/commonjs-browser/index.js"
      },
      "default": "./dist/esm-browser/index.js"
    },
    "./package.json": "./package.json"
  },
  "module": "./dist/esm-node/index.js",

In a Node.js environment, the .js files are interpreted as CommonJS and the .mjs file as ESM, since no "type": "module" is defined. This means that the files in ./dist/esm-*/*.js are interpreted as CJS, even though they contain ESM code.

In a normal Node.js ESM environment, this is not a problem, since it will resolve to the .mjs file defined in the node.import export. However, there are other environments which resolve dependencies in a different way but still interpret the file extensions in the same way as Node.js.

Problem with esbuild

In particular, I'm having trouble with a project that is compiled using esbuild. According to its docs, in node mode, the modules condition is included by default, so the node.module export is used, leading to the following error:

import { v4 } from "../../node_modules/uuid/dist/esm-node/index.js";
         ^^
SyntaxError: Named export 'v4' not found. The requested module '../../node_modules/uuid/dist/esm-node/index.js' is a CommonJS module, which may not support all module.exports as named exports

As a workaround, I specified conditions: [] in my esbuild config.

Problem with jest

I'm also experiencing the same problem with Jest that is reported in #678. Testing has revealed that Jest (at least in a jsdom environment) uses the browser.import export.

Suggested solution

In other issues reported here I have seen comments about needing to add "type": "module", which would have a lot of side effects and break things. I think this is not needed, it is only needed to rename the ESM bundles to .mjs. I believe this would also make the wrapper.mjs file unnecessary.

There might be some outdated build tools that don't support the .mjs extension yet. In immerjs/immer#939, we had this problem with Metro, however the underlying issue facebook/metro#535 seems to have been fixed by now. If support for such outdated build tools should be preserved, a .js ESM file should be referenced in the module field in package.json (while a .mjs file should be in the exports.module field).

Recipe for reproducing

To reproduce the issue with esbuild, create a file test.mjs that imports uuid:

import { v4 } from 'uuid';

Then build the file by executing a Node.js script with the following content:

import { build } from 'esbuild';
import { fileURLToPath } from 'url';

await build({
    entryPoints: [fileURLToPath(new URL('./test.mjs', import.meta.url))],
    platform: 'node',
    bundle: true,
    format: 'esm',
    outfile: './out.mjs',
    external: ['./node_modules/*']
});

It is important to use a recent version of esbuild, as older versions were not using the module condition.

Additional information

No response

Environment

No response

Marking as stale due to 90 days with no activity.

Marking as stale due to 90 days with no activity.

Closing issue due to 30 days since being marked as stale.

bjmc commented

We're running into this issue with Jest tests in a jsdom test environment. Is there any interest from the maintainers in accepting a patch?

The fix suggested by @cdauth sounds like it should be relatively low impact (compared to adding "type": "module")

We're running into this issue with Jest tests in a jsdom test environment. Is there any interest from the maintainers in accepting a patch?

The fix suggested by @cdauth sounds like it should be relatively low impact (compared to adding "type": "module")

This would be helpful for us too. Alternatively switch the default export to the commonjs version.