multiformats / js-multihashing

Use all the functions in multihash.
MIT License
11 stars 18 forks source link

Use `sha.js` instead of full crypto in browser #10

Open pelle opened 8 years ago

pelle commented 8 years ago

Only use sha.js per suggestions from @dignifiedquire here https://github.com/ipfs/js-ipfs-api/issues/353

Not sure how much of an effect it will have on the distribution size

pelle commented 8 years ago

Failure in Travis is due to lint complaining about digest function names:

''' 49:10 error Identifier 'gsha2_256' is not in camel case camelcase 53:10 error Identifier 'gsha2_512' is not in camel case camelcase '''

I suppose it could be fixed, since I guess these aren't called externally.

dignifiedquire commented 8 years ago

To make this really efficient we need to do a bit more work, isNode is an npm module, and not a static variable, that means the require('crypto') does not get optimised away.

What we want is something like this

let createHash

if (buildingForBrowser) {
  createHash = require('sha.js')
} else {
  createHash = require('crypto').createHash
}

and then let our build tool define buildingForBrowser. This way when building for the browser this would be transformed into

let createHash

if (true) {
  createHash = require('sha.js')
} else {
  // unreachable code
  createHash = require('crypto').createHash
}

This in turn would be then properly analyzable by webpack & uglify and they could remove the unreachable code.

dignifiedquire commented 7 years ago

@pelle I just finished #12 which uses webcrypto instead of JavaScript fallbacks in the browser reducing the size even further :)

daviddias commented 7 years ago

@pelle did you had the chance to check @dignifiedquire suggestions?

pelle commented 7 years ago

Sorry I missed this. I guess I was at devcon and all was a blur. Having a look now.

pelle commented 7 years ago

Looks great. Great progress @dignifiedquire and @diasdavid

kumavis commented 7 years ago

can also set crypto: false in the browser field in package.json

hacdias commented 6 years ago

@diasdavid I think this one could be closed since it is a bit old, has a lot of conflicts, and js-multihashing is using webcrypto now.

daviddias commented 6 years ago

@hacdias only multihashing-async is using webcrypto, not this one.

hacdias commented 6 years ago

Oh. Sorry. Should we use web crypto on this one too? On Tue, 2 Jan 2018 at 17:17, David Dias notifications@github.com wrote:

@hacdias https://github.com/hacdias only multihashing-async is using webcrypto, not this one.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/multiformats/js-multihashing/pull/10#issuecomment-354819703, or mute the thread https://github.com/notifications/unsubscribe-auth/AFMdsK2uTRZ3h2MsOFDd6dMzYsgjfKNnks5tGmSvgaJpZM4JpVIB .

daviddias commented 6 years ago

@hacdias the thing is that you can't. WebCrypto is async only (that is why there is a multihashing-async)