gajus / lightship

Abstracts readiness, liveness and startup checks and graceful shutdown of Node.js services running in Kubernetes.
Other
518 stars 32 forks source link

Problem importing lightship #63

Closed rjbma closed 1 year ago

rjbma commented 1 year ago

In my typescript ESM project, i get the following error when trying to use import { createLightship } from 'lightship';:

Cannot find module 'lightship' or its corresponding type declarations.ts(2307)

One way to fix this would be to edit my tsconfig.json file:

"moduleResolution": "nodenext",   // or "node16"       

The problem with this is that now a lot of other dependencies in my project will start to break, particularly the ones that use default exports (see https://github.com/microsoft/TypeScript/issues/50175).

One way to solve the problem would be to add "main": "./dist/index.js" to Lightship's package.json. This way, the exports section would still take precedence, but main would make sure that environments with "moduleResolution": "node" would still work.

Would it be possible to add the main prop to pacakge.json?

mstroiu commented 1 year ago

I'm experiencing the same issue in a Next.js project

erichuang-bh commented 1 year ago

I'm experiencing the same issue in a Next.js project

same here.

rjbma commented 1 year ago

@gajus any thought on this? I'm happy to open a PR if you see fit.

gajus commented 1 year ago

(Something is wrong with GitHub notifications; I've not seen notifications about neither the original issue or the subsequent comments. Thank you for tagging me!)

@rjbma Have you confirmed that adding main indeed fixes the issue?

I was under the impression that main is only for commonjs.

All my projects are using nodenext, so this didn't surface earlier. Sorry!

rjbma commented 1 year ago

No problem whatsoever, thanks for the time you invested in this! And great job with lightship, by the way.

I'm no expert in the matter, but this SO answer seems to sums it up pretty well: both main and exports apply to ESM and CJS, however exports is the preferred way, since it's more flexible (allows to define multiple entry points).

That said, there doesn't seem to be any problem in defining both, by the contrary, it will support more environments (for example, typescript project where moduleResolution is node - which is exactly my case).

I did test with importing lightship locally with npm link in my project, and it indeed fixed the issue. As such, I'm assuming it will do the same with a regular npm install once the package is published to npm.

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 8.0.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket: