Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BigInt support and Unicode handling #9

Merged
merged 10 commits into from
Nov 18, 2019

Conversation

broofa
Copy link
Contributor

@broofa broofa commented Aug 1, 2019

  • Adds support for the fnv1a.bigInt() api suggested in Support higher bit variants #3.
  • Also deals with unicode strings in a way that's consistent with other fnv1a hash implementations. [Edit: This changes the hash for any string that has unicode in it, hence the BREAKING CHANGE action.]

mocha and ava both puke if you try to do BigInts in them, so I've had to resort to testing with the native assert module. That may or may not be adequate for you. And xo doesn't recognize bigint literals, nor does ESLint, so I've had to remove the linting step from npm test.

Note: I ended up incorporating some of @ShynRou's work for BigInt support. He should get the IssueHunt bounty for this.

P.S. FWIW, the legacy implementation (32 bit Numbers) is ~9X faster than the BigInt counterpart. https://jsperf.com/fnv1a-number-vs-bigint-perf

Fixes #2
Fixes #3
Closes #7
Closes #4

@broofa
Copy link
Contributor Author

broofa commented Aug 6, 2019

@sindresorhus
Copy link
Owner

@broofa Really sorry for leaving this for so long. Real-life and a way too long PR queue came in the way...

@sindresorhus
Copy link
Owner

ava both puke if you try to do BigInts in them,

I just tested with latest version of AVA and BigInt and 1n works fine now. Can you bring back AVA as testing?

And xo doesn't recognize bigint literals, nor does ESLint, so I've had to remove the linting step from npm test.

XO and ESLint support it now. Can you bring back XO?

@sindresorhus sindresorhus changed the title BREAKING CHANGE: BigInt support + Unicode handling - Fixes #2, Fixes #3 Add BigInt support and Unicode handling Nov 17, 2019
@sindresorhus
Copy link
Owner

You need to update index.d.ts too.

@broofa
Copy link
Contributor Author

broofa commented Nov 18, 2019

@sindresorhus Updated to use xo and ava. I need some guidance on the following, however:

  1. I'm not a TypeScript person, so have no idea what I'm doing in index.d.ts
  2. I updated to xo@latest, but it's still failing due to "BigInt is not defined. no-undef". (See failing Travis tests)

@sindresorhus sindresorhus merged commit cbdc497 into sindresorhus:master Nov 18, 2019
@sindresorhus
Copy link
Owner

Thank you for your work on this, @broofa 👍

sindresorhus added a commit that referenced this pull request Nov 18, 2019
@broofa
Copy link
Contributor Author

broofa commented Nov 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support higher bit variants About Unicode
2 participants