agnoster / base32-js

Base32 encoding for JavaScript, based (loosely) on Crockford's Base32
https://github.com/agnoster/base32-js
MIT License
121 stars 61 forks source link

Separate sha1 function/make it optional #7

Open epoberezkin opened 10 years ago

epoberezkin commented 10 years ago

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?

agnoster commented 10 years ago

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?

epoberezkin commented 10 years ago

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.

agnoster commented 10 years ago

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

epoberezkin commented 10 years ago

sha1 works in the browser?

agnoster commented 10 years ago

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

epoberezkin commented 10 years ago

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.

agnoster commented 10 years ago

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.

epoberezkin commented 10 years ago

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

lbeschastny commented 8 years ago

:+1: same problem with webpack

cyxou commented 7 years ago

Solved Webpack build error with the

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