[HOLD for payment 2024-12-25] [Performance] useOnyx doesn't return new data if the selector changes but the data in the store is the same

[pac-guerreiro]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

What performance issue do we need to solve?

Inefficient React component rendering when onyx collection data is updated, although we're only accessing unmodified entries of the onyx collection.

Example case: #49919 (review)

What is the impact of this on end-users?

App performance can degrade with the inefficient rendering

List any benchmarks that show the severity of the issue

N/A

Proposed solution (if any)

Mimic dependency array from some React hooks, like useEffect, to force useOnyx to fetch data if any dependency changes.

Something like useOnyx(key, options, [dependencyA]).

List any benchmarks after implementing the changes to show impacts of the proposed solution (if any)

N/A

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Version Number: v9.0.62-4
Reproducible in staging?: yes
Reproducible in production?: yes
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: #49919 (review)
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

View all open jobs on Upwork

Issue Owner

Current Issue Owner: @sonialiap

[melvin-bot]

Auto-assigning issues to engineers is no longer supported. If you think this issue should receive engineering attention, please raise it in #whatsnext.

[pac-guerreiro]

@mountiny can you make this issue internal? @fabioh8010 and I are working on it

[fabioh8010]

You tagged the wrong person @pac-guerreiro , it's @mountiny 😄

[pac-guerreiro]

Ups, sorry! 😅

[mountiny]

Sounds good

[pac-guerreiro]

Last friday I didn't work on this but tomorrow I'll open a draft PR with a possible solution

[pac-guerreiro]

I opened a draft PR with a possible solution for this issue - Expensify/react-native-onyx#599

If you all agree, I can also create an eslint rule to enforce the usage of the dependency array when developers pass variables in useOnyx selector. To achieve it, I'll need to adapt this and create an additional PR to add this new rule to https://github.com/Expensify/eslint-config-expensify

[pac-guerreiro]

I'm waiting on @fabioh8010 feedback on this since he has an alternate solution

[fabioh8010]

We are currently discussing the issue internally.

[pac-guerreiro]

Today @fabioh8010 and I discussed this issue internally. There are some ideas on the table on how to tackle this issue, so we'll continue discussing them tomorrow!

[pac-guerreiro]

Today @fabioh8010 continued our discussion, we are still trying to find a proper solution for this issue.

[pac-guerreiro]

Today I tried more possible solutions and discussed them with @fabioh8010. I've been having problems with infinite loops on getSnapshot. On Monday I'm going to discuss an alternative idea of @fabioh8010 for this performance issue.

[melvin-bot]

@fabioh8010, @mountiny, @pac-guerreiro Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[pac-guerreiro]

Yesterday I was able to provide a possible solution to the problem and created a draft PR - Expensify/react-native-onyx#601

@fabioh8010 provided a solution of his, which I didn't have the time to check yesterday but today I'll do it!

[pac-guerreiro]

Today I reviewed @fabioh8010 solution and we discussed it. We need to confirm which one is the best and how we should proceed next

[fabioh8010]

Taking this issue over Pedro since he's going to be OOO until December 12th

[melvin-bot]

@fabioh8010, @mountiny, @pac-guerreiro Eep! 4 days overdue now. Issues have feelings too...

[fabioh8010]

Update: We agreed to test my solution against E/App and if everything is ok, we will proceed with it.

[melvin-bot]

@fabioh8010, @mountiny, @pac-guerreiro 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[fabioh8010]

Created a WIP Draft PR, testing the solution against E/App.

[fabioh8010]

Will open the Bump PR today

[fabioh8010]

Bump PR created -> #53890

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.76-12 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Bump Onyx to 2.0.86 #53890

If no regressions arise, payment will be issued on 2024-12-25. 🎊

For reference, here are some details about the assignees on this issue:

• @fabioh8010 does not require payment (Contractor)
• @pac-guerreiro does not require payment (Contractor)

[melvin-bot]

Skipping the payment summary for this issue since all the assignees are employees or vendors. If this is incorrect, please manually add the payment summary SO.

[melvin-bot]

Triggered auto assignment to @sonialiap (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[melvin-bot]

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

[melvin-bot]

Auto-assign attempt failed, all eligible assignees are OOO.

[mountiny]

$250 to @sobitneupane No need for regression testing on this one

[sonialiap]

Payment summary:

• @sobitneupane $250 via NewDot