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/auto reconnect #34

Closed
wants to merge 22 commits into from
Closed

Conversation

Aanhane
Copy link
Contributor

@Aanhane Aanhane commented Sep 9, 2022

I'm wondering if recent changes to Vite have caused the issue as mentioned in #27 (Wallet not stays connected), so I decided to update vite in this package. Then I got into the update rabbit hole as things didn't compile anymore. I also made it easier for collaborators to run the demo locally, as I wanted a shorter debug cycle.

  • Updated Typescript
  • Updated Vite
  • Add Prettier config file
  • Add http-server dev dependency for easily running the demo
  • Make signTransaction in demo work again
  • Update vueuse
  • Markdownlint the README.md

Let's have the demo be deployed by this PR and then people that have the issue test it, as I don't have it locally on my demo after these updates.

The React package also struggled with the same issue, as seen in anza-xyz/wallet-adapter#527. I'm finding it hard to replicate/pinpoint the issue in this package.

Update:
I found out it's a timing issue. A while back the beforeunload changed to unload which caused the wallet adapter to sometimes trigger a disconnect event and clear the storage. I made it so that it's explicitly cleared only when the user disconnects, and not on any disconnect. (e.g. caused by the browser.)

@netlify
Copy link

netlify bot commented Sep 9, 2022

Deploy Preview for solana-wallets-vue-demo ready!

Name Link
🔨 Latest commit dc4ca47
🔍 Latest deploy log https://app.netlify.com/sites/solana-wallets-vue-demo/deploys/631c36c0aee42a0008bdfdaf
😎 Deploy Preview https://deploy-preview-34--solana-wallets-vue-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@Aanhane
Copy link
Contributor Author

Aanhane commented Sep 10, 2022

@lorisleiva if you could that a look when you have time, that would be great :). If you have any comments or questions, please let me know 🙏 .

URL to test: https://deploy-preview-34--solana-wallets-vue-demo.netlify.app/.

Related issue: #27

One thing I haven't been able to verify is whether Buffer is now needed as a polyfill. I added it to the demo to make it work, not sure if/how it will be for the compiled package? 🤷

@lorisleiva
Copy link
Owner

lorisleiva commented Sep 12, 2022

Hi there 👋

It's extremely difficult to grasp the changes of this PR with all the linting changes.

This PR claims to fix the auto-connect issue but I can't find which part of the code is trying to do that.

I'm also struggling the understand what this piece of code achieves since, Buffer needs to be polyfiled to work on the browser.

import { Buffer } from 'buffer';
window.Buffer = Buffer;

Would you be able to create a PR that just focuses on fixing that issue and, once that's working, I'm happy to accept infrastructure improvement such as better linting, etc.

Thank you for your work!

@Aanhane Aanhane closed this Sep 12, 2022
@Aanhane Aanhane deleted the fix/auto-reconnect branch September 12, 2022 08:21
@Aanhane
Copy link
Contributor Author

Aanhane commented Sep 12, 2022

Hello!

I understand that. 👍 It did become quite much when there was a domino effect of outdated packages.

Since I don't have the time to create new separate PRs for it, unfortunately I cannot continue this, sorry. I already went 100% over the allocated time I had for this fix.

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