ai / nanoid

A tiny (124 bytes), secure, URL-friendly, unique string ID generator for JavaScript
https://zelark.github.io/nano-id-cc/
MIT License
24.33k stars 788 forks source link

nanoid@3(3.3.6) doesn't work with CommonJS #422

Closed ToshihitoKon closed 10 months ago

ToshihitoKon commented 1 year ago

Running npm install nanoid@3 will install v3.3.6 https://www.npmjs.com/package/nanoid/v/3.3.6

v3.3.6 is not listed in the repository tags, so I assume it is v4 and does not support CommonJS. I have confirmed that if I specify 3.3.5 in package.json, it works with CommonJS.

Can you please remove 3.3.6 from npm?

The following is an error when "nanoid":"^3.3.6" is specified

error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("nanoid")' call instead.
  To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/project/path/package.json'.
ai commented 1 year ago

Very strange. We can check npm package content by https://www.npmjs.com/package/nanoid/v/3.3.6?activeTab=code and it has all CommonJS files.

I tested 3.3.6 and it works:

❯ mkdir test
❯ cd test
❯ npm install nanoid@3
❯ echo "console.log(require('nanoid').nanoid())" > index.js
❯ node index.js 
NOTSDu0T64EyUDSCZB7a9
  1. Check your Node.js version. Some versions had issues with dual CJS/ESM packages.
  2. Check your lock file, maybe you have 2 versions of Nano ID?
ToshihitoKon commented 1 year ago

Thanks for the reply. Sorry, I don't see tag:3.3.6 and I thought it was a mistake.

Here's the node version and package-lock.json

 % node -v
v18.10.0

% cat package-lock.json| grep nanoid -A1 -B1
                "mysql": "npm:mysql2@^3.2.4",
                "nanoid": "3.3.6",
                "nestjs-redis": "^1.3.3",
