Remove function binds in render wherever possible, use PureComponent

[Gargron]

Function.bind() creates a new function, doing it in render() means that every render, lots of new functions are created. Furthermore, new functions are different from old ones, even when the underlying function is the same, which means if it's passed as a prop to a child component, the child component re-renders.

Doing the bind inside class constructor ensures this happens only once.

Furthermore, inheriting from React.PureComponent means that components do a simple equality comparison on their props and state, and if they are equal, re-rendering is skipped altogether. This is safe to use when there are no side-effects (e.g. when components only need props+state)

[Gargron]

Oof, looks like I'll need to dig into the code again to resolve this conflict

[EtienneLem]

It looks good, but I would rather keep the Emoji component as a functional component for the time being, which is even more performant than a pure component. We wrote a blog post about that: https://medium.com/missive-app/45-faster-react-functional-components-now-3509a668e69f

In other words: With {Emoji()} instead of <Emoji /> we’re completely skipping react internals. Giving the huuuuge list of emojis, it really adds up when mounting them at the same time. It could definitely be argued that we should only render visible ones (#6), but until then I think it was slightly more optimized like it was before. Thoughts? 😄

[Gargron]

Interesting! Thanks for the link. I have not realized this was what was happening. But if you're doing bind() inside the function call, you're still creating new functions every time, and there's gotta be a perf cost from doing that... Oh well, can you revert the Emoji changes from this branch before merge or should I do that?

Speaking of virtualization, I was considering working on that. emojione-picker which we were using before had that and emoji-mart feels a little bit heavier during mount

At last, I have used a different, self-made component for the skin tone selector instead of the emoji-mart one, that looks like this (screenshot in PR): mastodon/mastodon#5046 If you've used Discord, it's basically like that. Do you want me to submit that upstream?

[EtienneLem]

Oh well, can you revert the Emoji changes from this branch before merge or should I do that?

I can do it, no problem.

skin tone selector […] Do you want me to submit that upstream?

Definitely a must to be able to set skin tone somewhere else since preview can be hidden w/ showPreview. Would be nice if the emoji could be customized 😄