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

Update sdk to 13 #23

Merged
merged 11 commits into from
Apr 26, 2023
Merged

Update sdk to 13 #23

merged 11 commits into from
Apr 26, 2023

Conversation

davidyuk
Copy link
Member

No description provided.

@davidyuk davidyuk marked this pull request as ready for review March 30, 2023 05:17
@davidyuk davidyuk force-pushed the feature/update-sdk branch from d2c0caa to 906ffbe Compare April 8, 2023 05:25
@davidyuk davidyuk changed the title Update sdk to 13 beta 0 Update sdk to 13 Apr 8, 2023
@davidyuk davidyuk requested review from thepiwo and marc0olo April 8, 2023 05:26
@thepiwo
Copy link
Collaborator

thepiwo commented Apr 12, 2023

would it be also good to migrate to vite and eventually pinia?

Copy link
Contributor

@marc0olo marc0olo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see following log in the console:

caught (in promise) NoWalletConnectedError: You are not connected to Wallet
    at Proxy._ensureConnected (AeSdkAepp.ts:172:1)
    at Proxy._ensureAccountAccess (AeSdkAepp.ts:176:1)
    at Proxy._resolveAccount (AeSdkAepp.ts:73:1)
    at get address (AeSdkBase.ts:131:1)
    at Reflect.get (<anonymous>)
    at Object.get (reactivity.esm-bundler.js:492:1)
    at fetchAccountInfo (wallet.js:87:1)
    at connectToNode (wallet.js:101:1)
    at Proxy.onNetworkChange (wallet.js:51:1)
    at updateNetwork (AeSdkAepp.ts:106:1)

can this be ignored? other than that the update looks good to me 👍

note: this is shown in the console before connecting the wallet. I am not sure if this is already a "problem" in the latest version of the boilerplate

@marc0olo
Copy link
Contributor

eventually pinata

do you really mean pinata or did you want to say pino? 🤔

@davidyuk
Copy link
Member Author

You are not connected to Wallet

It is not intended to be there, I'm looking for a fix

@thepiwo
Copy link
Collaborator

thepiwo commented Apr 20, 2023

@marc0olo pinia sorry

@davidyuk
Copy link
Member Author

I've fixed the error in a separate commit. The bug was because at the last moment before release I made sdk to call onNetworkChange right after connection, to avoid duplication of network initialization on aepp side aeternity/aepp-sdk-js#1779.

Would be really good to update this repo to vite and pinia, but this would break the integration of contract builder in #24, so I won't do it 🙃

@marc0olo marc0olo self-requested a review April 24, 2023 06:24
@davidyuk davidyuk merged commit 2e607c7 into master Apr 26, 2023
@davidyuk davidyuk deleted the feature/update-sdk branch April 26, 2023 07:46
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.

3 participants