Skip to content
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

Implement static type interface for Contract #4604

Merged
merged 18 commits into from
Dec 17, 2021
Merged

Conversation

nazarhussain
Copy link
Contributor

Description

Add contract static type support.

Fixes #4562

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • I ran npm run dtslint with success and extended the tests and types if necessary.
  • I ran npm run test:unit with success.
  • I ran npm run test:cov and my test cases cover all the lines and branches of the added code.
  • I ran npm run build and tested dist/web3.min.js in a browser.
  • I have tested my code on the live network.
  • I have checked the Deploy Preview and it looks correct.
  • I have updated the CHANGELOG.md file in the root folder.

@nazarhussain nazarhussain self-assigned this Dec 6, 2021
@nazarhussain nazarhussain linked an issue Dec 6, 2021 that may be closed by this pull request
@nazarhussain nazarhussain force-pushed the nh/4562-contract-typing branch from 8877b33 to 98c7bc4 Compare December 6, 2021 20:42
@@ -0,0 +1,7 @@
// TODO: Convert it to static file
type _SolidityIndexRange = 1 | 2 | 3 | 4 | 5;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the fixed array size is passed through the ABI as string so we need relevant map for integer values. We can set this up to maximum we want to support.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But isn't the range X >= 0? So it's some very large number that we'd have to support up to?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theoretically ECMA-262 supports 2^32 -1 which is very less than what solidity supports. We can add as much number we want, but the larger numbers here will make the intelisence of TS very slow. So we have to restrict this number to some limited values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the limitation reference in the README file.

@jdevcs
Copy link
Contributor

jdevcs commented Dec 10, 2021

Please test type checking support for famous community contracts:
Open Zeppelin :

  • ERC20,
  • ERC777,
  • ERC721, and

Gnosis :

  • Safe / multisig

@nazarhussain
Copy link
Contributor Author

@jdevcs I had added type tests for ERC20 and ERC721. I believe these are good enough to test the type interface for contract. Gnosis safe/multisig is splited among different contracts, if you think think that's necessary to add please share reference to its ABI to use.

@nazarhussain nazarhussain added the 4.x 4.0 related label Dec 15, 2021
@jdevcs
Copy link
Contributor

jdevcs commented Dec 15, 2021

@jdevcs I had added type tests for ERC20 and ERC721. I believe these are good enough to test the type interface for contract. Gnosis safe/multisig is splited among different contracts, if you think think that's necessary to add please share reference to its ABI to use.

For now just include ERC777 along with , including multisig in this PR will increase scope/time , add that in tests TODO items.

@nazarhussain
Copy link
Contributor Author

@jdevcs Created an issue for additional type tests #4631

Copy link
Contributor

@spacesailor24 spacesailor24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still don't fully understand all the TypeScript going on, but the tests seem to show it works as needed. I think we should find some more complex contracts to throw at this before releasing 4.x

@nazarhussain
Copy link
Contributor Author

nazarhussain commented Dec 16, 2021

@spacesailor24 Already created an issue to add more complex contract types tests. #4631

@nazarhussain nazarhussain merged commit 3f72f52 into 4.x Dec 17, 2021
@nazarhussain nazarhussain deleted the nh/4562-contract-typing branch December 17, 2021 12:24
? Address
: never;

export type PrimitiveStringType<Type extends string> = Type extends `string${string}[${infer Size}]`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to have extensive JSDocs here explaining the rationale of this types. For external contributors this is hard to read and comments would help a lot

Copy link

@dapplion dapplion Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there benchmarks of the compilation cost of extensive use of Template Literal Types? Docs say:

We generally recommend that people use ahead-of-time generation for large string unions, but this is useful in smaller cases. ref

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never found any such, but these types provides real-time developer feedback in IDE so fast enough to use.

Copy link

@dapplion dapplion Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any demo project I can try with this feature? Would love to see some usage guide too

@dapplion
Copy link

To better understand this PR, please correct me if I'm wrong:

  • The goal of this PR is to strongly type consumers of contracts, for example DApps and test suites
  • This is an alternative to Typechain but instead of doing code generation it derives the types from the JSON ABI directly on each run

@dapplion
Copy link

Very impressive use of Template Literal Types 👍 technically this is really cool

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x 4.0 related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement static type interface for Contract
5 participants