-
Notifications
You must be signed in to change notification settings - Fork 209
Fix CJS imports #1969
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
Comments
@domoritz, there's currrently some issues with porting to dual builds:
We would also need to make a lot of changes to the Currently, (until node v22 makes Things were much simpler previously :/ |
Changing the exports field isn't that bad I think. We did this in apache arrow: https://cdn.jsdelivr.net/npm/[email protected]/package.json |
I think the future is ESM so another option is all ESM and anyone who needs CJS uses an old version. Or is there some high-value project that cannot update to ESM? |
Sure it isn't, but in apache-arrow the We know that ESM cannot be imported by CJS, but CJS can be imported by both, so is there any benefit to write this library in ESM? A good landmark in time to port again this library to ESM is when https://nodejs.org/api/cli.html#--experimental-require-module gets stable, so ESM/CJS gets fully interoperable in every way and writing esm/cjs starts being only a matter of taste.
The future is indeed ESM, but until node's gets full support for cjs/esm interop a lot of problems would surface with cjs/esm... My main usecase with this project is with @kitajs, which uses ts-json-schema-generator from inside a typescript language service plugin that is loaded by the tsserver runtime written in CommonsJS, and as you might imagine, there's the typescript source won't be ported to ESM in the near future. If this project removes support for CJS, I couldn't use it anymore, just like every other project that might be dependent of a tool written in CJS. |
I see. I personally only need the cli so let's switch back to cjs only until the node ecosystem works better. |
Do you have any workaround to make it works for the moment ? Can't make the basic code from the README.md working... |
Use an old version. We will fix this soon. |
Ill be working on this later today |
Do you think the ecosystem is ready for ESM now? https://nodejs.org/en/blog/release/v23.0.0#requireesm-is-now-enabled-by-default seems to do what was missing. |
Node odd releases are non-stable. I'd wait until node 24 enters LTS. |
Okay. Let's leave this package as is for now then. |
Uh oh!
There was an error while loading. Please reload this page.
Hey guys so unfortunately I think this still doesn't work. Or it's also entirely possible I'm doing something wrong but, whenever I try to import something from
ts-json-schema-generator
I get this error:Sorry if this isn't the best place to discuss, lmk if I should open a new issue or what the best thing is to do.
Originally posted by @sacummings91 in #1964 (comment)
The text was updated successfully, but these errors were encountered: