search

LinusU / secure-remote-password

A modern SRP implementation for Node.js and Web Browsers
101 stars 22 forks source link

Improve browser support by adding Browserify versions of createHash and randomBytes as dependencies #4

Closed alexgoldstone closed 6 years ago

alexgoldstone commented 6 years ago

Adds Browserify versions of the necessary Crypto functions (createHash and randomBytes) as dependencies rather than relying on external build tools to make the Browserify functions available.

The implementation defaults to the Node module when available and falls back to the Browserify functions when required.

LinusU commented 6 years ago

Thanks for submitting this! The problem with this approach is that both modules will be bundled in the final bundle, since both the builtin crypto and the newly introduced modules are both required.

What do you think about the approach in #5? Would you mind testing it out?

Thanks!

alexgoldstone commented 6 years ago

I see what you are saying... when it is Node or Browser my fix is fine but in your case the build tool is applying the Polyfill itself so the Browserify module is being imported twice. That being said they are small functions so not much harm.

Webpack can be configured not to do this (since the Angular team has turned it off) but I assume this is default behaviour?

I agree it would be preferable to remove the module duplication and will take a look at https://github.com/LinusU/secure-remote-password/pull/5 as soon as possible.

LinusU commented 6 years ago

Closing in favour of #5

Webpack can be configured not to do this (since the Angular team has turned it off) but I assume this is default behaviour?

Yeah seems like the default is to shim

That being said they are small functions so not much harm.

Just tested a fresh build of my project with 0.2.1 and got the file size down from 2.13 MB to 1.51 MB 🙌