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

Improve performance and bundle size #537

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

DavideSegullo
Copy link

@DavideSegullo DavideSegullo commented Mar 3, 2024

Description and Motivation

This PR aims to improve the performance of the app and the overall experience:

  1. Fix the connection to some wallets, such as leap snap and others that currently do not work
  2. Support wallet connect, right now I have not implemented it, but you can easily do it in a separate PR, I didn't want to add too much to this PR
  3. Reduce the bundle size of the app and the first load js, this generally improves the speed of the app, especially in view of the possibility of supporting mobile
  4. Simplify the code, in general I wanted in some places to simplify some logic
  5. Added as bun package manager to improve dev experience, lower dependency install times.

TODO after this PR:

  1. Re-enable validation of TS types, which I currently saw were disabled.
  2. Fix linter configuration issues, currently they are not respected everywhere and in general there are several unfixed errors, separate from this PR
  3. Refactor the logic for signing messages, it can be improved to simplify it to avoid passing global parameters as functions to get signers and also adding dynamic imports in these steps, you can still reduce the size of the first load js.

Bundle size compare

Before:
Screenshot 2024-03-01 alle 00 08 19

After:
Screenshot 2024-03-04 alle 00 39 07

The impact on bundle size has been reduced by 9x, but I aim to reduce it again, you can go down further

Checklist:

  • I have read Migaloo's contribution guidelines.
  • My pull request has a sound title and description (not something vague like Update index.md)
  • All existing and new tests are passing.
  • I updated/added relevant documentation.
  • The code is formatted properly yarn lint.
  • The project builds and is able to deploy on Netlify yarn build.

Copy link

netlify bot commented Mar 3, 2024

Deploy Preview for exquisite-salamander-a1fe5e ready!

Name Link
🔨 Latest commit 426ef3f
🔍 Latest deploy log https://app.netlify.com/sites/exquisite-salamander-a1fe5e/deploys/65e509a2da71510008ee86ef
😎 Deploy Preview https://deploy-preview-537--exquisite-salamander-a1fe5e.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 28 (🟢 up 2 from production)
Accessibility: 95 (no change from production)
Best Practices: 92 (no change from production)
SEO: 83 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@DavideSegullo DavideSegullo marked this pull request as ready for review March 3, 2024 23:37
@DavideSegullo
Copy link
Author

Hola @nick134-bit, sorry to bother you, as I would like to add more enhancements, this PR is ready for review. When you can, could I get some feedback? Thank you!

@nick134-bit
Copy link
Contributor

Hey @DavideSegullo,
thank you for submitting this PR! Im not familiar with quirks and switching the wallet connector must be well thought out. We currently have several custom solutions for cosmos-kit in place, which might lead to non-functional wallet connectors for quirks, such as Station or Keplr Ledger.
Could you please provide more details on the major advantages of quirks, apart from the bundle size? Additionally, I am curious about how wallets are added and maintained for quirks. I noticed an open issue regarding Ledger support. Does this mean Ledger is currently unsupported on any wallet?
Thank you again for your contribution.

@DavideSegullo
Copy link
Author

Hey @DavideSegullo, thank you for submitting this PR! Im not familiar with quirks and switching the wallet connector must be well thought out. We currently have several custom solutions for cosmos-kit in place, which might lead to non-functional wallet connectors for quirks, such as Station or Keplr Ledger. Could you please provide more details on the major advantages of quirks, apart from the bundle size? Additionally, I am curious about how wallets are added and maintained for quirks. I noticed an open issue regarding Ledger support. Does this mean Ledger is currently unsupported on any wallet? Thank you again for your contribution.

Hola @nick134-bit thank you for your message, I understand the risks and doubts in changing the wallet manager, yes I saw that there were custom logics build on cosmos-kit, but most of these can be removed as they are already handled, so this would help simplify the code.

Reducing the bundle size is not the only benefit of course, for example:

  1. The state is accessible from the whole app, so there is no need to make calls to functions in which you manually pass the signer, thus having globally accessible utilities that allow you to reduce the code and in general the complexity of the logic
  2. The logic you write to work with transactions, etc., can also be reused on react-native should you decide to make a native app tomorrow, so the only thing to manage in that case would be the UI, but thanks to the deeplink you can easily connect to the installed native apps.

About the issue regarding ledger support, it doesn't refer to ledger support in wallets, those work on keplr etc., it refers to the ability to directly connect the ledger to the app without going through any wallet to sign a tx via USB connection.

Regarding wallets, terra station at the moment is totally supported even on chains other than terra2 (I did several tests on juno signed tx for cosmwasm), the only thing they don't allow you to do at the moment is to sign tx with direct sign.

The wallets are maintained publicly on PR by those who suggest new additions and from quirks team, via discord or github if anyone want support to integrate some wallet that is missing we are available, but also by staying up to date as much as possible with cosmos wallet registry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants