typescript definitions

[purplemana]

Hi,

Are there any existing ts definitions for this project?

Thanks!

[royaltm]

I did not created any. Do you want to contribute?

[purplemana]

I have created a partial one, works for my use case. Here it is:

declare module "murmurhash-native/stream" {
  import { Transform } from "stream";
  export function createHash(hashType: string, options: any): Transform;
}

If you like I can submit this as a PR, but it's far from complete.

[royaltm]

Could you create a minimal ts gist that contains the declaration and some simple test case in ts? I'm not familiar with ts and I don't have time to learn from scratch now. However with some starting point I'd be able to extend it and complete the declarations.

[purplemana]

Here you go...

https://gist.github.com/purplemana/7a15df3cceaf9cccda06b68c9f3ffe4f

I am using this in my repo https://github.com/purplemana/alchemist/blob/master/packages/api-server/src/core-fs/DiskFS.ts

[royaltm]

Thanks, that'll help!

[royaltm]

@purplemana what is the good/common practice of storing such declarations for non-typescript modules? Should I apply PR to DefinitelyTyped project, or rather I could put the ts files somewhere in the murmurhash-native module?

[purplemana]

Best you refer to https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html for the definitive answer. However, in general having declarations within the npm package itself has the benefit that consumers don't need to separately install @types/packagename. If you want to ship the declaration with the package, all you gotta do is put your package.d.ts file in a folder typically named as types, and add "types":"./types/package.d.ts" entry in package.json. You must include this folder in the published content.

As my gist only has those parts which I needed, you would need to build a more complete definition file. Take a look at https://medium.com/@jonjam/getting-started-with-typescript-type-definitions-1cda7094b8d2 which talks about tools which can help with this.

[Mikhus]

This package seems using defaull main file name which is index.js. Due to my experience it is enough just to put index.d.ts with type definitions and it works fine, at least with the latest typescript versions...

[Mikhus]

I'm not very familiar with the interface native bindings should expose, BTW here is an idea of possible type definitions as the most simple way I could imagine: https://github.com/Mikhus/node-murmurhash-native/tree/types Especially this: https://github.com/Mikhus/node-murmurhash-native/blob/types/index.d.ts and this: https://github.com/Mikhus/node-murmurhash-native/blob/types/stream.d.ts Correct me if I'm wrong or on a good track. If it is something satisfying I can continue work on it or someone could help me proceed with that - any wise help is welcomed...

[royaltm]

Thanks @Mikhus. I'm already on this here: https://github.com/royaltm/node-murmurhash-native/blob/types/index.d.ts My goals are:

• cover 100% api
• make it flexible (using interfaces where applicable) and strict at the same time (avoid any type). Perhaps I'm yet a little clumsy with ts. Any wisdom from knowledgeable TypeScript enthusiasts will be appreciated :)

[royaltm]

I like the names of your interfaces more. Perhaps I could use them?

[purplemana]

Hope this helps...

