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

Upgrade to elm 0.19 #63

Closed
VledicFranco opened this issue Aug 23, 2018 · 15 comments
Closed

Upgrade to elm 0.19 #63

VledicFranco opened this issue Aug 23, 2018 · 15 comments

Comments

@VledicFranco
Copy link

I tried with elm-upgrade but there are still some dependencies which are still not migrated or there are no alternatives. Maybe we can help them make a new version with 0.19, it is very straight forward the migration.

@VledicFranco
Copy link
Author

https://github.com/Chadtech/elm-bool-extra looks somewhat abandoned, would you prefer to manually import the code or make a fork?

@surprisetalk
Copy link

It looks like rtfeldman/hex was renamed to rtfeldman/elm-hex.

I think maybe-extra, result-extra, and elm-bool-extra should be pretty easy to get around. It may be worth reimplementing the functions that you need to reduce the number of dependencies.

Some of the other packages (particularly websocket) will be a pain. Let me know how I can help!

@cmditch
Copy link
Owner

cmditch commented Aug 23, 2018

https://github.com/elm-lang/websocket is our major deadlocker for getting elm-ethereum up to 0.19.

There's no clear ETA from Evan or the core team when an 0.19 websockets implementation will be done. The old WS lib has a lot of known bugs, and outstanding PR's/issues.

I'm working on PR's for things like elm-bigint, elm-keccak, and some of the ...-extra libs.

I also agree, some of the dependencies being used in the ...-extra libs are very small. Might be worth just adding them to Internal.Utils. If it only requires adding 1-3 functions in utils to drop dependencies on these kinds of libs, I'm open to a PR which takes care of this.

@FrancoAra @surprisetalk @francescortiz any interest in taking a look at which ...-extra lib dependencies can be dropped in this manner?

Thanks!

@VledicFranco
Copy link
Author

https://github.com/Chadtech/elm-bool-extra has been updated woot!

@cmditch
Copy link
Owner

cmditch commented Aug 27, 2018

Evan has spoken up regarding websockets. It might be a while :\

elm-lang/websocket#28 (comment)

and also Richard commenting on it here

gdotdesign/elm-github-install#62 (comment)

@svechinsky
Copy link

Not sure if I understand it all correctly since I started learning elm about a week ago.
However, maybe it will be possible to use https://github.com/slashmili/phoenix-socket instead?
If I understand correctly it uses long polling as a substitute to websockets.

@VledicFranco
Copy link
Author

I also found this one :P https://github.com/billstclair/elm-websocket-client

@cmditch
Copy link
Owner

cmditch commented Sep 4, 2018

@svechinsky Doesn't look like that package will help in our case.

@FrancoAra Interesting find!

My take on the situation is currently - it probably just makes sense to just wait for the 0.19 version of websockets to come out. (see below on alternative proposal)

The slight decline in UX that elm-websocket-client would introduce is not worth the complexity of making a big refactor, and then making yet another refactor once the 0.19 websockets package is released. The code in elm-ethereum utilizing websockets is pretty hairy (Eth.Sentry.Event module).

I think a nice middle road might be, release an elm-ethereum v2.0 which removes the Eth.Sentry.Event module for the time being. This provides some nice tradeoffs which I'll elaborate using some elm code ;P

case (desperatelyWantsToUseElm019, requiresContractEventListening) of
    (True, False) ->
        "Upgrade to elm-etheruem 2.0!"

    (True, True) ->
        "Use ports to bake your own custom contract event listening module, or
          contribute to the package by writing an pure http event/log listener."
       
    _ ->
        "Keep using Elm 0.18 and elm-ethereum 1.0 until new websockets pacakge is released, 
          or upgrade whenever you see fit."

Edit - Another option is to work with events like the old web3.js used to, using pure http calls to the RPC functions like getFilterChanges etc. I personally don't use events enough at the moment (in our production app at least) to justify the time required to write out this module, so I'd leave it to others to make a PR. I'd be happy to give guidance where it makes sense.

