search

robertklep / node-metrohash

Node.js bindings for MetroHash
MIT License
26 stars 8 forks source link

Add TypeScript type definitions. #4

Closed laurence-myers closed 7 years ago

laurence-myers commented 7 years ago

Adds some basic TypeScript typings. Makes it easier to user the module in TypeScript projects.

These typings would need to be kept up to date if the module's public API changes in the future.

robertklep commented 7 years ago

Thanks @laurence-myers!

I haven't really worked with TS yet, but this is a good moment to check it out.

robertklep commented 7 years ago

@laurence-myers

Question: I got it working with TS after installing @types/node. Is that something that's generally done in the project that will use metrohash as a requirement, or is more like dependencies, but only for TS-projects?

If the latter, is there a possibility to declare it a TS-specific dependency (in package.json, I assume)?

laurence-myers commented 7 years ago

Great question! My understanding is that consuming applications should install the node types as a dependency, otherwise there's a chance for a conflict between the version of node expected by this module, and the version installed in the consuming application.

If you were to use TypeScript elsewhere in this module, you could install @types/node as a dev dependency. That would fix compilation within the module (e.g. for unit tests), while still allowing consuming apps to use their own version of @types/node.

(There's a comment at the top of the typing declaration, that I believe indicates that it requires the "node" types to be present. Not sure how the compiler uses this.)

Hope that makes sense!

robertklep commented 7 years ago

@laurence-myers yes, makes perfect sense!

I just published metrohash@2.3.0 with your PR, thanks again 😃

laurence-myers commented 7 years ago

Excellent, thanks for accepting it, and thanks for the library! It's sped up my hashing by over 10x.

On 15 Jun 2017 3:58 PM, "Robert Klep" notifications@github.com wrote:

@laurence-myers https://github.com/laurence-myers yes, makes perfect sense!

I just published metrohash@2.3.0 with your PR, thanks again 😃

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/robertklep/node-metrohash/pull/4#issuecomment-308636620, or mute the thread https://github.com/notifications/unsubscribe-auth/AGCuMJGDWNPo5fZBw1aezldA_2tsppDxks5sEMfzgaJpZM4N2C6s .