dcodeIO / bcrypt.js

Optimized bcrypt in plain JavaScript with zero dependencies.
Other
3.47k stars 264 forks source link

fix#73: replaced .push with .concat as it is faster #88

Open erdahuja opened 5 years ago

erdahuja commented 5 years ago

@Ruffio Hi, i changed it into tests because salt2 is generated from bindings.genSalt instead of bcrypt.genSlat. Also the test would fail without my changes too. I checked it's 2b there. Am i missing something here?

Will fix the other one, yes I had that way too you are right. I am new to open source. Thanks for feedback :)

erdahuja commented 5 years ago

So the tests are failing in master too. because of binding.genSaltSync defaults to 'b' minor.

https://www.dropbox.com/s/png23w5x5214zlb/Screenshot%202018-08-25%2017.37.33.png?dl=0

A simple fix would be to pass the second parameter: 'a' as a minor.

https://www.dropbox.com/s/8dnfks09hyye1sj/Screenshot%202018-08-25%2017.39.13.png?dl=0

Everyone happy :)

Ruffio commented 5 years ago

@dcodeIO what do you think?

Ruffio commented 5 years ago

@dcodeIO what do you think?

WebMatrixware commented 4 years ago

I'm not sure that contact actually is faster anyway, based on these articles and tests I would say it is actually the other way around.

https://codeburst.io/jsnoob-push-vs-concat-basics-and-performance-comparison-7a4b55242fa9 https://dev.to/uilicious/javascript-array-push-is-945x-faster-than-array-concat-1oki