dethcrypto / TypeChain

🔌 TypeScript bindings for Ethereum smart contracts
MIT License
2.76k stars 365 forks source link

Add struct parsing to @typechain/web3-v1 (#779) #801

Open krzkaczor opened 1 year ago

changeset-bot[bot] commented 1 year ago

🦋 Changeset detected

Latest commit: a2e307c6e4460e43062bb1b0d705cdf2347619c1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ------------------ | ----- | | @typechain/web3-v1 | Minor |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

krzkaczor commented 1 year ago

@mihai9-lab there are still some problems with tests. Can u take a look?

sigmachirality commented 1 year ago

@mihai9-lab when do you think you'll have bandwidth to work on this change? Want to get this through pretty urgently, can I work on this now if you don't have time soon?

mihai9-lab commented 1 year ago

@mihai9-lab when do you think you'll have bandwidth to work on this change? Want to get this through pretty urgently, can I work on this now if you don't have time soon?

Hey, sorry for not replying sooner. Currently I'm pretty busy and won't be able to work on this for at least a week or two. I have no issues with you working on this, so feel free to do so

sigmachirality commented 1 year ago

The approach of this PR might be a bit flawed: as seen here: https://web3js.readthedocs.io/en/v1.8.1/web3-eth-contract.html#methods-mymethod-call

"Promise returns Mixed: The return value(s) of the smart contract method. If it returns a single value, it’s returned as is. If it has multiple return values they are returned as an object with properties and indices"

Since the return is "an object with properties and indices", we cannot return the type of a struct as a union of an array and an object, as this will annotate the object with all the members of an array (for example, .concat), which do not actually exist on the object.

mihai9-lab commented 1 year ago

Yeah i didn't expect it to behave like that, it seems that typescript does assume object has all properties of an array(although it isn't one).

I see 2 possible approaches to deal with this - one being to change array generation to object generation, other being to leave it as is but add an additional "converter" type which would exclude unwanted array properties.

I made a quick array to objected converter type as a proof of concept if going with second option, here's a playground for that. Maybe this is a bit hacky, so i'd like to see what you and the community thinks is the best approach.