feat: add parse/stringify/validate/version/NIL APIs #479

ctavan · 2020-07-17T12:48:51Z

@broofa should we just merge this into master and release as 8.2.0? As far as I can tell it only adds new features, no breaking changes. Or do I miss something?

Closes #475
Closes #478
Closes #480
Closes #481

* test: parsing non RFC uuid values * feat: create validate, version, uuidRegex (and import to public API) * style: always curly braces added comments to numberToBytes func preallocated array in uuidToBytes func * Update src/regex.js Co-authored-by: Robert Kieffer <robert@broofa.com> * Update src/validate.js Co-authored-by: Robert Kieffer <robert@broofa.com> * Update src/version.js Co-authored-by: Robert Kieffer <robert@broofa.com> * Update test/unit/version.test.js Co-authored-by: Robert Kieffer <robert@broofa.com> * feat: throw error when invalid uuid * test: validate function * feat: short version of parsing UUID * fix: remove explicit ie babel target * Revert "feat: short version of parsing UUID" This reverts commit d096cc2. Co-authored-by: Robert Kieffer <robert@broofa.com> Co-authored-by: Christoph Tavan <dev@tavan.de>

* fix: improved uuidToBytes * chore: comment tweak * chore: bump bundlewatch sizes * fix: uuidToBytes test * feat: export uuidToBytes, uuidToBytes benchmark * chore: review comments * feat: export validate, parse, stringify methods * chore: stringify unit test and benchmark * fix: stringify test in browser * feat: add uuidVersion to rollup exports * chore: bump bundlewatch limits * chore: tweak function name * test: enable browserstack.console setting * fix: revert padStart * fix: pr review comments * fix: no Array.from in IE11 * improvement: hardcode namespace length Co-authored-by: Linus Unnebäck <linus@folkdatorn.se> Co-authored-by: Christoph Tavan <dev@tavan.de> Co-authored-by: Linus Unnebäck <linus@folkdatorn.se>

ctavan · 2020-07-17T12:50:54Z

BTW we still need documentation in the README.

broofa · 2020-07-17T14:54:39Z

@ctavan: Agreed. As you said, there's no breaking changes.

I'll update the README. Also, I'm going to go ahead and add a NIL_UUID constant as well, per discussion in #475. In thinking further, I don't see much down side to that.

broofa · 2020-07-17T22:49:55Z

@ctavan Updated the README.

Added docs for new methods
Added API Summary section to show the API surface in one glance.
Reduced "quick start" section down to just the bare minimum for v4 generation since that's the 80+% use case...
... and users can refer to the examples in the API docs for how to create other versions of UUID.
Put method args, returns, and errors into tables

Note: I also made one minor functional change. stringify() now validate()s generated uiud strings to ensure they're valid. Previously you could hand it any bytes and it would blindly stringify, even if the result wouldn't pass validate(). I feel this is in keeping with the "this module follows the RFC" philosophy we've had but it wouldn't surprise me if/when someone complains about it.

broofa

Okay, I'm happy with this, but maybe get a sanity check review from @TrySound?

TrySound

Everything looks great

ctavan

@broofa this looks awesome, thanks a lot 👏

I've made some minor consistency adjustments to the README myself.

I still have 3 questions that we should address before merging this. Once merged I'd like to release this as a beta release and ask the people who wanted these features to try them out. Upon positive feedback I'll make the 8.2 release.

ctavan · 2020-07-20T08:25:45Z

examples/benchmark/benchmark.js

+}
+
+testParseAndStringify();
+testGeneration();


Will these tests run in parallel? Shouldn't we rather ensure that they are run in series?

Will these tests run in parallel? Shouldn't we rather ensure that they are run in series?

