Skip to content
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

[HIGH] Fully integrate Reassure into Onyx to improve performance monitoring and prevent regressions #56483

Open
mountiny opened this issue Feb 6, 2025 · 12 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Weekly KSv2

Comments

@mountiny
Copy link
Contributor

mountiny commented Feb 6, 2025

Background

Onyx is the backbone of our application, ensuring fast and reliable data access. To maintain a high-quality user experience, it is critical to monitor and measure its performance. Effective tracking allows us to detect regressions early and continuously improve efficiency.

Problem

When Onyx functions experience performance regressions, there is currently no automated way to detect them early, which makes it difficult to diagnose slowdowns and optimize performance. Without comprehensive performance testing in place, issues may go unnoticed until they affect end users, reducing app responsiveness and developer confidence in code changes.

Solution

Expand Reassure testing coverage in Onyx to ensure performance is continuously monitored and regressions are detected early.

Phase 1

  1. Update Reassure version to match E/App (or latest).
  2. Implement Reassure tests for all functions in Onyx.ts, OnyxUtils.ts, OnyxCache.ts, and OnyxConnectionManager.ts to maximize coverage.
  3. Add guidelines and update the PR/Reviewer Checklist to ensure all new Onyx functions include Reassure test coverage.

Phase 2

  1. Extend Reassure tests to remaining utilities like utils.ts and Str.ts.
  2. Explore testing render counts for useOnyx.ts to detect unnecessary re-renders.
    This approach will improve Onyx’s reliability by providing continuous, automated performance validation, ensuring that any performance degradation is caught before it impacts users.
Issue OwnerCurrent Issue Owner: @fabioh8010
@mountiny mountiny added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 labels Feb 6, 2025
@mountiny mountiny self-assigned this Feb 6, 2025
Copy link

melvin-bot bot commented Feb 6, 2025

Triggered auto assignment to @joekaufmanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@mountiny
Copy link
Contributor Author

mountiny commented Feb 6, 2025

Waiting for Callstack assignee to comment

@mountiny mountiny added the Weekly KSv2 label Feb 6, 2025
@joekaufmanexpensify joekaufmanexpensify removed the Daily KSv2 label Feb 6, 2025
@fabioh8010
Copy link
Contributor

Hi, I'm Fábio from Callstack - expert agency - and I would like to work on this issue.

@fabioh8010
Copy link
Contributor

Will start working on this next week (Tuesday, will be OOO on Monday).

@fabioh8010
Copy link
Contributor

Updates:

  • Started to work on this today.

@fabioh8010
Copy link
Contributor

Updates:

  • I'm splitting the work with other tasks, but slowly making progress here. Will publish a WIP Draft PR soon.

@fabioh8010
Copy link
Contributor

Updates:

  • Implemented most of the tests for Onyx.ts.
  • Created WIP Draft PR here.

Copy link

melvin-bot bot commented Feb 20, 2025

@fabioh8010 @mountiny @joekaufmanexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@joekaufmanexpensify
Copy link
Contributor

@fabioh8010 will a PR be up this week?

@fabioh8010
Copy link
Contributor

@joekaufmanexpensify I'm more confident to have the PR ready next week.

@joekaufmanexpensify
Copy link
Contributor

Got it, sounds good. Thanks for the update!

@fabioh8010
Copy link
Contributor

Updates:

  • Implementing tests for OnyxUtils.ts.

@mallenexpensify mallenexpensify changed the title Fully integrate Reassure into Onyx to improve performance monitoring and prevent regressions [HIGH] Fully integrate Reassure into Onyx to improve performance monitoring and prevent regressions Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Weekly KSv2
Projects
Status: HIGH
Development

No branches or pull requests

3 participants