feat: WalletAccount non-breaking temp solution #1259
Conversation
|
What are the differences with PR1245? |
Please check now, I started PR but got pulled on HH |
|
In general, the idea is to have a non-breaking constructor args. So for existing users, this should continue to work as it was before, but now they can also provide an address or use static methods to avoid wallet popup. |
|
But we have still something async in the constructor, that is the root cause of PR 1245 creation. |
|
The imminent Issue is the wallet popup opening without the option to stop it. The more proper solution could go into the next mayor versions with breaking changes. |
There was a problem hiding this comment.
I think that this solution is the best compromise, even if we don't solve the initial problem without braking changes.
For Information, OKX wallet team is reporting that they have problems with this subject.
They are waiting a new version with connect.
So, we should go forward on this subject.
Co-authored-by: Petar Penović <[email protected]>
Co-authored-by: Petar Penović <[email protected]>
src/wallet/account.ts
Outdated
| if (!address.length) { | ||
| requestAccounts(this.walletProvider).then(([accountAddress]) => { | ||
| this.address = accountAddress.toLowerCase(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Let's add a deprecation warning here saying that connect should be use instead.
| provider: ProviderInterface, | ||
| walletProvider: StarknetWalletProvider, | ||
| cairoVersion?: CairoVersion, | ||
| silentMode: boolean = false |
There was a problem hiding this comment.
I like my implementation better. Separating "connect" and "connectSilent" is a better approach imo, then passing a boolean here
There was a problem hiding this comment.
OK will add connectSilent as alias on fixed connect, I think these best compromise on this.
|
If it's based on my PR, I would also suggest changing the base branch to my PR branch and then merge this PR as "Backwards compatibility" or something. |
tabaktoni
changed the base branch from
develop
to
feat/improve-wallet-account
I tried but there are conflicts as I changed all ex. connect so it is easier to just merge this. |
tabaktoni
changed the base branch from
feat/improve-wallet-account
to
develop
# [6.18.0](v6.17.0...v6.18.0) (2024-11-18) ### Features * WalletAccount non-breaking temp solution ([#1259](#1259)) ([84b267c](84b267c))
|
🎉 This issue has been resolved in version 6.18.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Motivation and Resolution
Based on #1245
This should be a temporary fix that can be merged as it should not contain breaking changes.
Checklist: