You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fellow OSS contributor here, I came across your dependency while adding logging to my micro-service, I really like the scope & design that you've implemented. I can definitely see myself scaling this out to my other services if it works well with this first one!
One non-blocking item I wanted to start a conversation around was adding TypeScript Support. I'm a big TypeScript developer at this point and I do prefer packages with Type Definitions, as they provide both documentation & guards to quality-check the code that's written by myself and my team.
Adding a types definition file to the published package
Adding a types entry to your package.json manifest
I will say from personal experience, writing type definitions by hand is a pain (at least, trying to write them to cover someone else's code), but some packages are inherently more complex than others. I typically resort to converting code to TypeScript and letting the compiler autogenerate the definition file, but I do understand that's a personal preference you may not share. There may also be some tooling that can generate the definition file from the JS code itself.
An alternative way to include types is to publish a separate @types/express-request-logger package, which TypeScript devs download in addition to your package, but IMO that option is best suited for adding type support to unmaintained packages; relying on it for maintained packages just adds extra steps for both the publisher & the dev downloading the dependency.
Let me know your thoughts when you get the chance, like I said this more of a conversation starter than it is an attempt to dictate work. Depending on the outcome of the conversation, I may be open to contributing more to the final solution.
The text was updated successfully, but these errors were encountered:
Hello!
Fellow OSS contributor here, I came across your dependency while adding logging to my micro-service, I really like the scope & design that you've implemented. I can definitely see myself scaling this out to my other services if it works well with this first one!
One non-blocking item I wanted to start a conversation around was adding TypeScript Support. I'm a big TypeScript developer at this point and I do prefer packages with Type Definitions, as they provide both documentation & guards to quality-check the code that's written by myself and my team.
As far what acceptance criteria would look like for this feature request:
types
entry to your package.json manifestI will say from personal experience, writing type definitions by hand is a pain (at least, trying to write them to cover someone else's code), but some packages are inherently more complex than others. I typically resort to converting code to TypeScript and letting the compiler autogenerate the definition file, but I do understand that's a personal preference you may not share. There may also be some tooling that can generate the definition file from the JS code itself.
An alternative way to include types is to publish a separate
@types/express-request-logger
package, which TypeScript devs download in addition to your package, but IMO that option is best suited for adding type support to unmaintained packages; relying on it for maintained packages just adds extra steps for both the publisher & the dev downloading the dependency.Let me know your thoughts when you get the chance, like I said this more of a conversation starter than it is an attempt to dictate work. Depending on the outcome of the conversation, I may be open to contributing more to the final solution.
The text was updated successfully, but these errors were encountered: