search

philihp / openskill.js

A faster, open-license alternative to Microsoft TrueSkill
https://www.npmjs.com/package/openskill
MIT License
203 stars 19 forks source link

4.0.0 does not work in an ESM project. #619

Closed cpojer closed 4 months ago

cpojer commented 4 months ago

Hey @philihp,

I love that you are updating the project to the latest and most modern stuff. I'm using your package in Athena Crisis (open source, but the openskill usage is not open). However, it seems that 4.0.0 does not export anything correctly in order to work in an ESM only environment in Node.js. Here is a gist with a package.json and an index.js file: https://gist.github.com/cpojer/9ced8e205d55b2c363feb34ae90de7ec

When you run node index.js, it will say SyntaxError: The requested module 'openskill/dist/rate.js' does not provide a default export. The same is true for attempting to import { rate } … or other specifiers. Since you converted this module to ESM, I would suggest getting rid of all the Babel ESM to CJS conversions, and possibly just switching to esbuild for compilation. This will make the project work only with ESM, but it seems like that's what you are going for – and folks on older versions of Node can stick with openskill 3.

If you are looking for inspiration on how to set up esbuild and TypeScript within a super tiny library that is 90% boilerplate, check out @nkzw-tech/histogram. Let me know if you need any help – or if I got something wrong here and I'm doing something wrong when using openskill 4.

philihp commented 4 months ago

Oh gosh, thanks! I'll look into that and get this fixed. Thank you so much!

philihp commented 4 months ago

Should be fixed with 4.0.1! Thank you for reporting, the project is now building on esbuild. I would love to sound the death knell for CJS, but i think for compatibility reasons, I should hold off for a while.

cpojer commented 4 months ago

Thank you for fixing it, 4.0.1 works 🙇‍♂️

cpojer commented 4 months ago

Unfortunately, it seems like the ESM build is causing more trouble. When invoking rate within a testfile in an ESM context it fails with this:

CleanShot 2024-07-04 at 19 07 05@2x

This is because sort-unwind is exported in the same way as openskill 4.0.0 was, as type: "module" but without the correct ESM pieces so it cannot be imported.

philihp commented 4 months ago

Fixing

philihp commented 4 months ago

Okay, let's try this again with 4.0.2

Tests seem to work out... https://gist.github.com/philihp/bd7ed8daa7871d1922a8027e0540db90

cpojer commented 4 months ago

Seems to works now! Thank you so much 🙇‍♂️