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

chore(deps): use cuid2 instead of cuid #147

Closed

Conversation

yoshinorin
Copy link
Member

@yoshinorin yoshinorin commented Feb 6, 2023

Why

https://github.com/paralleldrive/cuid has been deprecated and we have to use https://github.com/paralleldrive/cuid2 instead.

Breaking change?

I assume warehouse uses cuid to generate post id. So, all of the post ids in db.json will be changed if we merge this PR. I think the posts ids are not on the premise that permanently stored (If hexo has to store ids permanently, should use RDB). But this change may be a breaking change for some users if they are using any plugin and it depends on that premise.


Closes: #146

@yoshinorin
Copy link
Member Author

Hmm... We have to fix TS2362 & TS2322 errors before submit this PR.

@yoshinorin yoshinorin marked this pull request as draft February 6, 2023 14:11
@yoshinorin
Copy link
Member Author

I can't reproduce TS2362 & TS2322 errors locally...

@yoshinorin
Copy link
Member Author

yoshinorin commented Feb 16, 2023

Blocked by: #148

@yoshinorin yoshinorin force-pushed the chore/use-ucid2-instead-of-cuid branch from e675601 to d55c08d Compare February 28, 2023 12:59
@yoshinorin yoshinorin marked this pull request as ready for review February 28, 2023 13:02
@yoshinorin
Copy link
Member Author

Ready

Copy link
Member

@SukkaW SukkaW left a comment

Choose a reason for hiding this comment

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

cuid is indeed deprecated, but I don't see any reason to upgrade to cuid2. warehouse doesn't rely on cryptographic safety

@yoshinorin
Copy link
Member Author

cuid is indeed deprecated, but I don't see any reason to upgrade to cuid2. warehouse doesn't rely on cryptographic safety

Yes. We can suspend migrating from cuid to cuid2 until we really needed. (e.g. cuid does not work with Node.js v20)
I convert this PR to draft.

Thanks

@yoshinorin yoshinorin marked this pull request as draft March 1, 2023 15:20
@SukkaW
Copy link
Member

SukkaW commented Mar 1, 2023

Yes. We can suspend migrating from cuid to cuid2 until we really needed. (e.g. cuid does not work with Node.js v20) I convert this PR to draft.

Also ideally, we should consider migrating to a more performant id generation (instead of sticking with the cuid). warehouse is basically a database, and we can choose basically any safe random id generator we want.

Let's run a benchmark on cuid, cuid2, https://www.npmjs.com/package/uid, and https://www.npmjs.com/package/nanoid, so we can find out the fastest one.

@D-Sketon
Copy link
Member

a simple benchmark with https://github.com/ai/nanoid/blob/main/test/benchmark.js

cuid2                         42,848 ops/sec
nanoid                     4,442,592 ops/sec
cuid                         154,202 ops/sec

Non-secure:
uid                       65,298,000 ops/sec

@stevenjoezhang
Copy link
Member

stevenjoezhang commented Apr 18, 2024

Considering their performance and maintenance (update frequency), I think nanoid is a good alternative option. I will attempt to migrate to it.

Superseded by #244

@yoshinorin yoshinorin deleted the chore/use-ucid2-instead-of-cuid branch April 18, 2024 11:50
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.

4 participants