onEmojiClick doesn't have access to the current state of the parent component

[TKasperczyk]

Consider the following example:

const [myState, setMyState] = React.useState(false);
const getMyStateText = () => (myState ? "true" : "false");

return (
    <div>
        Current state: {getMyStateText()}
        <button
            onClick={() => {
                setMyState(!myState);
            }}
        >
            Toggle state
        </button>
        <EmojiPicker
            onEmojiClick={() => {
                alert(`Current state: ${getMyStateText()}`);
            }}
        />
    </div>
);

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, the onEmojiClick 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.

[ealush]

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:

1. Simple, requires additional prop: Add another prop that serves as your external mem-key, it will trigger an update each time it changes. It is the simples to add on my end, but would be harder to discover, and require yet another step on your end.

2. More complex, without API changes Store all non-primitives in an internal ref, and pass them down regardless of memoization stage. This is slightly more complex on the internal picker implementation, and the main drawback is that it might miss some cases (as you experience now).

How would you, as a consumer, want to use it?

[TKasperczyk]

Thank you for your comprehensive explanation. I appreciate the suggested strategies on how to handle the issue. I believe the most consistent approach would be to make the user responsible for handling re-renders. In other words, you should shift towards utilizing the standard export default React.memo(EmojiPicker) strategy instead of the current memoization logic (where you ignore non-primitive prop changes). This means that EmojiPicker would re-render whenever the parent renders due to non-primitive prop changes. However, this is precisely why we should use React.useCallback, a standard that gracefully passes the responsibility of limiting unnecessary re-renders to the user. Here's what I mean:

// ...
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 useCallback declaration, with myState as a ref.

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. Nevertheless, if I were to choose between one of the proposed strategies, I'd go for the first one - a mem-key prop that would allow us to manually trigger re-renders of the EmojiPicker component.

Let me know if you have any comments or need further clarification on the proposed approach.

[cr0ybot]

I was pulling my hair out trying to figure out why I couldn't build an additive string of emojis with onEmojiClick, even using useCallback.

Is there really no way to update the internal ref of onEmojiClick while also not rerendering the entire component? Or is it just that you want a holistic solution for all props instead of only onEmojiClick?

[ealush]

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:

1. use refs
2. do not reference the state in your onEmojiClick function, but rather only set the next state using a callback function
3. add a key property to the picker. Its value should be your current emoji state.

[ealush]

@TKasperczyk I agree that this would give more granular control to the consumer side, and this is also something that's quite standard, but I am looking at it slightly differently.

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. As you noted, adding the ref on your end solves it, but it is icky, because it requires working around the component. I imagine having everyone use useCallback as a similar thing.

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.

[ealush]

@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.

[cr0ybot]

@ealush Thanks for you work on this, it works now with useCallback!

[ealush]

Closing, since this is no longer an issue