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

perf: Optimizing Loading time for Injected Wallets #704

Merged
merged 15 commits into from
Feb 27, 2023

Conversation

HarisSQA
Copy link
Contributor

@HarisSQA HarisSQA commented Feb 17, 2023

Description

This PR is about reducing the amout of time for the setupWalletSelector when all wallets are included in the modules array.

Went for many small commits only to clarify why and where the changes were necessary.

  • Changed the default timeout from 200ms to 100ms for waitFor helper function.
  • Check for isSignedIn() on Sender and Finer Wallet only if the wallet is installed this speeds up the loading time up to 300ms when wallet is not installed.
  • Fixed bug in the isInstalled function for Nightly and xDefi
  • Used waitFor helper function for the isInstalled function for Narwallets (by default this function used to wait up to 2s when wallet was not installed).
  • Removed waitFor from places where it was not needed.
  • Changed the timeout in detectEthereumProvider in Neth Account to 100ms.
  • Moved the check for if mobile before checking for isInstalled in injected wallets since we return null for these wallets in mobile devices anyway so we don't have to wait and check for isInstalled function.

Demo

To see the difference disable all wallet extensions and try to load these two examples:

Live Demo including these changes https://kujtimprenkusqa.github.io/wallet-selector/
Live Demo of current change in main branch: https://near.github.io/wallet-selector/

Conclusion

(On Desktop) These changes when all wallets are included in the modules array speed up the setupWalletSelector up to 90%.
(On Mobile) There's no delay/waiting anymore.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Type of change.

  • FIX - a PR of this type patches a bug.
  • FEATURE - a PR of this type introduces a new feature.
  • BUILD - a PR of this type introduces build changes.
  • CI - a PR of this type introduces CI changes.
  • DOCS - a PR of this type introduces DOCS improvement.
  • STYLE - a PR of this type introduces style changes.
  • REFACTOR - a PR of this type introduces refactoring.
  • PERFORMANCE - a PR of this type introduces performance changes.
  • TEST - a PR of this type adds more tests.
  • CHORE - a PR introduces other changes than the specified above.

@github-actions github-actions bot changed the title Optimizing Loading time perf: Optimizing Loading time Feb 17, 2023
@AZbang
Copy link
Contributor

AZbang commented Feb 21, 2023

@HarisSQA @kujtimprenkuSQA
In general, I also noticed that Wallet-selector has long time for initializing when loading the web page.
And this happens even if the desired wallet was already chosen in the previous session.

I suggest if the user already has a selected wallet - only it is initialized, and the rest are initialized in the background. This should accelerate the load until the application has API selected wallet in the last session

@kujtimprenkuSQA
Copy link
Contributor

@HarisSQA @kujtimprenkuSQA In general, I also noticed that Wallet-selector has long time for initializing when loading the web page. And this happens even if the desired wallet was already chosen in the previous session.

I suggest if the user already has a selected wallet - only it is initialized, and the rest are initialized in the background. This should accelerate the load until the application has API selected wallet in the last session

Thank you for your feedback, the loading time in Desktop is heavily impacted by the extensions some of the injected (extension) wallets take some time to be added on the window object, and when we check too soon for window.walletname the isInstalled() method would sometimes return false.

At least the idea of this PR is to improve the loading time in general and also on mobile devices for example for injected (extension) wallets we don't need to wait and check if these wallets are installed because by default they will never be available there.

@AZbang
Copy link
Contributor

AZbang commented Feb 21, 2023

@kujtimprenkuSQA But why we need wait until all wallets are injected, if this can be done during a direct call to a specific wallet?

const selector = await setupWalletSelector([ ... ]) // very fast!
// And then we wait if necessary
await selector.wallet("sender")

This is needed because sometimes the isInstalled function is returning false even though the extension is installed.
This is needed because sometimes even when the extension is installed the isInstalled function is returning false.
…stalled

This will speed up the setupWalletSelector on mobile devices since we return null for these wallets anyway we do not need to wait and check if these wallets are installed or not.
@kujtimprenkuSQA kujtimprenkuSQA changed the title perf: Optimizing Loading time perf: Optimizing Loading time for Injected Wallets Feb 27, 2023
@kujtimprenkuSQA kujtimprenkuSQA merged commit 097e472 into dev Feb 27, 2023
@kujtimprenkuSQA kujtimprenkuSQA deleted the optimizingLoadingtime branch February 27, 2023 13:44
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.

5 participants