-
Notifications
You must be signed in to change notification settings - Fork 116
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
Add Types for Contract methods, storage, views, etc. #1343
Conversation
New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/. Published packages:
|
New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/. Published packages:
|
Codecov Report
@@ Coverage Diff @@
## master #1343 +/- ##
=======================================
Coverage 78.96% 78.96%
=======================================
Files 188 188
Lines 10349 10349
Branches 2554 2554
=======================================
Hits 8172 8172
Misses 2057 2057
Partials 120 120
Continue to review full report at Codecov.
|
A new deploy preview is available on Netlify at https://6bfb6c08--tezostaquito.netlify.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me for the methods and views!
For the storage, should a generic type be added to the originate methods?
const op = await Tezos.contract.originate<storageType>({
code,
storage: {// type used here}
})
I see the type errors, fixing now |
New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/. Published packages:
|
A new deploy preview is available on Netlify at https://dc2b4138--tezostaquito.netlify.app |
@@ -158,7 +158,7 @@ export interface ContractProvider extends StorageProvider { | |||
* | |||
* @param OriginationOperation Originate operation parameter | |||
*/ | |||
originate(contract: OriginateParams): Promise<OriginationOperation>; | |||
originate<TContract extends DefaultContractType = DefaultContractType>(contract: OriginateParams<ContractStorageType<TContract>>): Promise<OriginationOperation>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the same should be added here for origination with batch and wallet:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
batch operations cannot be typed because the method accepts an array of operations:
There is no way to require a specific origination operation type at a specific index of the array (as a typed array). Each item of the array must be any of the possible types.
However, a utility method could be provided for this - see the example-usages.ts
in the related PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see how .withOrigination works --- looking at that now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add wallet originate()
and withOrigination()
types for contract and wallet batch()
Fixing the bug first |
New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/. Published packages:
|
A new deploy preview is available on Netlify at https://40397b70--tezostaquito.netlify.app |
New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/. Published packages:
|
New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/. Published packages:
|
A new deploy preview is available on Netlify at https://21834263--tezostaquito.netlify.app |
New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/. Published packages:
|
New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/. Published packages:
|
A new deploy preview is available on Netlify at https://17dcb175--tezostaquito.netlify.app |
New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/. Published packages:
|
A new deploy preview is available on Netlify at https://abcfa102--tezostaquito.netlify.app |
New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/. Published packages:
|
A new deploy preview is available on Netlify at https://039247ac--tezostaquito.netlify.app |
Verified that the CI failure for https://github.com/ecadlabs/taquito/runs/4987546701?check_suite_focus=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, great work :)
These changes are required to allow for improved type support:
See TezosTaqueria/taqueria#91
Before
Using taquito with the generated contract types:
The at method can be called providing a type with a utility method that can be provided:
This can work the same with a wallet
Alternatively, this can be done with a cast:
The originate contract does not have any way to provide a type, so this requires a cast:
For accessing storage, there is no way to pass the type through the contract class,
so it requires providing the type again:
After
Extending ContractAbstraction with additional generic types:
The at method can be called with the contract type provided:
This can work the same with a wallet
The originate contract now accepts a type:
The contract type now also provides the default storage type:
Thank you for your contribution to Taquito.
Before submitting this PR, please make sure:
Release Note Draft Snippet
If relevant, please write a summary of your change that will be suitable for
inclusion in the Release Notes for the next Taquito release.