ES-Module support (next)

[ctavan]

This is #331 but against the next branch instead of master.

[43081j]

Back when i did this too, i made the original sources browser ready and only had a build for producing a commonjs version.

Did you consider this? It does mean any node specific things need to be dropped or conditionally used somehow (i.e. crypto).

[ctavan]

@43081j yes, I did consider this. However I believe that this approach only really works with universal modules that don't have node- or browser-specific dependencies.

What would you suggest with respect to random number generation? It is absolutely vital that uuid uses a cryptographically secure pseudo-random number generator (CSPRNG) under the hood and so far the JavaScript language does not provide one so we have to rely on platform specific APIs like node's crypto module or WebCrypto in the browser.

So unfortunately I think in the case of uuid we will have to ship different builds for use in node and in the browser (until something like https://github.com/tc39/proposal-uuid gets through which might add a cryptographically secure pseudo-random number generator (CSPRNG) directly to the JavaScript language).

[broofa]

I think we can fix a number of issues here by distilling the implementation of this file down to:

export default function() {
  if (Math.getRandomValues) {
    return Math.getRandomValues(new UInt8Array(16));
  } else if (typeof(crypto) != 'undefined' && crypto.getRandomValues) {
    return crypto.getRandomValues(new UInt8Array(16));
  } else if (typeof(msCrypto) != 'undefined' &&  && msCrypto.getRandomValues) {
    return msCrypto.getRandomValues(new UInt8Array(16));
  }

  // TODO: Add README verbiage describing error and how to resolve by providing user-supplied rng function
  throw Error('Unable to locate getRandomValues() API.  See https://github.com/kelektiv/node-uuid#secure-api');
}

• Fixes Insecure random values should be opt-in #173 by removing dependency on Math.random()
• Forward-compatible with feat: introduce Math.getRandomValues() tc39/proposal-uuid#33
• Supports user-supplied rng (replace / supply Math.getRandomValues())

By not caching a reference to getRandomValues it becomes trivial for uses to supply their own implementation as needed. There's probably a modest perf penalty to feature sniffing this on each call but I don't think it's a significant concern.

[ctavan]

@broofa I'm all-in for this change, however I intentionally tried to not introduce any functional change within this pull request but instead exclusively get the switch to esmodules done before working on functionality changes.

Would you be fine with merging this PR first? I will make sure to implement your above suggestion in a follow-up PR and I absolutely agree, that we should fix #173 before releasing a next major versions.

Let's just finalize the es module switch first so we have smaller, easier-to-review pull requests for the functional changes.

[broofa]

Suggested change

	export default function nodeRNG() {
	export default function() {

Look for Math.getRandomValues() here, as above?

[ctavan]

Let's wait with functional changes for a follow-up PR.

[ctavan]

@broofa would it be OK for you to merge this issue without changing the RNG-behavior yet?

I have outlined my roadmap for the next major release in #343 (feedback very welcome!) and I would like to establish some browser tests before changing the API!

The only planned API change in this case is to make insecure RNGs opt-in.

[broofa]

Looks good. Sorry for the long wait on this - 'been vacationing with the 'fam. 🤷‍♂