Skip to content

fix: replace useId with v4 #907

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

Conversation

maosin77
Copy link

@maosin77 maosin77 commented Jan 18, 2023

useId was introduced in React 18 but a lot of projects still use React 17.
to achieve backward compability we could replace useId with uuid.

This PR:

  • replaces useId with uuid

@gabrieljablonski gabrieljablonski linked an issue Jan 18, 2023 that may be closed by this pull request
@gabrieljablonski
Copy link
Member

gabrieljablonski commented Jan 18, 2023

99% of the time when using the uuid package, you can switch to nanoid, which is an order of magnitude smaller when it comes to bundle size, and often achieve the same goal.

While I do agree making the package dependent on React 18 just because of one call to useId() may not be the best, we should find an approach that does not require adding a new dependency.

@gabrieljablonski gabrieljablonski added the Dependencies Pull requests that update a dependency file label Jan 18, 2023
@gabrieljablonski gabrieljablonski marked this pull request as draft January 18, 2023 14:06
@danielbarion
Copy link
Member

Hi, I agree with @gabrieljablonski.

I don't like the idea of including uuid again in the project to generate the IDs, as you can see in the image, we reduced the bundle size from v4 to v5, one of the reasons is the drop of uuid package.

image

But I do agree that we should support React <= v18 as v4 still has a few issues and it is not maintained anymore.

@gabrieljablonski
Copy link
Member

gabrieljablonski commented Jan 18, 2023

After looking at the source code, I've realized a unique id is not required for the provider to work properly. In a previous version, a unique id was essential, so useId() ended up coming along when it was no longer needed.

Along with some other improvements #911 drops useId() usage, so React v18 will no longer be required.

@maosin77
Copy link
Author

maosin77 commented Jan 19, 2023

Thanks for your comments, I didn't know nanoId. I hope this change will bring value to others as well :) My build passed so all good! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Build warnings related to 'useId'.
3 participants