The 0.19 compatible elm-ethereum 2.0 (sans events) is starting to sound more and more like a good tradeoff. Curious to hear if anyone has thoughts on the matter.

Thanks

@francescortiz
Copy link
Contributor

In my case I am a (False, False). Since events are very important in my projects I am still using elm 0.18 while the elm/websockets package gets released.

@francescortiz
Copy link
Contributor

In the end I needed to upgrade to elm 0.19 because I needed to reattach filters on WebSocket connection lost. My approach has been to use elm-websocket-client. Since it is using this library it gives full control over websocket handling. Unluckily, the library uses its own port multiplexing mechanism called PortFunnel, so there is some boilerplate code that you need to add to your project.

For information on how to use use elm-websocket-client port multiplexing you can check the documentation:
https://package.elm-lang.org/packages/billstclair/elm-websocket-client/latest/

The changes I made are in the elm-0.19 of my fork:
https://github.com/francescortiz/elm-ethereum/tree/elm-0.19

Elm 0.19 requires you to remove all Debug calls in order to have release builds. I didn't do it yet because I didn't analyse what is the strategy to do it and don't loose important information.

It is far from perfect but, if it can help anyone, I'll be glad.

@cmditch
Copy link
Owner

cmditch commented Oct 25, 2018

@francescortiz Nice, looking over the code now.

First thing I noticed, there's a BigInt for 0.19 that you should be able to use.

@cmditch
Copy link
Owner

cmditch commented Oct 25, 2018

I think the pattern you're going with would be a good direction for an initial elm-ethereum on 0.19. Thanks for exploring this!

Much like walletSentry, and txSentry - eventSentry can also link up with elm-ethereum-ports for the time being. Might also be possible to pass your WS port into the init as opposed to every function.

I'm heading to Devcon this week, so I'll try to work on this in my free time, but will be pretty occupied the next 2 weeks. Hopefully you're fork is suiting you well for the time being. I'll reference some of the other changes you've made as well (like getEvents, and the posix addition).

Is there anything else substantial you've changed that I should take note of?

Also, can you elaborate on why Eth.gasPrice had to become Eth. eth_gasPrice. I know gasPrice is a record field in Eth.Types. Was the 0.19 compiler complaining about this?

With the Debug stuff, we might also be able to make the API require the user to provide Debug.log for Sentry.Tx and Sentry.Event

Sentry.Tx.init : ( OutPort, InPort ) -> (Msg -> msg) -> HttpProvider -> TxSentry msg

Sentry.Tx.initWithDebug : (String -> a -> a) -> ( OutPort, InPort ) -> (Msg -> msg) -> HttpProvider -> TxSentry msg

and

Sentry.Tx.init : (Msg -> msg) -> String -> EventSentry msg

Sentry.Tx.initWithDebug : (String -> a -> a) -> (Msg -> msg) -> String -> EventSentry msg

@francescortiz
Copy link
Contributor

You covered well all the changes I made. I add the following:

  • I tried switching to cmditch/elm-bigint but I had decoding problems. The values that I tried to decode where hex strings passed to the uint decoder. I think the problem was that it was expenting string representations of ints, but I am not sure.
  • Also, there was a complaint about gasPrice, but I managed to refactor the code in order to avoid the need of renaming the function.

I really like the approach you propose for the debug stuff! I just want to make sure you don't miss that my code removed the Debug.log call from nodeResponseToMsg.

Good luck in the Devcon!

@cmditch
Copy link
Owner

cmditch commented Feb 20, 2019

Removed EventSentry for now, and just upgraded to 0.19.

Now finally getting back into development efforts, lot of cleanup to do, and some rethinking of how events are handled sans websockets.

@cmditch cmditch closed this as completed Feb 20, 2019
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

No branches or pull requests

6 participants
@VledicFranco @francescortiz @surprisetalk @cmditch @svechinsky and others