cmditch / elm-ethereum

dApps in Elm
https://package.elm-lang.org/packages/cmditch/elm-ethereum/1.0.1/
BSD 3-Clause "New" or "Revised" License
146 stars 21 forks source link

Sync as Result #26

Closed cmditch closed 7 years ago

cmditch commented 7 years ago

Yummy deletions. Added Native.Web3.toResult for synchronous functions. Cleaned up toTask. No more Sync/Async conditional. Removed Sync/Async from CallType and and Request types. Cleaned up all functions accordingly.

NOTE : Right now Web3.toResult returns a Result Web3.Error a, forcing the user to pattern match the Result as well as the Web3.Error type. I did this so the types would match up when using Result.andThen.

It's still not clear whether we'll need Web3.Error or if we should just return a stringy error on toTask and toResult. The idea was to mimic Elm's HTTP library. The problem is inferring the error case within Web3.js and responding accordingly. More on this later.

UPDATE This was implemented, tested, merged to master, tested again and realized it was broke, and consequently reverted. Didn't realize the existence of a revert PR in github, so I did a hard reset and force pushed master. 👎 Sorry about that

Turns out toResult causes a major bug where some functions relying on Web3.toResult were being executed during the loading of the Elm runtime.

Namely, any constant within one's elm code relying on a Web3.toResult would result in an Err: Web3 not defined.

All the more reason to stick with Tasks.

The UX/DX issue that might have been solved with toResult can be mitigated by using Task.andThen. This is something I considered a while back, but slipped my mind... oof.