--
        },
        "node_modules/nanoid": {
            "version": "3.3.6",
            "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.6.tgz",
            "integrity": "sha512-BGcqMMJuToF7i1rt+2PWSNVnWIkGCU78jBG3RxO/bZlnZPK2Cmi2QaffxGO/2RvWi9sL+FAiRiXMgsyxQ1DIDA==",
--
            "bin": {
                "nanoid": "bin/nanoid.cjs"
            },
--
        },
        "nanoid": {
            "version": "3.3.6",
            "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.6.tgz",
            "integrity": "sha512-BGcqMMJuToF7i1rt+2PWSNVnWIkGCU78jBG3RxO/bZlnZPK2Cmi2QaffxGO/2RvWi9sL+FAiRiXMgsyxQ1DIDA=="
ai commented 1 year ago

Could you reproduce an error in a small repo?

ToshihitoKon commented 1 year ago

I'll try. Give me some time.

ToshihitoKon commented 1 year ago

Reproduced in a smaller configuration https://github.com/ToshihitoKon/nanoid-v3-issue

Forgot to report, we are using TypeScript.

nanoid:3.3.5

 % npm run start

> test@1.0.0 start
> tsc && node build/index.js

Ns2M7ND8b8uPoV6EV4mb4

nanoid:3.3.6

 % npm run start

> test@1.0.0 start
> tsc && node build/index.js

index.ts:1:24 - error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("nanoid")' call instead.
  To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/kon-toshihito/tmp/nanoid/package.json'.

1 import { nanoid } from 'nanoid';
                         ~~~~~~~~

Found 1 error in index.ts:1
PPRobitaille commented 1 year ago

Got the same issue with v4.0.2 using typescript too.

ai commented 1 year ago

@PPRobitaille Nano ID 4 doesn’t support CommonJS. You need to downgrade to 3.x if your TS compiles to CJS code.

PPRobitaille commented 1 year ago

I'll probably move away from CommonJS instead.. Thanks for the quick reply ;)

tudorie-mariuscosmin commented 1 year ago

Got the same issue with V3.3.6 when using ts-node too

sgarner commented 1 year ago

I'm experiencing the same issue here with v3.3.6 (error TS1479). Downgrading to v3.3.5 resolves it.

Looking in the package contents on npm for v3.3.6 we can see that the package.json contains "type": "module", which should not be expected for a 3.x release?

ai commented 1 year ago

package.json contains "type": "module", which should not be expected for a 3.x release?

It should. 3.x branch is dual CJS+ESM package. We are using ESM by default (this is why we have "type": "module") and CJS versions are in .cjs files (and marked as CJS in "exports").

Note, that 3.3.5 was broken (I forgot to call dual CJS+ESM package convertor during the release) and has no ESM support at all. The previous working release is 3.3.4 (which has "type": "module" too).

Very likely that you have a problem in TS config, which leads to issues with dual CJS+ESM packages (I honestly stopped to believe in creating dual CJS+ESM packages by issues like this, ESM-only is the only stable way for the future).

sgarner commented 1 year ago

Note, that 3.3.5 was broken (I forgot to call dual CJS+ESM package convertor during the release) and has no ESM support at all. The previous working release is 3.3.4 (which has "type": "module" too).

Ohh, I see 🤔

Very likely that you have a problem in TS config, which leads to issues with dual CJS+ESM packages (I honestly stopped to believe in creating dual CJS+ESM packages by issues like this, ESM-only is the only stable way for the future).

Yes, I had been trying to convert my project to ESM but gave up when there were too many issues with other dependencies.

I missed the fact I still had "moduleResolution": "NodeNext" in my tsconfig. After correcting this back to "Node", my build is working with nanoid v3.3.6 now. Thanks

ai commented 1 year ago

Do you want you send PR to docs adding NodeNext incompatibility note to 3.x section?

ToshihitoKon commented 1 year ago

I am sorry, but I am not confident in my English. I would like to leave it to others.

tonynechar commented 1 year ago

Will there be long term support for the CommonJS nanoid@3 package? @ai

ai commented 1 year ago

@tonynechar do not plan to stop support soon

tonynechar commented 1 year ago

awesome! thanks @ai

jsw commented 1 year ago

I believe I have come across the same issue. I already had nanoid 3.3.6 installed and working, but upon upgrading @tsconfig/node18 2.0.1 to 18.2.0 I receive the following:

The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("nanoid")' call instead.
  To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/Users/jsw/Documents/projects/..../package.json'.

My tsconfig.json looks like:

{
  "extends": "@tsconfig/node18/tsconfig.json",
  "compilerOptions": {
    "declaration": true,
    "removeComments": true,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "allowSyntheticDefaultImports": true,
    "sourceMap": true,
    "outDir": "./dist",
    "baseUrl": "./",
    "incremental": true,
    "resolveJsonModule": true
  }
}

I import nanoid like so:

import { customAlphabet } from 'nanoid';

This is a NestJS application.

bradennapier commented 1 year ago

Same issue here when upgrading to 3.3.6 and using the base config that is recommended for commonjs typescript so this will be very common issue.

Note this seems to become an issue due to typescript 5.2 beta requiring using Node16 moduleResolution when seeing module to Node16 (whereas before it owuld work if you set moduleResolution to node which is no longer possible).

https://devblogs.microsoft.com/typescript/announcing-typescript-5-2-beta/#module-and-moduleresolution-must-match-under-recent-node-js-settings

A solution may be to have somthing like import { nanoid } from 'nanoid/cjs' or import { nanoid } from 'nanoid/nanoid.cjs' although im not 100% on the specifics of how all the interop works.

hfhchan-plb commented 1 year ago

The types declaration files need to be split for TypeScript to detect that it is a dual package. See: https://github.com/microsoft/TypeScript/issues/50466#issuecomment-1346916117

See also: https://www.typescriptlang.org/docs/handbook/esm-node.html#packagejson-exports-imports-and-self-referencing

It’s important to note that the CommonJS entrypoint and the ES module entrypoint each needs its own declaration file, even if the contents are the same between them. Every declaration file is interpreted either as a CommonJS module or as an ES module, based on its file extension and the "type" field of the package.json, and this detected module kind must match the module kind that Node will detect for the corresponding JavaScript file for type checking to be correct. Attempting to use a single .d.ts file to type both an ES module entrypoint and a CommonJS entrypoint will cause TypeScript to think only one of those entrypoints exists, causing compiler errors for users of the package.

i.e. The file ./index.d.ts should be cloned to ./index.d.cts and ./index.browser.d.ts, and then change:

    ".": {
      "types": "./index.d.ts",
      "browser": "./index.browser.js",
      "require": "./index.cjs",
      "import": "./index.js",
      "default": "./index.js"
    },

to

    ".": {
      "browser": "./index.browser.js",
      "require": "./index.cjs",
      "import": "./index.js",
      "default": "./index.js"
    },
gregg-cbs commented 11 months ago

I am having this issue on "^5.0.1"

zirkelc commented 11 months ago

@hfhchan-plb I applied those changes in a branch https://github.com/zirkelc/nanoid/tree/fix/442-types

However, TypeScript still complains about. Could you try to install nanoid from my branch and verify if it works for you?

gregg-cbs commented 11 months ago

i ended up switching from commonjs to modules for many more reasons than this and so i went around the problem.

myftija commented 10 months ago

Are there any plans to support Node16 module resolution in the 3.x.x versions?

ai commented 10 months ago

@myftija please send a PR if you know how to fix it.

myftija commented 10 months ago

@ai I investigated a bit and found a fix. The issue is in the package.json exports. It seems that the require and import conditions should have separate type files for typescript to be able to resolve CJS modules correctly (reference), i.e.:

"exports": {
  ".": {
    "browser": "./index.browser.js",
    "require": {
      "types": "./index.d.cts",
      "default": "./index.cjs"
    },
    "import": {
      "types": "./index.d.ts",
      "default": "./index.js"
    },
    "default": "./index.js"
  }
}

instead of the current version:

"exports": {
  ".": {
    "types": "./index.d.ts",
    "browser": "./index.browser.js",
    "require": "./index.cjs",
    "import": "./index.js",
    "default": "./index.js"
  },
}

Note also the .d.cts extension for the types file.

It would be a trivial fix, but since the exports field is being generated by dual-publish on module publishing, it's a bit more involved. I opened a PR on the dual-publish repo to fix this. Tested locally and on the example from @ToshihitoKon and it works well. Let's continue the discussion on the PR.

ai commented 10 months ago

The fix by @myftija was released in Nano ID 3.3.7. Thanks for great investigation and fix.

4skinSkywalker commented 10 months ago

I've had to downgrade to 3.3.5, this mess with different module technology is really annoying

mhamann commented 9 months ago

@ai I tried v3.3.7 and it seems to work with moduleResolution: "NodeNext", which is great! Can you apply this fix to the 5.x branch too?

ai commented 9 months ago

@mhamann open new issue for 5.x and explain the problem with more details.