feat: deprecate deep requires

[ctavan]

• Add test that covers the fact that deep requiring still works.

[ctavan]

@broofa while comparing the docs of the current v3.4.x and the new master I realized that now that I ship all transpiled sources in ./dist the previously encouraged deep import style

const v1 = require('uuid/v1');

no longer works.

I remember from my analysis in the tc39 repo that lots of libraries are actually using the deep import style, after all it was the recommended way of importing the uuid module for years…

I feel somewhat bad about introducing this breaking change:

• If we don't introduce this breaking change, upgrading from v3.4.0 to v7.0.0 should work without any code adjustments.
• If we do introduce it, a significant amount of users of this library will have to adjust their code.

Restoring the old directory structure in the npm package is certainly doable although with a bit more effort than just packaging the ./dist folder as I'm doing it right now.

How do you feel about this one?

[ctavan]

Note to self: I should then probably also add a test case which covers this… I just found this issue by coincidence, there should be a regression test if we want to continue to support this style of import.

[ctavan]

@broofa I have thought about this one again and I believe now is not the time to introduce a breaking change in how this library can be imported/required.

Instead, I decided to remain backwards-compatible with deep requiring the algorithm versions but emit a deprecation warning when people do that.

It looks something like this:

> const uuidv4 = require('./v4');
undefined
> uuidv4();
'5b7cced3-d703-49ff-b05e-a3c727f6e3e2'
> (node:77419) DeprecationWarning: Deep requiring like `const uuidv4 = require('uuid/v4');` is deprecated as of uuid@7.x. Please require the top-level module when using the Node.js CommonJS module or use ECMAScript Modules when bundling for the browser. See https://github.com/uuidjs/uuid/blob/master/README.md#upgrading-from-v3x-of-this-module for more informat

Edit:

The warning is currently emitted each time the deep-required function is called. That could potentially lead to a lot of noise in applications, that generate a lot of UUIDs.

How do you feel about this? Do you think the risk of people flooding their logs is too big? Or should this be annoying enough for people to change their implementation?

An alternative would be to emit such a warning only once when the module is required.

I should have RTFM: https://nodejs.org/api/util.html#util_util_deprecate_fn_msg_code

When called, util.deprecate() will return a function that will emit a DeprecationWarning using the 'warning' event. The warning will be emitted and printed to stderr the first time the returned function is called. After the warning is emitted, the wrapped function is called without emitting a warning.

[ctavan]

@broofa I have now added a test case which asserts that deep imports now produce a DeprecationWarning.

[broofa]

No strong opinions, just a couple observations:

1. We're talking about a major release here... breaking changes are, by definition, expected. 🤷‍♂
2. What API would a "first principles" approach dictate? I.e. Ignoring past history, and given the current JS landscape, what is the right API? ("What API are we heading for?")

On that last item, one approach we haven't really discussed is breaking this up into separate modules, ala lodash's "lodash/array", "lodash/core", etc. E.g. npm install uuid/v4, then require('uuid/v4'). This would address the current "deep" dependency issue that tends to break packagers. I'm not necessarily advocating this, but it's worth considering.

[ctavan]

1. We're talking about a major release here... breaking changes are, by definition, expected. 🤷‍♂

When I initially picked up the modernization of this library it was because more and more people were specifically asking for ESModule support.

So my main intention was to put out a new major release that makes the requested switch to ESModules so that Typescript and Webpack/rollup users can benefit from that.

Dropping support for default insecure RNGs seems like a long-overdue leftover that, given the widespread support for crypto, is likely not a breaking change for >99% of the users of this library.

I think we can provide what has been asked for by many users (ESModules) without having to break any existing usage of this library.

After that we can still think of more radical approaches to evolve the API of this library, but I would prefer to deliver ESModules without any significant other changes to the API surface.

Does that sound reasonable?

2. What API would a "first principles" approach dictate? I.e. Ignoring past history, and given the current JS landscape, what is the right API? ("What API are we heading for?")

On that last item, one approach we haven't really discussed is breaking this up into separate modules, ala lodash's "lodash/array", "lodash/core", etc. E.g. npm install uuid/v4, then require('uuid/v4'). This would address the current "deep" dependency issue that tends to break packagers. I'm not necessarily advocating this, but it's worth considering.

I think these days ESModules and named exports are the way to go.

When you compare to lodash the per-method packages like lodash.get are now deprecated and their docs mention that deep requiring is no longer necessary when using modern bundlers (https://lodash.com/per-method-packages):

Furthermore, modern tree-shaking bundlers like webpack and rollup can avoid bundling code you don't need even if you don't use direct imports or the babel plugin.

So I'm confident that promoting the modern

import { v4 as uuidv4 } from 'uuid';

style syntax is as good as we can get with JS in 2020.

The only thing that would come to my mind is renaming the imports, e.g.

import { uuidv4 } from 'uuid';

However then we should probably go into the discussion of export names again. Shouldn't it then maybe rather be

import { randomUUID, timeUUID, nameUUID3, nameUUID5 } from 'uuid';

?

I would prefer to leave that discussion for a later time.

[ctavan]

I'm considering to rewrite the entire README to use import instead of require syntax. However it looks like it would be a major effort to get this to work with runmd. Any thoughts on this one @broofa?

[broofa]

That all sounds good to me.

Re: RunMD and ES6 imports, RunMD was a weekend project of mine (inspired by the need to keep uuid's docs up to date, actually) that's proven useful, but it's also... well... "quirky". Supporting ES6 is doable, but not something I have time to dive into at the moemnt. Thus, if that gets in the way of a README rewrite I'm fine with ditching it. If when someone (me, probably) gets RunMD working, we can decide whether to pull it back into the toolchain here.