marigold-dev / training-dapp-shifumi

shifumi mobile training (Ionic React)
1 stars 1 forks source link

WIP - Vite migration #4

Closed dvkam closed 1 year ago

dvkam commented 1 year ago

DO NOT MERGE YET

I migrated the App to Vite.

However I noticed some odd behaviour in both versions (CRA and Vite) - when I try to submit a tx with Temple the operation is never even added to the mempool (I think) - so it triggers the timeout every time. However when I switch to Kukai it works. Other people experience a similar behaviour in other situations: https://github.com/ecadlabs/taquito/issues/1690

What needs to be done:

Will finish this asap.

zamrokk commented 1 year ago

This is weird, I never noticed this and I have done this training quite a lot

Do you have a complete scenario yo reproduce it?

Le mar. 11 juil. 2023 à 17:44, dvkam @.***> a écrit :

DO NOT MERGE YET

I migrated the App to Vite.

However I noticed some odd behaviour in both versions (CRA and Vite) - when I try to submit a tx with Temple the operation is never even added to the mempool (I think) - so it triggers the timeout every time. However when I switch to Kukai it works. Other people experience a similar behaviour in other situations: ecadlabs/taquito#1690 https://github.com/ecadlabs/taquito/issues/1690

  • removed unused dependencies due to the migration and updated to current versions e.g. taquito to v17
  • removed unused files now that were related to CRA and replaced with needed Vite files e.g. Vite config
  • in Vite its import.meta.env.VITE_ instead of process.env.
  • Also I added eslint so it highlights possible wrong patterns. The app still works, so fixing issues that are highlighted could be done as another PR after this - I started with some small changes but stopped as this would be better as another PR, I can revert these changes if thats desired.

What needs to be done:

  • The app needs to be tested with Android studio to see if it also works on Android like before.
  • make changes in the README so it reflects the updated code

Will finish this asap.

You can view, comment on, or merge this pull request online at:

https://github.com/marigold-dev/training-dapp-shifumi/pull/4 Commit Summary

File Changes

(224 files https://github.com/marigold-dev/training-dapp-shifumi/pull/4/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/marigold-dev/training-dapp-shifumi/pull/4, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATLHMGL3MTLT6PK34OGR3DXPVYHDANCNFSM6AAAAAA2GGS2YA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

dvkam commented 1 year ago

This is weird, I never noticed this and I have done this training quite a lot Do you have a complete scenario yo reproduce it?

Interesting, I tested it with Temple in different browsers and on two different machines and with different rpc. The flow is:

When I try with Kukai it works perfectly fine.

zamrokk commented 1 year ago

and with other wallet like airgap or trustwallet ?

dvkam commented 1 year ago

I will test it with other wallets later and let you know

zamrokk commented 1 year ago

hum , With temple, I confirm, nothing happen ...

zamrokk commented 1 year ago

there is actually an issue with Temple 1.16.3 , I will report that to their team

zamrokk commented 1 year ago

I posted it on MadFish Telegram, it can be only the Temple extension, I can't see other parts failing

Thanks, dude !

zamrokk commented 1 year ago

They released a new version but for me it does not fix the issue they have

I need to try to play with Android first before merging the PR

zamrokk commented 1 year ago

@dvkam You can bypass the Temple issue by adding more gas during the confirmation pop-up ;o)

Btw, I cannot compile the android version with Vite

Any idea ?

image

zamrokk commented 1 year ago

ok, the app works on my Android phone =)

zamrokk commented 1 year ago

@dvkam I will have a look at the other trainings now for Vite