CosmWasm / ts-codegen

Convert your CosmWasm smart contracts into dev-friendly TypeScript classes so you can focus on shipping code.
https://cosmology.zone/products/ts-codegen
Apache License 2.0
116 stars 29 forks source link

bug: ts-codegen fails to generate correct bindings #91

Open abhimanyusinghgaur opened 1 year ago

abhimanyusinghgaur commented 1 year ago

What is happening

Cw3 Fixed Multisig has Vote as both an ExecuteMsg and a QueryMsg:

When you generate the TS bindings for the Cw3 contract:

cosmwasm-ts-codegen
? [cmd] what do you want to do? generate
? [schema] which directory contains the the Rust contracts? ./schema
? [out] where is the output directory? ./ts
? [name] contract name? Cw3FixedMultisig
? [plugin] which plugins? client, message-composer
? [bundle] enable bundle? No

It ends up generating the bindings where the vote property on Cw3FixedMultisigReadOnlyInterface gets overridden by the vote property on Cw3FixedMultisigInterface with a different syntax.

Expectation

The Cw3 contract is correct. ts-codegen should be able to correctly handle this edge case when the names across Execute and Query msgs might conflict.

pyramation commented 1 year ago

the issue is that there is a duplicate named field, this is a known issue.

There are two solutions

  1. change the name of one of them (I think there are two vote methods, perhaps use vote and getVote)
  2. use the client.execExtendsQuery option here — this will create two separate clients though, so you'll have to manage those a bit more.
abhimanyusinghgaur commented 1 year ago

Thanks. I went ahead with option 1. Will try option 2 as well.

But, overall, as a user, my expectation is that this issue should be handled within the default implementation somehow. So that I don't have to adapt to a custom solution just to resolve this.

pyramation commented 1 year ago

But, overall, as a user, my expectation is that this issue should be handled within the default implementation somehow. So that I don't have to adapt to a custom solution just to resolve this.

completely agree! I think we'd have to default to the option being the default, but many teams prefer to manage less clients. Perhaps we can look at creating scoped objects.