jsr-io / jsr-npm

A cli tool to make installing packages form jsr.io in node easy
https://npmjs.com/package/jsr
MIT License
102 stars 13 forks source link

CommonJS #65

Open EdJoPaTo opened 8 months ago

EdJoPaTo commented 8 months ago

Why is the tool for the ECMAScript module only registry outputting CommonJS?

I would expect it to be in ESM too, ignoring the legacy CommonJS like its registry does. When the (Node.js) runtime does not support esm in the first place JSR as a whole is useless anyway. Publishing the code is not of interest and installing from JSR wont work either.

https://github.com/jsr-io/jsr-npm/blob/1973af9b5549eaf68757312d2e224e55cdc67c06/tsconfig.json#L3

marvinhagemeister commented 8 months ago

For users of the jsr tool it doesn't matter whether it's authored in cjs or esm since it's a cli tool, not a library that you import. There isn't any particular reason other than that's the TS default and I just went with that. Happy to accept a PR that converts the codebase to npm.

EdJoPaTo commented 8 months ago

I am mostly curious why there is both CommonJS and ESM (somewhat, see #64) and not only one of them: ESM. It's just simpler to do one thing rather than multiple ones.

Personally I would just remove CommonJS completely. Going with the spirit of JSR. But it's also a library and that will be a breaking change. Are you ok with that? Then I can create a Pull Request for that.

EdJoPaTo commented 8 months ago

Looks like __dirname is used which is not available for ESM. The direct equivalent import.meta.dirname is only available since Node.js v21.

For the src/bin.ts the relative path from the working directory could be used as I would expect jsr to be run from there in the first place. But it's not completely equivalent. __dirname in the tests seems more complicated. Which is why I haven't continued to look into it further.

lishaduck commented 2 weeks ago

You can use this snippet to polyfill __dirname (specifically taken from SO, but it's the standard polyfill):

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);