-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
useInstanceId: Convert to typescript #43790
Conversation
Size Change: +28 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
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.
Great work here!
I've left a few comments re. cleaning the code up, but I've already tested the changes and they seem to work fine 🚀
The compose
package also seems to have a CHANGELOG, we should probably add an entry there as well
function useInstanceId( object: object ): number; | ||
function useInstanceId( object: object, prefix: string ): string; | ||
function useInstanceId< T extends string | number >( | ||
object: object, | ||
prefix: string, | ||
preferredId?: T | ||
): T; |
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.
Nice usage of TypeScript overloading!
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.
🚀
What?
Refactors
useIntanceId
to TypeScript instead of relying on JSDocs.Why?
When using
useInstanceId
in a typescript file it always returnsstring | number
. By refactoruseInstanceId
to typescript we can return a specific type based on what params you pass to the function.How?
Refactored
useIntanceId
to TypeScript.Before
After