Crypto dependency issue when bundling with browserify or webpack

[josdejong]

Decimal.js has a dependency on the crypto library. When bundling decimal.js as part of a larger application using browserify or webpack, a browser compatible version of crypto is bundled with the app by default. This works, but crypto is quite a large dependency.

Looking at the code of decimal.js, the intention is to use native crypto functionality when in the browser, and use the crypto library when in node.js.

I can exclude crypto when bundling my application. That works fine in node.js, but not in the browser via AMD/require.js: decimal.js thinks that we are in a node.js environment because module and module.exports are defined, and tries to load crypto via require rather than using the browsers native crypto implementation (see L3598).

It would be great if this loading of crypto could be rewritten such that the browser environment is correctly detected. Maybe by just checking the ducktype way: first check whether window and window.crypto exist, and if not, fallback to loading via require('crypto'), and if that fails, silently disable the crypto functionality.

[josdejong]

Not yet fully tested, but the following seems to work when bundled via webpack: test for amd first.

    if ( typeof define == 'function' && define.amd ) {
        // AMD.
        crypto = global['crypto'];

        define(function () {
            return DecimalConstructor
        });
    }
    else if ( typeof module != 'undefined' && module && module.exports ) {
        // Node and other CommonJS-like environments that support module.exports.
        module.exports = DecimalConstructor;

        if ( typeof require == 'function' ) {
            crypto = require('crypto');
        }
    } else {
        // Browser.
        crypto = global['crypto'];

        noConflict = global['Decimal'];

        DecimalConstructor['noConflict'] = function () {
            global['Decimal'] = noConflict;

            return DecimalConstructor;
        };
        global['Decimal'] = DecimalConstructor;
    }

[MikeMcl]

Looks good, Jos.

Fixed in v2.1.0.

I am also pushing v3.0.0 in a minute or two. Only one small API change - a simplification of random - but a big speed increase for some operations due to switching to storing digits internally in base 10000. One consequence is that the properties of a Decimal will now need to be considered strictly read-only, e.g. changing an exponent by writing over the e property may no longer work correctly.

[josdejong]

Thanks a lot for this fast fix Michael! It works great now with webpack.

With browserify I have still some trouble. When I bundle decimal.js like this (imagine as part of a larger application):

browserify node_modules/decimal.js/decimal.js -s Decimal -o myapp.js --external crypto

And load it in a browser like this:

<script src="myapp.js"></script>

I still get an error that crypto is missing. I think this can be solved by adding a check whether typeof window !== undefined in the module loading part for commonjs:

    // ...
    else if ( typeof module != 'undefined' && module && module.exports ) {
        // Node and other CommonJS-like environments that support module.exports.
        module.exports = DecimalConstructor;

        if (typeof window !== 'undefined' ) {
          // browserified app
          crypto = global['crypto'];
        }
        else if ( typeof _dereq_ == 'function' ) {
            crypto = _dereq_('crypto');
        }
    } else {
    // ...

[MikeMcl]

Fixed in v3.0.1.

I've taken a try catch approach as some browserify users may want the require('crypto') call if crypto it is not available in the browser and they have not excluded it from the bundling.

Let me know if you have any issues with it.

[josdejong]

Awesome, your try/catch solution is indeed a better solution. I can now bundle and load decimal.js in any way I can imagine and in it loads without errors. Thanks!

p.s. I really hope this kind of dependency issues will become easier once ES6 modules is adopted by all browsers...

[ryanmcgrath]

I hope nobody minds me leaving this note here - it took me a bit of time to realize this library is what was injecting the entire crypto package into my build. What I'm building really doesn't require this in any way and it (like mentioned above) is a bit heavy - it looks like this library is just using crypto for random number generation, no?

The way I got around it being injected for Browserify was by just marking it as external. It might be worth documenting this somewhere?

[MikeMcl]

@ryanmcgrath

Yes, crypto is just used for random number generation.

I think a check to see if window is defined needs to be added, as suggested above, to stop Browserify doing this, as the browser does not need the crypto package if it supports crypto.getrandomvalues.

[MikeMcl]

@ryanmcgrath

After looking through the Browserify docs, I see that the window check wouldn't work due to Browserify using 'static analysis' of the file.

The solution seems to be to add the following to the package.json here

"browser": {
    "crypto": false
}

That should avoid the need to exclude the crypto package.

[soulchainer]

@MikeMcl Hi. I just want yo say that using the "browser" field doesn't work. At least for me. I solved the issue telling browserify to ignore crypto.

Thanks a bunch for leaving that note, @ryanmcgrath.

[MikeMcl]

@soulchainer

Okay, I'll have a look at this again. I want to get it right as I am pushing a major update in a couple of weeks.