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

feat: Allow UUIDv6/7/8 in UUIDField validation #221

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Conversation

ide
Copy link
Member

@ide ide commented Apr 8, 2024

Why

Currently the UUIDField type supports only UUIDv1-5 and to use a newer UUID version we need to use StringField.

How

Manually implemented UUID validation, adding support for versions 6, 7, and 8. Also added support for the "max" UUID, which is now part of the spec.

Test Plan

Added a UUIDv7 value (from https://uuid7.com/) to the test cases.

Why
---
Currently the UUIDField type supports only UUIDv1-5 and to use a newer UUID version we need to use StringField.

How
---
Manually implemented UUID validation, adding support for versions 6, 7, and 8. Also added support for the "max" UUID, which is now part of the spec.

Test Plan
---
Added a UUIDv7 value (from https://uuid7.com/).
@ide ide requested a review from wschurman as a code owner April 8, 2024 22:40
Copy link

codecov bot commented Apr 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.46%. Comparing base (9c43a56) to head (d4f06a2).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #221   +/-   ##
=======================================
  Coverage   95.46%   95.46%           
=======================================
  Files          79       79           
  Lines        2074     2074           
  Branches      258      282   +24     
=======================================
  Hits         1980     1980           
+ Misses         93       88    -5     
- Partials        1        6    +5     
Flag Coverage Δ
integration 95.46% <100.00%> (ø)
unittest 95.46% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -1,7 +1,9 @@
import { validate as validateUUID } from 'uuid';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this a dev dependency?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out uuid is used by StubDatabaseAdapter, which ends up getting exported as part of the package's production interface. So for now the right thing to do AFAIK is to keep it as as dependency. If we introduced @expo/entity-testing as a separate package, we could make uuid a devDep of @expo/entity but I don't think that'd get us much.

@ide ide merged commit cc6f3dc into main Apr 9, 2024
3 checks passed
@ide ide deleted the @ide/uuid-validation branch April 9, 2024 01:17
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

Successfully merging this pull request may close these issues.

None yet

2 participants