Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doesn't work with UUID v7 #38

Open
IlyaSemenov opened this issue Nov 19, 2024 · 6 comments
Open

Doesn't work with UUID v7 #38

IlyaSemenov opened this issue Nov 19, 2024 · 6 comments

Comments

@IlyaSemenov
Copy link

The project doesn't work with UUID v7:

s.encode('01924249-77d3-7641-8c67-613b2881d5d4')
Uncaught TypeError: Invalid UUID
    at Object.parse (/Users/is/work/b/node_modules/.pnpm/[email protected]/node_modules/uuid/dist/parse.js:14:11)
    at exports.encode (/Users/is/work/b/node_modules/.pnpm/[email protected]/node_modules/slugid/src/slugid.js:46:24)

Because it relies on uuid.parse() which only works with UUID v1-v5.

@djmitche
Copy link
Contributor

@petemoore what do you think? I think we have historically only supported UUIDv4?

@IlyaSemenov what is the use-case for v7?

@IlyaSemenov
Copy link
Author

v7 uuids are binary/lexicographically sortable, and in particular can be used for fast and reliable cursor pagination.

@djmitche
Copy link
Contributor

Thanks! Looking at the README, this library has been super-clear that a slugid is an encoded v4 Uuid. I suspect changing that to include v7 wouldn't affect most uses of the library, but we'd need to be careful. Maybe this would be a major version bump?

I believe the Mozilla folks most responsible for this library have been at a work-week all week, so hopefully we can hear from them next week.

@IlyaSemenov
Copy link
Author

this library has been super-clear that a slugid is an encoded v4 Uuid

Absolutely, I didn't mean to present this is a bug — sorry for confusion.

But, from end user's point of view, v7 is really no different from v4, it's seemingly the same 128 bits / 32 hex chars string and I suppose slugifying process is agnostic to the nature of these 128 bits. I didn't even realize the version is encoded with the UUID itself.

@lotas
Copy link
Contributor

lotas commented Nov 25, 2024

Thanks for the suggestion.
I tried to upgrade the library to see if v7 would work out of the box, but there seems to be some bit-wise magic happening around encoded values, which produced some unexpected characters.
I'll check if it is possible to decode slugid back to the correct uuid version without loosing information.

In worst case, I think, we can just introduce a config option or have separate methods for different uuid versions

@petemoore
Copy link
Member

petemoore commented Nov 26, 2024

I agree that this library is specifically designed to work with v4 uuids, but that the encode method could be engineered to work with any of the original five versions defined in RFC 4472, or even the extended 8 versions defined in RFC 9562. It is unfortunate (and surprising) if the upstream uuid.parse() really does only work with v1-v5 uuids (I haven't tested), since there seems to be no encoding limitation in RFC 9562 that should prevent it from working with any of them (the same 6 bits for ver/var are used across all 8 uuid versions). If that is the case, it is probably better to fix the upstream uuid.parse function to account for newer uuid versions. Once this library picks up the newer version (usually via a dependabot PR) the v7 uuids should work with the encode function this library provides. Note, this library still does not provide a way of generating v7 "slugs", and the regexp that is documented in the README only applies to v4 slugs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants