-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
chore(deps): use nanoid instead of cuid #244
Conversation
Pull Request Test Coverage Report for Build 8733513242Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nanoid LGTM. It is faster and is collision resilient enough.
src/types/cuid.ts
Outdated
validate(value?: string): string { | ||
if (value && (value[0] !== 'c' || value.length !== 25)) { | ||
if (value && (value.length !== 21)) { | ||
throw new ValidationError(`\`${value}\` is not a valid CUID`); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the schema of the id has changed, previously generated db.json
will become invalid after the PR. It is a breaking change that we might want to avoid.
Let's keep the c
prefix, and let the nanoid generate random ids with the size of 24. Now previously generated db.json
is still valid and Hexo will not crash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. I will change its format to be compatible with old cuid.
check list
Description
See #147
Additional information