base32-js icon indicating copy to clipboard operation
base32-js copied to clipboard

Separate sha1 function/make it optional

Open epoberezkin opened this issue 10 years ago • 10 comments

We use base32 in the browser with browserify. sha1 requires crypto which adds 5k+ lines to the bundle. Also, I don't think sha1 will work in the browser anyway...

In the fork I separated sha1 (https://github.com/MailOnline/base32-js/commit/9c7fe124b3c7245d1271794965e9d75d98caa746), but I don't think it is a good solution, it will break code that uses it. Any better ideas?

epoberezkin avatar Feb 24 '14 19:02 epoberezkin

Perhaps we could simply have sha1 require crypto when it's needed, and memoize? Will browserify still pull in require statements in a function body or no?

agnoster avatar Feb 24 '14 19:02 agnoster

It is in the function body already, but browserify simply bundles together everything that is required... Maybe the right thing is to have one index file which is the same as current that requires both encode/decode and sha1 (to keep all existing users happy) and add another file for the browser that will only require endode/decode. I can refactor like this and add something to readme about another version for the browser.

epoberezkin avatar Feb 24 '14 20:02 epoberezkin

Ironically, it should work fine in the browser without browserify - it’s actually browserify that makes it not work :-(

agnoster avatar Feb 24 '14 20:02 agnoster

sha1 works in the browser?

epoberezkin avatar Feb 24 '14 20:02 epoberezkin

No, but I mean base32 loads and you can use all the other methods, right?

agnoster avatar Feb 24 '14 20:02 agnoster

I can use it with browserify too (without sha1). It's just that id adds almost 6000 lines to the bundle... So my idea was to make a second file for the browser without sha1 without duplicating the code and without breaking existing api. I will show you.

epoberezkin avatar Feb 24 '14 21:02 epoberezkin

Right, but typically one uses browserify to take a module that doesn’t work in the browser and make it work, while this already works in the browser and only brings in a bunch of unnecessary code for you.

agnoster avatar Feb 24 '14 21:02 agnoster

Maybe I simply have to load it separately in the browser. As it's used in the framework all dependencies of which are relatively small we had everything in one bundle... I'll think about it. Maybe we can just have an extra build step and concatenate base32 to the bundle rather than require it

epoberezkin avatar Feb 24 '14 21:02 epoberezkin

:+1: same problem with webpack

lbeschastny avatar Nov 30 '15 10:11 lbeschastny

Solved Webpack build error with the

module: {
        noParse: [
            /base32/,
        ],
...

cyxou avatar Apr 08 '17 18:04 cyxou