feat: per-version uuid validation

[Mearman]

adds specific validation for all uuid versions
should be 100% backward compatible

[broofa]

Thanks for the PR.

Suggesting some minor changes if we decide to take this, however we should have @ctavan and others weigh in before you do more here.

My main concern is that validating uuid versions is already trivially done with version(). E.g. version(uuid) === 1. This expands the API surface significantly, but I'm not really sure it's adding much value.. 😦

[Mearman]

hey @broofa, I like your suggestions.

The main reason for this is because of a project specifically checking for v4 UUIDs, we were using the uuidv4 which deprecated and switched to internally redirected to uuid's validate, which wasn't fit for purpose in our project.

I think there is still value for people being able to call for per-version validation. I'm going to revise my branch.

[Mearman]

@broofa what do you think to this revision? I've used an internal call to version for simplicity.

As it looks like you like monolithic commits on here, I amended and force-pushed on my fork.
I've preserved the old commit if you'd prefer for me to redo it as a separate commit?

[TrySound]

I think it makes sense to use only named exports in this module.

[Mearman]

not sure what you mean?
Most other things seem to be done as default exports.

[TrySound]

They don't have named exports. Validate is not that special to use different export syntax than other methods in this module.

[Mearman]

Ahhh, I understand what you mean now. Good point.
I agree that'd be more consistent with the rest of the source. I'll revise my branch.

[TrySound]

IMO so much fragmented source code is awful (hard to maintain). And this is exactly the reason I personally always avoid default exports.

[Mearman]

IMO so much fragmented source code is awful (hard to maintain). And this is exactly the reason I personally always avoid default exports.

@TrySound I'm not a fan, but I appreciate the importance of consistent style rather than one specific style, so I'm happy to do it to stay in-line with the rest of the library's source.

[broofa]

The main reason for this is because of a project specifically checking for v4 UUIDs

Does this add enough value for enough users to warrant inclusion in this module, though? Validation functions are trivially easy to implement, as demonstrated by the implementations in this PR. It may be simpler/better to direct users to roll their own so they get the exact behavior they need (e.g. including validate() or not, or allowing multiple versions for something like const isNamespaceUuid = uuid => version(uuid) === 3 || version(uuid) === 5).

The cost of including these is mostly the API and documentation bloat. While it may not seem that big a deal, this affects a lot (100K's or M's?) of users, 99% (???) of whom won't need these methods.

IMO so much fragmented source code is awful (hard to maintain).

Does file fragmentation affect dead code removal ("tree-shaking")? The current function-per-file schema is an artifact of the CommonJS deep-include approach in version 3 of this module, but that's been deprecated so there's no compelling reason for it. Assuming bundlers are smart about removing dead code within a module we can structure the files in whatever way makes the most sense. For example, the v1-v5 methods are large and distinct enough they should probably be separate (I wouldn't expect much disagreement on that), but the other methods could go in a 'util.js` file.

[ctavan]

This feature request has been raised a couple of times in the past, see #526 (comment) and #498 (comment) and my suggestion has always been to simply make use of the existing API and write out the one-liner

validate(uuid) && version(uuid) === uuidVersion;

that this pull request implements.

Now this PR adds 239 lines of code and I'm with @broofa in that I think this is really a lot of overhead to maintain in the future for something that can already be achieved by combining two existing API calls in one single line of code.

So unless we find really compelling reasons of why having to learn about another 4 methods of the API surface is significantly better for end users than to just combine two existing ones my tendency is to not accept this pull request.

I would, however, be more than happy to accept a pull request that improves the documentation and provides the above mentioned one-liner as a copy & pastable example.

[Mearman]

@ctavan 160 of those lines are in the documentation and 47 are in testing, but I see your point.
I'm happy to create a new PR for the addition of example snippets for per-version validation.
Where would you like them in the docs?

[ctavan]

@ctavan 160 of those lines are in the documentation and 47 are in testing, but I see your point.

This is precisely the point 😉 . It's documentation and tests that are tedious to maintain on the long run.

I'm happy to create a new PR for the addition of example snippets for per-version validation.
Where would you like them in the docs?

Where have you been looking for it (to then be disappointed not to find version-specific validation)?

[Mearman]

Where have you been looking for it (to then be disappointed not to find version-specific validation)?

Hahah, I suppose by the validate and version methods. Though I feel it'd be out of place in the API section of the documentation.

What about a new Additional Examples section @ctavan?

[ctavan]

I agree with you in principle, but the question is if people will find it if it appears in another section?

I personally wouldn't mind just adding another short paragraph and another code block after the uuid.validate() example.

[Mearman]

Makes sense 👍

[Mearman]

Haven't runmd yet, but what do you think to this?
Mearman@55d26ed

[ctavan]

For my personal taste this becomes to verbose. I'd restrict it to one simple example:

import { version as uuidVersion, validate as uuidValidate } from 'uuid';
function uuidValidateV4(uuid) {
  return validate(uuid) && version(uuid) === 4;
}
const v1Uuid = 'd9428888-122b-11e1-b85c-61cd3cbb3210';
const v4Uuid = '109156be-c4fb-41ea-b1b4-efe1671c5836';
console.log(uuidValidateV4(v4Uuid)); // RESULT
console.log(uuidValidateV4(v1Uuid)); // RESULT

And I'd also not put it under a new section but just as a second example in the validate section.

[Mearman]

Closed in favour of #543