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

fix: freeze when opening a live-app [LIVE-13411] #7625

Merged
merged 1 commit into from
Aug 20, 2024
Merged

Conversation

Justkant
Copy link
Contributor

@Justkant Justkant commented Aug 20, 2024

✅ Checklist

  • npx changeset was attached.
  • Covered by automatic tests. Not really possible to test a perf regression, I validated the improvement locally
  • Impact of the changes:
    • Web3Hub live-app opening (still behind a disabled FF)
    • Discover and other live-apps (should open faster now)

📝 Description

This fix (#7230) introduced a performance regression in some cases (e.g. when a lot of tokens are defined in the currencies field of the manifest) as we now get the currency list again and again.
This PR only removes the listing of currencies that we were doing on all apps even if it was not using the dapp browser v3, this helps most apps for the moment but we still need to improve the performance of the currency listing.

In a next PR we'll try to remove the spreading done on tokens, only do the minimal work necessary to get the informations and try to improve caching on this by instead listening to changes to only pull currencies when it updates.

❓ Context


🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.
  • Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

@Justkant Justkant self-assigned this Aug 20, 2024
@Justkant Justkant requested a review from a team as a code owner August 20, 2024 07:28
Copy link

vercel bot commented Aug 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

5 Skipped Deployments
Name Status Preview Comments Updated (UTC)
ledger-live-docs ⬜️ Ignored (Inspect) Aug 20, 2024 7:28am
ledger-live-github-bot ⬜️ Ignored (Inspect) Aug 20, 2024 7:28am
native-ui-storybook ⬜️ Ignored (Inspect) Aug 20, 2024 7:28am
react-ui-storybook ⬜️ Ignored (Inspect) Aug 20, 2024 7:28am
web-tools ⬜️ Ignored (Inspect) Aug 20, 2024 7:28am

@live-github-bot live-github-bot bot added the mobile Has changes in LLM label Aug 20, 2024
@Justkant Justkant merged commit 2871edd into develop Aug 20, 2024
38 of 40 checks passed
@Justkant Justkant deleted the bugfix/LIVE-13411 branch August 20, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mobile Has changes in LLM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants