skonves / express-http-context

Get and set request-scoped context anywhere
MIT License
299 stars 26 forks source link

cls-hooked error #25

Closed joshghent closed 5 years ago

joshghent commented 5 years ago

Hi there, I'm getting an error with express-http-context v1.2.0 I'm not sure why but I'm getting the following error

node_modules/express-http-context/index.d.ts:2:27 - error TS7016: Could not find a declaration file for module 'cls-hooked'. '/home/josh/Projects/express-api/node_modules/cls-hooked/index.js' implicitly has an 'any' type.
  Try `npm install @types/cls-hooked` if it exists or add a new declaration (.d.ts) file containing `declare module 'cls-hooked';`

2 import { Namespace } from 'cls-hooked';

Could you point me in the right direction please?

skonves commented 5 years ago

Thank you for your interest in this project!

Internally, express-http-context uses cls-hooked and exposes a function that returns the Namespace type. In order for Typescript to use that type, you will need to install @types/cls-hooked as a dev dependency:

npm install --save-dev @types/cls-hooked

joshghent commented 5 years ago

@skonves No thank you for building a kick ass project! 🎉 Thanks for the fix. Should this not be a dependency of express-http-context itself though?

skonves commented 5 years ago

This project includes them as a dev dependency. However, adding the types as a (non-dev) dependency seemed a little heavy-handed, especially for non-typescript consumers.

In general, I think it's best practice to include types as dev dependencies. Maybe the confusion lies within exposing a type defined in a dev dependency. Overall, I wanted to keep the package size and dependency count as small as possible, but I definitely see how it can cause confusion.

I'll leave this issue open to see if the community has any opinions on this.

joshghent commented 5 years ago

@skonves Yes I understand the argument against including them as a dependency. But for the purpose of being able to use this module with typescript, I would say it is worth for the sake of a few bytes. There must be a better solution though. Maybe worth asking around the community

victordidenko commented 5 years ago

This error could happen only if you set noImplicitAny to true. I think, if you did that — you know what you are doing, and you can figure out, that you need @types/cls-hooked as well.

skonves commented 5 years ago

closed by #37