I believe Benchmark.js runs synchronously, so these do run in series. (At least, that's what I see when I do cd examples/benchmark; node benchmark.js.)

rollup.config.js

src/stringify.js

ctavan · 2020-07-21T11:16:33Z

Changes look good. What about my comment regarding the benchmark parallelism?

zaggino · 2020-07-31T10:20:13Z

@broofa @ctavan I upgraded package from 8.2.0 to 8.3.0 and this should be marked as a breaking change (major) update. Before the routine would work:

import { v5 as uuidv5 } from 'uuid';

const UUID_V5_NAMESPACE = '00112233-4455-6677-8899-aabbccddeeff';

function getUuidHash(str: string) {
  return uuidv5(str, UUID_V5_NAMESPACE);
}

Now after update it throws an error:

TypeError: Invalid UUID

ctavan · 2020-07-31T11:38:31Z

@zaggino the RFC requires the namespace to be a valid UUID. It's true that up to 8.2.0 we did not verify that the passed namespace was a truly valid UUID, and we added this check in 8.3.0. However this should rather be considered a bug fix than a breaking change since the behavior before was counter the RFC.

Can you update your code to pass only valid UUIDs as namespaces? A valid UUID must match this regex:

uuid/src/regex.js

Line 1 in ade3655

    
           export default /^(?:[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}|00000000-0000-0000-0000-000000000000)$/i;

I.e. it must either be the NIL UUID or it has the UUID version after the second hyphen (in your example there's a 6 which is not a valid UUID version) and one of 89ab after the third hypen (you have an 8 in your example here, so this would be valid).

zaggino · 2020-07-31T12:27:10Z

I did update the code, but my concern was that a ^8.0.0 dependency which we had caused a break all over the application when a package-lock.json was refreshed.

From the RFC perspective, I agree on fixing this, but from a user-developer perspective - I never noticed any buggy behaviour before this, so was there ever an actual bug when a namespace which was not a 100% proper uuid was used?

ctavan · 2020-07-31T12:32:34Z

I think this can be a serious interoperability issue: if you have other libraries or programming languages that already do the right thing by validating the passed namespace UUIDs then the UUIDs generated from malformed namespaces with old versions of this library won’t be compatible with those other libraries/languages.

So as long as everything that ever has to deal with the v5 UUIDs generated off of bad namespace ids will always use the same flawed library there’s probably no problem, but as soon as you need to be interoperable this might cause very subtle and hard-to-find bugs.

ctavan · 2020-07-31T12:35:05Z

All that said I agree that it is maybe not the best developer experience to introduce such a change in a minor version bump. Unfortunately it’s out now and too late. Our assessment that this should be a non-breaking change for most developers was apparently wrong. We‘ll be more careful in the future. I’m sorry for the inconvenience.

awwit and others added 4 commits June 2, 2020 08:37

Merge branch 'master' into v9

82ef52b

Merge remote-tracking branch 'origin/master' into v9

bb21fcf

ctavan requested a review from broofa July 17, 2020 12:48

broofa added 2 commits July 17, 2020 11:01

feat: add NIL_UUID export, fixes #475

7e2a384

chore: cleanup REGEX export

2585728

ctavan mentioned this pull request Jul 17, 2020

toBytes #481

Closed

broofa added 8 commits July 17, 2020 15:06

chore: rename NIL_UUID -> NIL

b3e9ec9

chore: throw if stringify UUID is not valid

3116a9e

chore: rename UUID_NIL -> NIL

13db698

docs: v9 docs

5d5f23b

Merge branch 'v9' of github.com:uuidjs/uuid into v9

df6fed7

fix: browser spec for nil UUID

b84bfbe

fix: remove nil uuid browser tests

44cc707

chore: tweak README

2ee8068

broofa added 2 commits July 19, 2020 11:15

fix: readme

a9791a6

fix: bundlewatch

af334d3

broofa approved these changes Jul 19, 2020

View reviewed changes

broofa requested a review from TrySound July 19, 2020 18:41

TrySound approved these changes Jul 19, 2020

View reviewed changes

ctavan added 2 commits July 20, 2020 10:23

fix: readme

39a07c9

fix: language

f15caed

ctavan commented Jul 20, 2020

View reviewed changes

broofa added 3 commits July 20, 2020 14:03

fix: add uuidNIL

40cf16a

Merge branch 'v9' of github.com:uuidjs/uuid into v9

ccc161e

fix: simplify stringify validation

91c1599

fix: rename NIL -> nil

c16ebbd

ctavan mentioned this pull request Jul 21, 2020

Add browser tests for stringify/parse/validate features. #485

Closed

ctavan changed the title V9 feat: add parse/stringify/validate/version/NIL APIs Jul 22, 2020

ctavan merged commit 0e6c10b into master Jul 22, 2020

ctavan deleted the v9 branch July 22, 2020 20:32

ctavan mentioned this pull request Jul 22, 2020

Expose bytes to uuid #180

Closed

broofa mentioned this pull request Mar 8, 2022

fix: unhandle error when parameter is not set in v3 and v5 #618

Closed

This was referenced Oct 27, 2024

chore(main): release 11.0.0 #808

Merged

chore(main): release 11.0.0 #809

Merged

chore(main): release 11.0.0 #810

Closed

chore(main): release 11.0.0 #813

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add parse/stringify/validate/version/NIL APIs #479

feat: add parse/stringify/validate/version/NIL APIs #479

ctavan commented Jul 17, 2020 •

edited

Loading

ctavan commented Jul 17, 2020

broofa commented Jul 17, 2020 •

edited

Loading

broofa commented Jul 17, 2020 •

edited

Loading

broofa left a comment •

edited

Loading

TrySound left a comment

ctavan left a comment

ctavan Jul 20, 2020

broofa Jul 21, 2020 •

edited

Loading

ctavan commented Jul 21, 2020

zaggino commented Jul 31, 2020

ctavan commented Jul 31, 2020

zaggino commented Jul 31, 2020

ctavan commented Jul 31, 2020

ctavan commented Jul 31, 2020

feat: add parse/stringify/validate/version/NIL APIs #479

feat: add parse/stringify/validate/version/NIL APIs #479

Conversation

ctavan commented Jul 17, 2020 • edited Loading

ctavan commented Jul 17, 2020

broofa commented Jul 17, 2020 • edited Loading

broofa commented Jul 17, 2020 • edited Loading

broofa left a comment • edited Loading

Choose a reason for hiding this comment

TrySound left a comment

Choose a reason for hiding this comment

ctavan left a comment

Choose a reason for hiding this comment

ctavan Jul 20, 2020

Choose a reason for hiding this comment

broofa Jul 21, 2020 • edited Loading

Choose a reason for hiding this comment

ctavan commented Jul 21, 2020

zaggino commented Jul 31, 2020

ctavan commented Jul 31, 2020

zaggino commented Jul 31, 2020

ctavan commented Jul 31, 2020

ctavan commented Jul 31, 2020

ctavan commented Jul 17, 2020 •

edited

Loading

broofa commented Jul 17, 2020 •

edited

Loading

broofa commented Jul 17, 2020 •

edited

Loading

broofa left a comment •

edited

Loading

broofa Jul 21, 2020 •

edited

Loading