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

CommonJS #65

Open
EdJoPaTo opened this issue Mar 16, 2024 · 4 comments
Open

CommonJS #65

EdJoPaTo opened this issue Mar 16, 2024 · 4 comments
Labels
question Further information is requested

Comments

@EdJoPaTo
Copy link
Contributor

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.

"module": "CommonJS",

@marvinhagemeister
Copy link
Collaborator

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.

@marvinhagemeister marvinhagemeister added the question Further information is requested label Mar 16, 2024
@EdJoPaTo
Copy link
Contributor Author

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

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

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);

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

No branches or pull requests

3 participants