-
-
Notifications
You must be signed in to change notification settings - Fork 173
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
onEmojiClick doesn't have access to the current state of the parent component #365
Comments
Hey @TKasperczyk, as you noted correctly, the most straightforward workaround is to use refs. But, as you also mentioned, it is cumbersome and counter intuitive. I'll shed some light on what's going on, and let's try to find together a solution for it that would be viable for you. The reason onEmojiClick gets an outdated state reference is the internal memoization of the picker. The picker uses memeoizeation because otherwise, each state update outside of the picker would trigger full rerender of all emojis (1000s of elements), which would cause significant lag. Instead, I memoize by all primitive props. The others are not factored in (cannot compare a function, for example), so state update outside of the picker do not change the onEmojiClick reference. There are two ways to handle it, as I see it:
How would you, as a consumer, want to use it? |
Thank you for your comprehensive explanation. I appreciate the suggested strategies on how to handle the issue. // ...
const onEmojiClick = React.useCallback((emoji: EmojiClickData) => {
alert(`Current state: ${myState}`);
}, [myState]);
// ...
<EmojiPicker onEmojiClick={onEmojiClick} /> Notably, this triggers the picker to only re-render when the state value changes, and not every time the parent does. But if users would still like to limit re-renders of the picker even further, they are welcome to use refs instead by having an empty dependency array in the This approach maintains what I believe most users anticipate when interacting with external libraries. While it's true that it may not be suitable for every scenario, it has the advantage of aligning with widespread, standardized practices. Let me know if you have any comments or need further clarification on the proposed approach. |
I was pulling my hair out trying to figure out why I couldn't build an additive string of emojis with Is there really no way to update the internal ref of |
Sorry for not responding earlier. @TKasperczyk, It's been quite difficult to concentrate on OSS work during the current war situation. @cr0ybot, as a workaround until I introduce one of these options that I described above (or go with @TKasperczyk's suggestion), one of these could work:
|
@TKasperczyk Essentially, what you're describing is a prerequisite to using the picker, meaning, everyone who wants to use the picker will have to do this. 100% boilerplate code that all consumers need to add. In that case, I want to reduce that and have it a part of the picker. I think that regardless of what I pick as an api change I will make that internal ref thing that I mentioned to unbreak most existing use-cases, and this will give us some time to discuss a longer-term solution. |
@cr0ybot try 4.5.3. I just releaesd it a few minutes ago. It is possible that it will solve it for you, if not, can you share with me a repro sandbox? I will try to get this fixed. |
@ealush Thanks for you work on this, it works now with |
Closing, since this is no longer an issue |
Consider the following example:
Clicking the button will set the value of
myState
to true, and that's what will be displayed at the beginning of the component. However, when I click on an emoji, the alert will still show "false". In other words, theonEmojiClick
callback doesn't have access to the current state of the parent component.Here's a snippet that presents the issue: https://codesandbox.io/s/infallible-sanderson-yx4jz8?file=/src/App.tsx:0-838
The current workaround is to use refs, however, it creates unnecessary complications, especially when working with state management libraries like redux.
The text was updated successfully, but these errors were encountered: