KingSora / OverlayScrollbars

A javascript scrollbar plugin that hides the native scrollbars, provides custom styleable overlay scrollbars, and preserves the native functionality and feel.
https://kingsora.github.io/OverlayScrollbars
MIT License
3.83k stars 214 forks source link

Jest with Node.js v20.11.0 does not understand overlayscrollbars-react #604

Closed krutoo closed 8 months ago

krutoo commented 8 months ago

Hi, i noticed about "module" field in package.json is not standard way to define ESM entry point of package.

According this answer: https://github.com/jestjs/jest/issues/9430#issuecomment-1905771034

Can we update package.json of overlayscrollbars packages for to comply with the standard?

KingSora commented 8 months ago

Good day @krutoo

overlayscrollbars-react package.json looks like this: https://cdn.jsdelivr.net/npm/overlayscrollbars-react@0.5.3/package.json

It defines the exports, main and module fields.

krutoo commented 8 months ago

@KingSora hmmm, for some reason Jest does not understand that this file must be interpreted as an ESM

KingSora commented 8 months ago

@krutoo Would you be able to create an example repository with your jest setup and a library which works with it? I could take a look what could potentially be wrong there.

krutoo commented 8 months ago

@KingSora Here is repo with reproduction of problem: https://github.com/krutoo/jest-esm-overlayscrollbars-react

Just make npm install and npm run test

In my computer i have Node.js v20.11.0

krutoo commented 8 months ago

@KingSora sorry for wrong link, I fixed it

lpjune commented 8 months ago

Also experiencing this issue with Node 20.11

Smrtnyk commented 8 months ago

Do you need maybe "type": "module"?

KingSora commented 8 months ago

@krutoo @lpjune @Smrtnyk I've identified the issue and will publish a fix soon :)

The problem is that even though the package jsons exports field looks like this:

"exports": {
  ".": {
    "require": "./overlayscrollbars-react.cjs.js",
    "import": "./overlayscrollbars-react.es.js",
    "types": "./types/overlayscrollbars-react.d.ts"
  }
}

Jest or node is reading the overlayscrollbars-react.es.js as a commonjs file. The reason being that I have no "type": "module" field and the fileextension of that file does not end with .mjs thus causing it to be read as a commonjs file.

krutoo commented 8 months ago

@KingSora Please also note that the order of the keys matters according to the docs

Conditional exports: https://nodejs.org/api/packages.html#conditional-exports

Node.js implements the following conditions, listed in order from most specific to least specific as conditions should be defined...

Smrtnyk commented 8 months ago

if you set "type": "module" you should not need mjs as extension node will read .js files as es modules by default

KingSora commented 8 months ago

@Smrtnyk Thats true, but it would also read the commonjs files as modules in that case

ChristophP commented 8 months ago

Regaring the extensions

In that sense if you have type: module set, the ESM file can be called .js the CJS file .cjs. If you have no type module set or type: commonjs you can do .mjs for ESM and .js for CJS. If you wanna be absolutely sure regardless you can use .mjs/.cjs for them respectively.

nickserv commented 8 months ago

I'd also recommend linting your package with publint.

KingSora commented 8 months ago

@krutoo I'll continue to provide commonjs and module versions because of backwardscompatibility reasons and because some frameworks / tools simply only support one or another but not always both (or the "right" one).

@ChristophP Thank you for the input - really appreciate it!

@nickmccurdy Very useful tool, I'll definitely use it before I publish the fixed versions - Thanks a bunch!

KingSora commented 8 months ago

I've published:

Which all address this issue and should hopefully fix it properly. I've used https://publint.dev and https://arethetypeswrong.github.io/ to identifiy additional issues and fixed them where the error / warning was appropriate.

Please try it out and give feedback :)

krutoo commented 8 months ago

@KingSora Yes, it helped in example repo and in my project, thank you so mush

Can you tell me what exactly you did to fix this problem?

Issue can be closed i think

KingSora commented 8 months ago

@krutoo I've changed the package.jsons exports field.. I've described the problem here: https://github.com/KingSora/OverlayScrollbars/issues/604#issuecomment-1907577254

To see exactly how the I changed the files you can compare the old and new versions.