• https://github.com/royaltm/node-murmurhash-native/blob/types/index.d.ts#L324: To reuse MurmurHash class definition from incremental:

  declare module "stream" {
  import {MurmurHash} from "incremental";
  ....

• https://github.com/royaltm/node-murmurhash-native/blob/types/index.d.ts#L325: Privates are typically omitted in type definitions as they don't serve any purpose.

• https://github.com/royaltm/node-murmurhash-native/blob/types/index.d.ts#L308: if MurmurHash is completely type equal with Transform, prefer to return Transform, unless you want different documentation for the methods/params to be used by the IDE.

[purplemana]

Node types are a great reference too: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/node/v9/index.d.ts

[royaltm]

• https://github.com/royaltm/node-murmurhash-native/blob/types/index.d.ts#L324: To reuse MurmurHash class definition from incremental:

  declare module "stream" {
  import {MurmurHash} from "incremental";
  ....

I was getting a strange error from tsc when trying to do that so I moved the "incremental" and "stream" declarations to its own files. But probably for the better as native incremental is a separate module anyway.

• https://github.com/royaltm/node-murmurhash-native/blob/types/index.d.ts#L325: Privates are typically omitted in type definitions as they don't serve any purpose.

Even when one would try to extend the class in ts?

• https://github.com/royaltm/node-murmurhash-native/blob/types/index.d.ts#L308: if MurmurHash is completely type equal with Transform, prefer to return Transform, unless you want different documentation for the methods/params to be used by the IDE.

MurmurHash is an extension of Transform, it has bi-api interface with its own methods - with stream-like API and update/digest methods similar to node's crypto hashers and has serialization ability.

[purplemana]

Even when one would try to extend the class in ts?

privates are not available to extended classes, only protected and public are inherited.

MurmurHash is an extension of Transform, it has bi-api interface with its own methods - with stream-like API and update/digest methods similar to node's crypto hashers and has serialization ability.

It makes sense to keep it then.

[Mikhus]

I like the names of your interfaces more. Perhaps I could use them?

Sure, use everything you need, we're opensource :)

[Mikhus]

Thanks @Mikhus. I'm already on this here: https://github.com/royaltm/node-murmurhash-native/blob/types/index.d.ts Perhaps I'm yet a little clumsy with ts. Any wisdom from knowledgeable TypeScript enthusiasts will be appreciated :)

What I can see is that probably the overloads interface should be re-ordered a bit, because we have this in docs: https://www.typescriptlang.org/docs/handbook/functions.html

In order for the compiler to pick the correct typecheck, it follows a similar process to the underlying JavaScript. It looks at the overload list, and proceeding with the first overload attempts to call the function with the provided parameters. If it finds a match, it picks this overload as the correct overload. For this reason, it’s customary to order overloads from most specific to least specific.

[royaltm]

Wow, definitely should be re-ordered. I have another question, should I avoid abstract classes? I don't see anybody is using them in declarations, but it comes in handy - less repeating of method declarations especially in incremental.d.ts.

The current declarations and tests compiles with typescript as low as 2.0.10 successfully.

[Mikhus]

As I understand you may replace them with interface declarations, as far as you have no implementation in typedefs, so abstract classes can be meaningful only if you have implementation done in typescript... Correct me if I'm wrong... You may extend interfaces as well

[royaltm]

The problem is that when implementing interfaces for classes (in declarations) you need to repeat all of the interface methods. I evaded it by using abstract class to implement them once and then extend the abstract class on each of the hasher implementation. here

[royaltm]

When trying to extend the interface I get the error indicating that interfaces are meant to be implemented instead.

[Mikhus]

Aha... I see... I don't get a clue if you really need to export classes, due to your docs in README.md, you work with createHash(). Or lib really exports classes in JavaScript? UPD: alright, got it, interesting... hmm..

[Mikhus]

As an idea, why not to do like this:

export declare const MurmurHash: IMurHasherConstructor;
export declare const MurmurHash128: IMurHasherConstructor;
export declare const MurmurHash128x64: IMurHasherConstructor;
export declare const MurmurHash128x86: IMurHasherConstructor;  

Then you do not need at all your abstract class and export them as classes...

UPDATE: Tried this locally, for me it works like a charm. IDE recognizes and provides auto-complete, typescript compiles as expected with no errors... I just kept IMurHasher interface definition, IMurHasherConstructor definition and put my exports, all other thigs removed.

The problem with exporting abstract class is that from TypeScript point of view you can import it and extend, but your real JavaScript implementation (or even lower - native) does not provide in reality that abstract class. From the other hand, interfaces are something available on TS level only so after compile to JS they are removed from the code, while classes are not. I guess that abstract class which exists on TypeScript level, but absent on JS level may cause some confusion or problems on the end-user side... I mean that interface is only a type reference, while class - is an implementation reference. Unless your lib does not provide the implementation you can provide the declaration like that, but it may cause some errors if I would like to instantiate (or extend) something which does not exist in reality...

[Mikhus]

Another idea is that maybe you can keep your abstract class, but you should not export it as far as your real implementation does not provide such thing. This is in case if you want to extend some specific implementations in the exact classes which are exported (MurmurHash, MurmurHash128x64, MurmurHash128, MurmurHash128x86)...

[royaltm]

I have experimented with what you gave me and:

• I could remove abstract class but then I loose the possibility to provide specialized digest types for 128/64 bit hashes (the hashes are never number)
• I definitely could hide abstract class, it actually works unless you explicitly require it to import.

[royaltm]

I have another question regarding typescript and packaging. To compile successfully implementors would require to:

1. add @types/node to their dependencies - (at least for the Buffer declaration)
2. set some compiler options: tsc --lib es2015 (mainly for @types/node to compile)

Is there a way to hint this here - should I provide some tsconfig.json? Obviously I don't want to add @types/node to the regular dependencies.

[Mikhus]

I do not think you have to. Because those who uses TypeScript will either include types for node or core-js (if they are on browser). As far as your library is stick with node, it means that it will be added anyway on package which uses typescript and added your lib as dependency.

I personally do not think you have to add any special dependencies to your package.

To be certainly clear, I guess, it is enough to state that in docs. Like "murmurhash-native is node-specific package, if you're going to use TypeScript, do not forget to include @types/node and enable es2015 language features in your tsconfig.json" (which is usual stuff for those who is using TypeScript on node).

I'm, as one who regularly uses TypeScript in my projects, can tell that this is more than enough for me :)

In other words, just stay with only *.d.ts files in your package - it's enough and quite OK, I see no need to put any deps, tsconfig, etc...

[royaltm]

Thanks, @Mikhus for the help. Ok, I think the declarations are ready.

@purplemana would you like to check with:

"murmurhash-native": "royaltm/node-murmurhash-native#types"

in deps in your project if it works for you?

[Mikhus]

@royaltm No, man, big human thanks to YOU! You did a great job, I rely on your package heavily in my new framework, and I will try your typing as well in my code :)

Really, thank you very much for this great package!

[Mikhus]

Tried, for me it works perfectly, thank you. Will wait for next official release from you on npm to update my packages :) BTW, I'm using it in very limited manner - a single function call of murmurHash128

@royaltm, could I ask you a question? Do I understand correctly that 128 bit version guarantees minimal amount of collisions in comparison to other versions? From the other hand, which version is faster for unpredictable message length hashing? I know it may be found due to some of experiments, but maybe you can suggest depending on your experience... Thank you in advance for the answer!

[royaltm]

Once upon a time :) I've built some hash-based key/value database in pure C (similar to level-db, but for different purposes - unfortunately not opensource). The keys were mainly human language based, but unpredictable. From my experiments the best max speed/min collision ratio had murmurhash64x64. 128 was observably slower, in comparison to collision reduction and the hashes took less space. Unfortunately I didn't implement progressive version of murmur2 64bit (if it bothers you at all). 128 is good if you have really large body to hash (hence incremental implementation of it written here).

[royaltm]

In other words: do your own experiments. There are no definitive answers to such questions.

[Mikhus]

Sure, I do understand that well, but your insights were absolutely valuable. Thanks again!

[purplemana]

Worked great for me, thanks!