-
Notifications
You must be signed in to change notification settings - Fork 1
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
Remove method overloads #61
Comments
cc @asutula idk if you've ran into any issues here yet wrt the Studio. some methods (like querying objects) have a handful of overloads. when I built out the SDK, I manually removed some ABI methods so that only one method was included in the ABI (i.e., the method that had "all" of the params). this is because I statically defined the types (like this), and viem doesn't handle overloads well, so I ran into type issues with the TS compiler. just a heads up in case you run into the same problem. |
I did run into similar issues, but basically just have to be very careful and explicit about how I construct args and pass them to the function calls. After struggling a while with it, I got the hang of it. It seems like the overloads are good to have just for gas savings. But if that's not so important, I'm also fine with removing them. The thing that was more confusing to me is that it seemed like in some places it was a bit arbitrary which overloads existed and which didn't. You mentioned maybe the work on the facades would clean up this situation... Is that true? Should we wait and see? |
yeahhh, I stopped creating them as time went on due to the viem issues and the complexity of params (too many different ways to overload). it seemed like it'd be easier to just handle overlaods at the client layer (if you're aware of the "C" like behavior wrt Rust
um, I think it'll just make it easier to understand by way of better documented behavior. i guess that doesn't really depend on the facades though....i could improve the docs of the existing codebase to make it less opaque. however, i'd imagine we wont use any overloads with the facades. it's not that we cant include overloads there, but it'll probably be easier to have a 1:1 facade method mapped to each rust method. |
This isn't necessarily essential, but viem tends to struggle with heavily overloaded methods and type inference. We can either:
Overloads were created to help facilitate a better DX for Solidity-first developers. However, the only user for the foreseeable future is internal client tooling (JS SDK, Studio, etc.).
The text was updated successfully, but these errors were encountered: