Are you sure this improves performance? Did you measure it after making this change and compared it with before the change? Performance improvements are tricky.
As far as I understand useThreadsWithPermission will be called every time useThreadHasPermission is called, even with useMemo. This is a hook, not a component.
Here is a snack with this code but simplified: https://snack.expo.dev/@angelikaserwa/suspicious-violet-tortillas
After you press the button, the app will re-render and useThreadsWithPermission will be called, even with memo.

In D13874#387555, @angelika wrote:

Are you sure this improves performance?

Unfortunately, after some testing it doesn't seem that this solves the performance issues described in ENG-9862. As such, I'm continuing to investigate... currently working on setting up a custom environment with production data so I can do some better profiling (eg. enable React profiling).

Did you measure it after making this change and compared it with before the change?

I haven't measured directly, but I did do a subjective test and it doesn't seem like the performance is improved much.

That said, I'm fairly sure it's a positive change since we're memoizing something that can cause parts of useThreadsWithPermission to have to be recalculated.

As far as I understand useThreadsWithPermission will be called every time useThreadHasPermission is called, even with useMemo. This is a hook, not a component.

I probably should have explained this differently. What I meant is that we're currently forcing the useMemo in useThreadsWithPermission to be recalculated on every call from useThreadHasPermission. This diff fixes that.

Here is a snack with this code but simplified: https://snack.expo.dev/@angelikaserwa/suspicious-violet-tortillas
After you press the button, the app will re-render and useThreadsWithPermission will be called, even with memo.

I haven't checked the snack as I think we're on the same page about React behavior, but let me know if you think I'm missing something and I'll take a look :)

Ok, it makes sense, though it's a bit weird to me. This useMemo affects how another hook is calculated.

To be honest, I'm unsure why it's weird to you. On our team, we always memoize non-primitive inputs to hooks. I consider this to be good practice in React, and would advocate for it in any project I work on. I would also request changes on any diff where I saw somebody not memoizing non-primitive inputs to hooks, and hope you would do the same.

React depends a lot on comparing values to determine if things need to be recalculated or rerendered. For non-primitives, such comparisons occur by reference. This is how all dep lists work (such as in React.useMemo), as well as how React.memo works.

Maybe a comment would be useful?

If we left comments in places like this, we would have hundreds of such comments littering the codebase. I frankly don't think it would be useful...

Let's talk about this in our next 1:1. I can explain more about how/why this affects perf, as well as pull up some examples where similar mistakes caused React render cycles for us in the past.

I compiled some notes that we can go through in our next 1:1. Ahead of that, I figured it might help to share a list of React render cycles issues solved by adding React.useMemos like this:

• React render cycle example: D13465, ENG-9391
• React render cycle example: D10409, ENG-6139
• React render cycle example: D9297, ENG-5041
• React render cycle example: D8760, ENG-4605