search

koopjs / winnow

Deprecated
Apache License 2.0
90 stars 18 forks source link

Change farmhash to an optional dependency and add fall back to string-hash #91

Closed CorinChappy closed 6 years ago

CorinChappy commented 6 years ago

Hi there, I am trying to build a version of Koop in an environment where I am not able to compile native node modules, this means I can't use the most up-to-date version of Koop due to winnow's dependency on farmhash.

This PR makes the native farmhash module optional, adding a fallback to a pure JavaScript hashing library when farmhash is not installed. This means those that cannot or don't want to use native modules can still use winnow, while others can still take advantage of farmhash's superior speed.

I roughly followed the instructions for optional dependencies on npm but let me know if you'd rather I implemented it in a different way.

Thanks!

rgwozdz commented 6 years ago

@CorinChappy - Thanks very much for your contribution here. By coincidence, I just finished moving that farmhash implementation to a different part of the code base. It's now here: https://github.com/koopjs/winnow/blob/master/src/sql.js#L143. So if it's not to much trouble, could you adjust your PR to reflect that change?

CorinChappy commented 6 years ago

That is no problem @rgwozdz, I will make the changes later today

CorinChappy commented 6 years ago

@rgwozdz merged your updates in and made the change!

CorinChappy commented 6 years ago

@rgwozdz Any updates on this?

rgwozdz commented 6 years ago

@CorinChappy it's been merged into master and released as npm 1.16.2.