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 taquito #413

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

update taquito #413

wants to merge 10 commits into from

Conversation

Zir0h
Copy link
Contributor

@Zir0h Zir0h commented May 4, 2024

fixes #397

@melMass
Copy link
Member

melMass commented May 4, 2024

Finally!
Very nice.
Did you test it?

@Zir0h
Copy link
Contributor Author

Zir0h commented May 4, 2024

Finally! Very nice. Did you test it?

yes, but only basic stuff, sync/unsync collect, I'll check more later this afternoon

@Zir0h
Copy link
Contributor Author

Zir0h commented May 4, 2024

ehh, wait, it doesn't look like this is the release the kukai team wants to have 😅

2024-05-04-113115_2013x227_scrot

@Zir0h Zir0h marked this pull request as draft May 4, 2024 09:34
@Zir0h
Copy link
Contributor Author

Zir0h commented May 4, 2024

ecadlabs/taquito@4360f9c still needs to be adapted in the teia code

@melMass
Copy link
Member

melMass commented May 4, 2024

when a wallet would disconnect the teia app wouldn't know how to deal with it

Is it a known issue I'm not aware of? 😅


I think adapting shouldn't be too hard but I just wonder if we shouldn't wait for after beta? Or is it urgent to update for another reason?

Cc @CousinIt

@Zir0h
Copy link
Contributor Author

Zir0h commented May 4, 2024

when a wallet would disconnect the teia app wouldn't know how to deal with it

Is it a known issue I'm not aware of? 😅

I think adapting shouldn't be too hard but I just wonder if we shouldn't wait for after beta? Or is it urgent to update for another reason?

Cc @CousinIt

no, I don't think there's an immediate issue

yeah, I'd like to wait for a full release too, that's why I also marked this PR as a draft for now

@Zir0h
Copy link
Contributor Author

Zir0h commented May 4, 2024

The BeaconEvent stuff does seem to be causing some issues, but I don't know if we really need that 🤔

✘ [ERROR] Could not read from file: /home/user/teia-ui/vite-compatible-readable-stream

    ../node_modules/cipher-base/index.js:2:24:
      2 │ var Transform = require('stream').Transform
        ╵                         ~~~~~~~~

8:18:01 PM [vite] error while updating dependencies:
Error: Build failed with 1 error:
../node_modules/cipher-base/index.js:2:24: ERROR: Could not read from file: /home/user/teia-ui/vite-compatible-readable-stream
    at failureErrorWithLog (/home/user/teia-ui/node_modules/esbuild/lib/main.js:1649:15)
    at /home/user/teia-ui/node_modules/esbuild/lib/main.js:1058:25
    at /home/user/teia-ui/node_modules/esbuild/lib/main.js:1525:9
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

@Zir0h
Copy link
Contributor Author

Zir0h commented May 8, 2024

I pushed this to https://preview.teia.art/

Copy link

github-actions bot commented May 8, 2024

PR Preview Action v1.4.7
🚀 Deployed preview to https://teia-community.github.io/teia-ui/previews/pr-413/
on branch gh-pages at 2024-05-18 15:19 UTC

@Zir0h
Copy link
Contributor Author

Zir0h commented May 8, 2024

I updated everything except react-pdf and eslint, because they gave errors and I'm not smart enough to fix them 😅

@Zir0h Zir0h marked this pull request as ready for review May 8, 2024 13:09
@Zir0h
Copy link
Contributor Author

Zir0h commented May 8, 2024

Sometimes it works, sometimes it does not (with walletconnect wallets)
2024-05-08-202252_1200x293_scrot

@Zir0h
Copy link
Contributor Author

Zir0h commented May 8, 2024

The error is related to the static build, using the node dev webserver gives no issues :/

@Zir0h
Copy link
Contributor Author

Zir0h commented May 8, 2024

Uncaught (in promise) TypeError: "list" argument must be an Array of Buffers
    concat contracts-B2xcCTA_.js:11
    JC crypto.js:166
    onNewAccount DAppClient.js:1775
    requestPermissions DAppClient.js:1013
    requestPermissions taquito-beacon-wallet.es6.js:91
    fs taquito-beacon-wallet.es6.js:30
    fs taquito-beacon-wallet.es6.js:26
    requestPermissions taquito-beacon-wallet.es6.js:90
    sync userStore.ts:203
    $ Header.jsx:148
    React 16

@Zir0h
Copy link
Contributor Author

Zir0h commented May 8, 2024

feedback from the kukai devs:
image

@Zir0h
Copy link
Contributor Author

Zir0h commented May 8, 2024

So I'm guessing we're running into the "˚ shimmed, but too complex to polyfill fully. Avoid if at all possible. Some bugs and partial support expected." disclaimer from https://www.npmjs.com/package/rollup-plugin-polyfill-node 🤣

so, this update is probably a no-go

@melMass
Copy link
Member

melMass commented May 14, 2024

so, this update is probably a no-go

Yep I think too unless there is a real reason to. Once it's "stable" or not beta we can try again, I'm just hoping they are now properly bundling it so we can remove 3/4th of our vite config (all this should be done upstream ecadlabs/taquito#1281 (comment))

@Zir0h
Copy link
Contributor Author

Zir0h commented Aug 27, 2024

https://github.com/ecadlabs/taquito/releases/tag/20.0.2-beta.2 looks like all the messing around that I did in this PR won't be necessary. I think it's better to create a new PR (when taquito gets out of beta) and trash this one?

@melMass
Copy link
Member

melMass commented Aug 27, 2024

Yeah 😅
I'm also willing to help of cleaning up our vite config since I introduced a bunch of hacks we might not need anymore

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.

[feat] update beacon
2 participants