blakeembrey / change-case

Convert strings between camelCase, PascalCase, Capital Case, snake_case and more
MIT License
2.21k stars 92 forks source link

Please restore CJS support #305

Closed moltar closed 9 months ago

moltar commented 9 months ago

Don't see a technical reason not to support both ESM and CJS, given that tools like tsup make this an easy achievement.

However, if this is purely an ideological matter, then please disregard my issue and feel free to close it.

blakeembrey commented 9 months ago

It's mostly ideological. I don't want to ship two copies of everything I write.

Saying just use tsup is unfortunately naive. Bundling all the dependencies of a library instead of using dependencies results in a larger build and slower code for everyone. Say I bundle this library twice. In other libraries where I depend on this one, it also needs to bundle twice inlining those copies of change-case. If I write three packages using change-case, that's now 3 copies of change-case, and it's unable to take advantage of any features of package managers to reduce this duplication.

blakeembrey commented 9 months ago

Actually, here's a much better reply from someone else: https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c?permalink_comment_id=3850849#gistcomment-3850849

moltar commented 9 months ago

Saying just use tsup is unfortunately naive. Bundling all the dependencies of a library instead of using dependencies results in a larger build and slower code for everyone.

That's not how tsup works though. It does not bundle your deps by default. All dependencies remain external.

You can instruct it to bundle packages, but it is an opt-in configuration and is only useful in a narrow set of circumstances.

It does have a useful feature of bundling definition types from packages, but also under a flag and under certain conditions: it must be a dev dep, and you must re-export a type. Then tsup will bundle those types for you.

https://tsup.egoist.dev/#excluding-packages

By default tsup bundles all import-ed modules but dependencies and peerDependencies in your package.json are always excluded, you can also use --external <module|pkgJson> flag to mark other packages or other special package.json's dependencies and peerDependencies as external.

moltar commented 9 months ago

What tsup does by default is it bundles your source code into a single entry point file (./index.{js,mjs}).

Which IMO is desirable in most cases, unless you truly want to expose multiple entrypoints in your package. But even then, you can avoid this behaviour.

In the case of change-case, there are only two exported paths, which you can then configure in tsup to be separate entry points:

https://github.com/blakeembrey/change-case/blob/f07181487fd2ab6ed5e75d9f196ab8be0ff391b9/packages/change-case/package.json#L9-L17

blakeembrey commented 9 months ago

That's not how tsup works though. It does not bundle your deps by default. All dependencies remain external.

If you don't bundle dependencies, you can't easily get ESM and CJS to work at the same time. You are aware of that right? If I was to publish CJS and want to use something from ESM, it'll break. So I'd be restricting all dependencies to only packages that dual publish CJS and ESM. I might have well have stuck to CJS only. At least with ESM only I can use anything I want.

moltar commented 9 months ago

If you don't bundle dependencies, you can't easily get ESM and CJS to work at the same time. You are aware of that right?

Have never heard of this constraint.

Do you have a docs by chance?

blakeembrey commented 9 months ago

The link I shared to the gist above kind of touches on it, but the issue I was trying but very unclear about is that you can not use ESM from CJS. So if I was to provide a CJS build, without bundling dependencies, it would break if I ever wanted to depend on something written for ESM.

Honestly though, trying to do both is just more work than I want as a package maintainer. I only intend to maintain ESM going forward.

olsonpm commented 7 months ago

It's nice js is moving forward. Unfortunately I'm another one who enjoys the reliable old syntax so these modules do provide a hiccup. I agree supporting cjs is a non-negligible amount of effort - so if an author wants pure esm then that's how it should be :).

For anyone running into this, the quickest and most reliable solution is to obviously depend on an older version. I think this works for most cases people need.

If you want to venture into less reliable territory, I've been using fix-esm. However for change-case, fix-esm breaks due to package.json -> exports lacking a "require" property. My solution was to fork this project and add the require property.

Lately I've been creating personal repos which are simple cjs wrappers via fix-esm and it's worked alright so far. I imagine if someone wanted to create a cjs repo analagous to DefinitelyTyped they could, though supporting it would be equally daunting

blakeembrey commented 1 month ago

An update on this for anyone monitoring, but node core has been working to enable ESM in CommonJS support and it's currently under an experimental flag: https://nodejs.org/api/modules.html#loading-ecmascript-modules-using-require. Hopefully that makes the situation better for everyone. (I haven't tested whether that means this and other modules work properly yet, just saw this a while back and remembered this issue today).