Skip to content

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

Closed
yoshinorin wants to merge 1 commit intohexojs:masterfrom
yoshinorin:chore/use-ucid2-instead-of-cuid
Closed

chore(deps): use cuid2 instead of cuid#147
yoshinorin wants to merge 1 commit intohexojs:masterfrom
yoshinorin:chore/use-ucid2-instead-of-cuid

Conversation

@yoshinorin
Copy link
Member

@yoshinorin yoshinorin commented Feb 6, 2023 •
edited
Loading

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 yoshinorin mentioned this pull request Feb 6, 2023
Closed
@yoshinorin
Copy link
Member Author

yoshinorin commented Feb 6, 2023

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

yoshinorin commented Feb 15, 2023

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

@yoshinorin
Copy link
Member Author

yoshinorin commented Feb 16, 2023 •
edited
Loading

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

yoshinorin commented Feb 28, 2023

Ready

SukkaW
SukkaW requested changes Feb 28, 2023
View reviewed changes
Copy link
Member

@SukkaW SukkaW left a comment •
edited
Loading

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

yoshinorin commented Mar 1, 2023

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 •
edited
Loading

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

D-Sketon commented Oct 17, 2023

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 •
edited
Loading

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

@stevenjoezhang stevenjoezhang mentioned this pull request Apr 18, 2024
Merged
2 tasks
@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

Reviewers

@SukkaW SukkaW SukkaW requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yoshinorin @SukkaW @D-Sketon @stevenjoezhang