search

liberation-data / drivine

Best and fastest graph database client (Neo4j, AgensGraph, Apache AGE, FalkorDB) for Node.js & TypeScript.
https://drivine.org
Apache License 2.0
151 stars 32 forks source link

dependency cls-hooked won't resolve #66

Open Bastianowicz opened 3 years ago

Bastianowicz commented 3 years ago

hi. since me updating npm to version 7 i am facing serious issues with your dependency of cls-hooked. in my local dev environment everything is fine but in CI this dependency won't be resolved since npm cannot access it without an ssh key. this is because npm resolves the dependency as git+ssh://git@github... since the update. so my question is: what is the reason for using the unreleased version 4.3.0 of cls-hooked and not just depending on the npm released 4.2.2?

jasperblues commented 3 years ago

@Bastianowicz So sorry for the slow reply! I didn't see your issue until now.

The forked version of cls-hooked is to fix a number of issues with the base version. There's a pull request, I'll need to check if it is merged yet. If not we can publish a packaged fork that can be pulled (with http) from npm.

On version of node that support it async-local-storage is used in place of cls-hooked.

Alternatively we might mark it as an optional dependency. Are you still facing trouble?

Bastianowicz commented 3 years ago

I patched it by resolving it explicitly and adding it as my dependency in package.json so I could define what protocol to use. So it doesn't bother me at the moment but it is an unused dependency 😉

jasperblues commented 3 years ago

OK, noted. Thanks @Bastianowicz. Sorry again for the slow reply!

Bastianowicz commented 3 years ago

Don't worry. If I couldn't had fixed it I would've been more annoying 😉

nartc commented 3 years ago

Ran into the same issue here :(. Was just removing node_modules and try to reinstall it then this error happens.

jasperblues commented 3 years ago

cls-hooked is usually not needed. A good solution would be to mark it as optional or external and listed in docs.

dotrunghieu2151 commented 2 years ago

@jasperblues, Could you please mark cls-hooked as an optional dependency ? I intend to run my app inside a docker container so it's not possible for me to manually resolve it in package-lock.json.

jasperblues commented 2 years ago

@dotrunghieu2151 Could you send a PR pls? I will push a new npm build as soon as received.