Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

shortid is deprecated #2549

Open
wengtad opened this issue Jan 17, 2021 · 6 comments
Open

shortid is deprecated #2549

wengtad opened this issue Jan 17, 2021 · 6 comments
Assignees
Labels
deprecation in progress indicates that issue/pull request is currently being worked on

Comments

@wengtad
Copy link

wengtad commented Jan 17, 2021

Hi, thanks for this project. I am realized shortid is listed deprecated in their package.

shortid is deprecated, because the architecture is unsafe.

They do recommend using Nano ID and the bundle size is smaller according to Bundlephobia.
I am not sure will it have any impact if change from shortid to any other similar package.

@matteovivona matteovivona added deprecation good first issue indicates an issue is good for a first time contributor help wanted indicates that an issue is open for contributions labels Jan 18, 2021
@SamuelQZQ
Copy link
Contributor

@tehkapa I'd like to be assigned to this task.

@matteovivona
Copy link
Contributor

@SamuelQZQ okay. let's be very careful if Nano ID works the same way as Short ID

@matteovivona matteovivona added in progress indicates that issue/pull request is currently being worked on and removed good first issue indicates an issue is good for a first time contributor help wanted indicates that an issue is open for contributions labels Jan 19, 2021
@SamuelQZQ
Copy link
Contributor

@tehkapa The shortid uses a dynamic length id. But the nanoid use a fixed-length id. So, after migrate to nanoid, the question is, what length should we use? I think 9 or 10 is enough. Here is a collision calculator: Nano ID Collision Calculator

@SamuelQZQ
Copy link
Contributor

@tehkapa
Finally, I found that the nanoid above 3.0 met a bug in Create React App, which only fixed after Create React App 4.0.
See: nanoid doc about Create React App.

We have two choices:

  • Use the nanoid 2.0 first, and upgrade it to the latest version after we upgrade the react-scripts.
  • Keep using shortid, migrate to nanoid after we upgrade the react-scripts.

Personally prefer the latter.

@matteovivona
Copy link
Contributor

damn these updates are getting more and more complicated. @HospitalRun/core-contributors what do you think?

@blestab
Copy link
Contributor

blestab commented Jan 24, 2021

  • Keep using shortid, migrate to nanoid after we upgrade the react-scripts.

Sounds like a plan

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
deprecation in progress indicates that issue/pull request is currently being worked on
Projects
None yet
Development

No branches or pull requests

4 participants