freshgum-bubbles / typedi

Elegant Dependency Injection in TypeScript and JavaScript.
http://typedi.js.org/
MIT License
53 stars 3 forks source link

CommonJS Support #166

Closed MarcosAlvesTJr closed 6 months ago

MarcosAlvesTJr commented 7 months ago

Is your question related to a contrib/ feature? No

Question Is this library suppose to work with CommonJS modules at all? I have a Typescript project that builds to CommonJS and I'm having trouble to make it work with TypeDI++. I see that in node_modules a cjs directory is present, but I'm still confused.

freshgum-bubbles commented 6 months ago

Hello,

Could you elaborate upon the issues you experienced, and possibly provide a repro?

In general I'll admit that packaging isn't my strongest point (#165), but I may be able to solve basic issues like these with something like publint... Hmm.

freshgum

freshgum-bubbles commented 6 months ago

Coming back to my notes on this, the CommonJS support is more of a hold-over from upstream.

I believe this is a choice I made a while back: having just one module format and sticking to it is way easier than juggling two (upstream has four: CJS, ESM, ES2015, and UMD).

There's also the Dual Package Hazard that I'm painfully aware of: I'd hate for someone to spend hours debugging why certain symbols don't resolve in their container, when it was just down to them importing the package in a slightly different way. (I suppose I could add a require field to each of the exports. The only possible problem here is that the build:cjs task emits CommonJS files as .mjs, as opposed to .cjs.)

Could I ask why you want CommonJS support in the first place? ESM has been well-supported in Node for years. Are you using an alternative runtime, or just a very very old Node?

MarcosAlvesTJr commented 6 months ago

@freshgum-bubbles Hey, I had this error:

require() of ES Module /var/task/node_modules/@freshgum/typedi/build/esm5/entry/index.mjs not supported.\nInstead change the require of /var/task/node_modules/@freshgum/typedi/build/esm5/entry/index.mjs to a dynamic import() which is available in all CommonJS modules.

CommonJS is not a hard requirement for me, I just find the whole Typescript/Node/ESM integration a bit annoying (imports with .js extensions in Typescript files e.g.). I have switched to using ESM and found workarounds for the annoying parts. The package works smoothly now :)

freshgum-bubbles commented 6 months ago

CommonJS is not a hard requirement for me, I just find the whole Typescript/Node/ESM integration a bit annoying

Hehe, join the club -- the whole migration is still a disaster to figure out. Been writing JavaScript for 6 years and I still don't get it ;-)

(imports with .js extensions in Typescript files e.g.)

Well to try and avert that, I've specifically made use of .mts files (which emit corresponding .mjs files). It's my understanding that, if Node sees an import for .mjs, it doesn't default to CommonJS, but instead knows it's ESM. Something like that, anyway...

The package works smoothly now :)

Woohoo! Thanks for the UX feedback too -- I'll be sure to add info re: this to the README, as I totally get how annoying this must have been.

github-actions[bot] commented 5 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.