pvorb/node-md5

A little question

ktimothy opened this issue · 8 comments

There is a really native way to calculate md5 hash in nodejs:

crypto.createHash('md5').update(string).digest("hex");

So what is your module for?

  • It's easier to use

    md5(string) vs crypto.createHash('md5').update(string).digest("hex");

  • It's got a stable API, while crypto has not

    Stability: 2 - Unstable; API changes are being discussed for
    future versions. Breaking changes will be minimized. See below.

    http://nodejs.org/api/crypto.html

  • It works in the browser

Do you need more reasons?

The existence of the module is unquestionable, a simple md5 function is very nice and a no-brainer npm install, even if all it did where to simplify 3 lines into 1.

The fact that the full implementation works in the browser is a very good reason to keep it as is. I would recommend, however, defaulting to the crypto module if its available.

From my attempt to use this module I learned that it can take significantly longer to hash large files and even throw out of memory exceptions (tested on 45mb file). Using the crypto library is much faster and so if its available it would be nice to default to it. Something like:

if (process && process.versions && process.versions.node) {
  var crypto = require('crypto');
  module.exports = function md5(content) {
    return crypto.createHash('md5').update(content).digest('hex');
  };
} else {
  // curent code..
}

Using the crypto library is much faster

Of course it is faster, it is written in native code.

pvorb commented

@justinmchase If I did what you proposed, it would include crypto-browserify in browserify installations. This is not an option.

If you need a simpler api around crypto, you should implement one on your own.

The main purpose of this library is to be cross plattform, which forces it to use a JS implementation.

@ktimothy Writing software in C++ doesn't automatically make it faster.

@pvorb Why would you need to do that?

In the browser you would _not_ use the crypto library, in node you would. Use the process.version to detect if you're running in node vs. a browser. Why is that not an option? Seems pretty straight forward. What does crypto-browserify have to do with anything?

This module isn't really usable in node otherwise, to be frank.

pvorb commented

Browserify inspects code for require()s. When it spots crypto it will automatically include a JS implementation for it (crypto-browserify). It won't look for conditional require()s.

pvorb commented

This module is useable in Node anyway. It simply does not match your use case.

This module is useable in Node anyway. It simply does not match your use case.

Yep, it simply does not work in some cases, but it is still usable. If it is not usable, you are just using it wrong way. But it is usable. Lol. :D