multiversx / mx-sdk-js-extension-provider

Signing provider for dApps: DeFi Wallet (browser extension).
Other
1 stars 3 forks source link

.isConnected() always returns true. .logout() does not remove the address from provider #4

Closed bogdanIuga closed 2 years ago

bogdanIuga commented 2 years ago

Hello,

I think there's an issue with the .isConnected() function. It always returns true, giving the fact that you check if it's truthy but in the constructor you're initalizing the address as empty.

Also, the .logout() method shouldn't remove the address from the provider.account? I've added this function on your example: image

But the address is still on the provider, after clicking on "Logout". The .isConnected() method returns true image

The function is added in the app.js from out-examples.

Hopefully I'm not doing anything wrong :). Using Angular image

juliancwirko commented 2 years ago

I missed the issue and created a PR, but just found the same: https://github.com/ElrondNetwork/elrond-sdk-erdjs-extension-provider/pull/6

it is also not concise across the providers, it would be good to have the same logic for isConnected, for example, wallet connect provider returns the isInitialized: https://github.com/ElrondNetwork/elrond-sdk-erdjs-wallet-connect-provider/issues/8

andreibancioiu commented 2 years ago

@bogdanIuga, @juliancwirko, thank you for raising the issue.

@juliancwirko, thank you for the fix :pray:

The new version has been released a few moments ago: https://github.com/ElrondNetwork/elrond-sdk-erdjs-extension-provider/releases/tag/v2.0.3

andreibancioiu commented 2 years ago

Regarding the uniformity of function names / interfaces across different providers - indeed, there are discrepancies between them at this moment.

However, the initialized state (kept in a private variable for the extension provider) and the connected state have slightly different semantics in the context of the extension provider, and we'd like to keep them explicitly separated, at least for the moment - this could be improved in the future.

All right to close this issue?

Thanks again :pray: