search

loathers / libram

libram is a Typescript library that intends to provide comprehensive support for automating KoLmafia
https://loathers.github.io/libram
9 stars 24 forks source link

Fully switch to ESM #609

Closed oxc closed 5 months ago

oxc commented 5 months ago

Note: these changes only apply to libram as a npm package. It should not affect the release built for kolmafia git command.

Since we export files in ESM format, the package.json should include this information by setting type: module, otherwise consumers will try to import the files as CJS.

Furthermore, I cannot find documentation on a field called "module" in a quick search. I assume this was intended to have the affect of "type: module, main: dist/index.js".

These changes together are needed to make my tests not break on finding libram, so this is a good indicator that they are correct :)

gausie commented 5 months ago

module was an old proposal for how to do this, but has since been removed. It's supported by most package managers for backwards compat. See https://esbuild.github.io/api/#main-fields and https://stackoverflow.com/questions/42708484/what-is-the-module-package-json-field-for

oxc commented 5 months ago

That explains why the tests fail, because node does not support it.

I built garbo without and with this change, and this is the diff I get in the output:

image

oxc commented 5 months ago

Ok, this breaks too much of the tool chain.

gausie commented 5 months ago

Are you planning on continuing work for this? I can look if not

oxc commented 5 months ago

It seems more viable to only switch the published package.json to type: module, and keep the "development" package on CJS.

Changing the package.json in the repository breaks a lot of commands, especially ts-node for ./tools/parseItemSkillNames.ts etc.

It seems to work out of the box with tsx, but I have no experience with that tool. Also, __dirname does not exist on ESM, so that would require some code adjustment as well.

If you say switching to full-fledged ESM is something we want to strive for, I'm willing to invest some more time. Otherwise I would have abandoned this for now, and kept on monkey patching libram for my test runs.

gausie commented 5 months ago

I think going for fully fledged ESM is worth doing, and all these fixes are relatively easy to do. Feel free to keep trying, or I can take a look after baby bedtime tonight.

oxc commented 5 months ago

Ok, I'll give it another try and report back.

oxc commented 5 months ago

This set of changes seems to do it. With the weird loader hack in package.json I don't need to replace ts-node with tsx (so I could actually also get rid of the esbuild upgrade, if you want to keep that separate).