-
Notifications
You must be signed in to change notification settings - Fork 1
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
[FCE-929] Add useUpdatePeerMetadata hook #254
base: main
Are you sure you want to change the base?
Conversation
* @param peerMetadata string indexed record with metadata, that will be available to all other peers | ||
* @category Connection | ||
*/ | ||
updatePeerMetadata, |
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.
nitpick: This is already a static reference so useCallback
does not add anything here, it could be written as such:
updatePeerMetadata: RNFishjamClientModule.updatePeerMetadata<PeerMetadata>
but I'm ok with useCallback
solution as well.
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.
I was thinking about it - and decided to go for useCallback to made it more explicit. But I don't have strong opinion on which solution is better 🤷
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.
Then let's leave it as it is.
Description
Instead of static method, let's have hook that will allow updating metadata.
I also updated jsdoc comments - to remove deprecated methods from existing categories (so they are not easily discoverable in documentation)
Motivation and Context
Let's have unified API that rely mostly on hooks
Types of changes
not work as expected)
Checklist:
Screenshots (if appropriate)