Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ToshihitoKon opened this issue May 18, 2023 · 30 comments
Closed

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

ToshihitoKon opened this issue May 18, 2023 · 30 comments

Comments

@ToshihitoKon
Copy link

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
Copy link
Owner

ai commented May 18, 2023

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 testcd 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
Copy link
Author

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
Copy link
Owner

ai commented May 18, 2023

Could you reproduce an error in a small repo?

@ToshihitoKon
Copy link
Author

I'll try. Give me some time.

@ToshihitoKon
Copy link
Author

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
Copy link

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

@ai
Copy link
Owner

ai commented May 18, 2023

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

@PPRobitaille
Copy link

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

@tudorie-mariuscosmin
Copy link

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

@sgarner
Copy link

sgarner commented May 27, 2023

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
Copy link
Owner

ai commented May 27, 2023

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
Copy link

sgarner commented May 27, 2023

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
Copy link
Owner

ai commented May 27, 2023

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

@ToshihitoKon
Copy link
Author

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

@tonynechar
Copy link

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

@ai
Copy link
Owner

ai commented Jun 4, 2023

@tonynechar do not plan to stop support soon

@tonynechar
Copy link

awesome! thanks @ai

@jsw
Copy link

jsw commented Jun 29, 2023

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
Copy link

bradennapier commented Jul 3, 2023

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
Copy link

The types declaration files need to be split for TypeScript to detect that it is a dual package. See: microsoft/TypeScript#50466 (comment)

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
Copy link

gregg-cbs commented Sep 21, 2023

I am having this issue on "^5.0.1"

@zirkelc
Copy link

zirkelc commented Sep 27, 2023

@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
Copy link

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

nickcernis added a commit to getflywheel/local-components that referenced this issue Oct 13, 2023
When linking local-components for development use, our use of nanoid causes
the error below, preventing the app from loading.

Our options to fix this seem to be to:

1. Downgrade from nanoid 5 to 3: ai/nanoid#422 (comment)

2. Refactor to use a dynamic import (potentially infecting all
call sites with `await` and refactoring to make parent functions async).

3. Switch from "module": "commonjs" to "module": "ESNext" and assess the impact of that change.

4. Remove nanoid and create a simple randomId function instead.

I went with (4).

We don't need the full hardware-random IDs that nanoid provides, and we
only used nanoid in one component.

nanoid error in getflywheel-local after running nps components.link and nps.build, then opening the app:

 require() of ES Module /Users/nick.cernis/localdev/local-components/node_modules/nanoid/index.js from /Users/nick.cernis/localdev/local-components/dist/index.js not supported.
Instead change the require of /Users/nick.cernis/localdev/local-components/node_modules/nanoid/index.js in /Users/nick.cernis/localdev/local-components/dist/index.js to a dynamic import() which is available in all CommonJS modules.
nickcernis added a commit to getflywheel/local-components that referenced this issue Oct 13, 2023
When linking local-components for development use, our use of nanoid causes
the error below, preventing the app from loading.

Our options to fix this seem to be to:

1. Downgrade from nanoid 5 to 3: ai/nanoid#422 (comment)

2. Refactor to use a dynamic import (potentially infecting all
call sites with `await` and refactoring to make parent functions async).

3. Switch from "module": "commonjs" to "module": "ESNext" and assess the impact of that change.

4. Remove nanoid and create a simple randomId function instead.

I went with (4).

We don't need the full hardware-random IDs that nanoid provides, and we
only used nanoid in one component.

nanoid error in getflywheel-local after running nps components.link and nps.build, then opening the app:

 require() of ES Module /Users/nick.cernis/localdev/local-components/node_modules/nanoid/index.js from /Users/nick.cernis/localdev/local-components/dist/index.js not supported.
Instead change the require of /Users/nick.cernis/localdev/local-components/node_modules/nanoid/index.js in /Users/nick.cernis/localdev/local-components/dist/index.js to a dynamic import() which is available in all CommonJS modules.
@myftija
Copy link

myftija commented Oct 23, 2023

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

@ai
Copy link
Owner

ai commented Oct 23, 2023

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

@myftija
Copy link

myftija commented Oct 24, 2023

@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
Copy link
Owner

ai commented Nov 6, 2023

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

@ai ai closed this as completed Nov 6, 2023
@4skinSkywalker
Copy link

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

@mhamann
Copy link

mhamann commented Dec 12, 2023

@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
Copy link
Owner

ai commented Dec 12, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests