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

Create typescript type defs #102

Conversation

dallasjohnson
Copy link
Contributor

This extends the type generator to allow nested types in actions parameters and tables as they can be derived from the provided abi file for a contract.
The current implementation falls back to a string for non-primitive types in action parameters which opens up a potential source of bugs in created tests with the absense of type checking.

This change populates the tables and actions parameters using not just the provided primitives but also nested structs referenced and used from the abi before finally defaulting to a string if a matching primitive and embedded struct is not found.

@MitchPierias
Copy link
Collaborator

@thekevinbrown Code looks good, seems to be failing allot of pipeline checks in Netlify however. Can you please look into this please?

@thekevinbrown
Copy link
Member

The errors from Netlify are for tests around type generation:

11:19:13 AM: Using TypeScript 3.2.4 from /opt/build/repo/node_modules/typedoc/node_modules/typescript/lib
11:19:18 AM: Error: /opt/build/repo/src/contracts/typeGenerator.test.ts(45)
11:19:18 AM:  Expected 3 arguments, but got 1.
11:19:18 AM: Error: /opt/build/repo/src/contracts/typeGenerator.test.ts(49)
11:19:18 AM:  Expected 3 arguments, but got 1.
11:19:18 AM: Error: /opt/build/repo/src/contracts/typeGenerator.test.ts(56)
11:19:18 AM:  Expected 3 arguments, but got 1.
11:19:18 AM: Error: /opt/build/repo/src/contracts/typeGenerator.test.ts(65)
11:19:18 AM:  Expected 3 arguments, but got 1.
11:19:18 AM: Error: /opt/build/repo/src/contracts/typeGenerator.test.ts(76)
11:19:18 AM:  Expected 3 arguments, but got 1.
11:19:18 AM: Error: /opt/build/repo/src/contracts/typeGenerator.test.ts(86)
11:19:18 AM:  Expected 3 arguments, but got 1.
11:19:18 AM: Error: /opt/build/repo/src/contracts/typeGenerator.test.ts(90)
11:19:18 AM:  Expected 3 arguments, but got 1.

What's interesting is this should be failing Travis if it's a real thing, and if it's not, why is Netlify choking on it? I'll investigate.

@thekevinbrown
Copy link
Member

The problem was that the tests were broken because of the changes. Separately there's a PR to get Travis to actually run our tests (#108), but you can run them locally in the meantime with npm t.

I've pushed a commit to your branch to fix the tests, and while I was at it I used a fancy Javascript feature called destructuring to make the function calls a bit cleaner since those params weren't required everywhere. Hope you don't mind, but if you do feel free to change the function signature back to how you had it.

As next steps:

  • It'd be great if you could add a few basic tests to typeGenerator.test.ts to assert that the new contract specific nested type behaviour works.
  • I'm pretty sure we can come up with a better type than any for contractStructs. It's an object with strings as keys and struct definitions as values, yeah? If I'm right, I'd suggest { [structName: string]: any } as a starting point, but if we can be even more specific on what the values are that'd be great, but since the code isn't actually using the values this is enough to get this merged in.

@dallasjohnson dallasjohnson force-pushed the feature/nested-type-creation-from-abi branch from 43df56f to 5832aca Compare October 9, 2019 22:07
@MitchPierias MitchPierias merged commit 5890ed2 into CoinageCrypto:master Oct 11, 2019
@dallasjohnson dallasjohnson deleted the feature/nested-type-creation-from-abi branch October 12, 2019 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants