Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Document use of contract_templates #699

Closed
wants to merge 3 commits into from
Closed

Conversation

feuGeneA
Copy link
Contributor

@feuGeneA feuGeneA commented Jun 14, 2018

Description

Explain what npm packages must be installed, and what TypeScript configuration changes must be made, in order to use the contract wrappers generated by using abi-gen with contract_templates.

Also, I made small a change to one of the templates, to work around a TypeScript compiler bug, which I had encountered in my usage. See details in my commit message.

Motivation and Context

I tried to start from a freshly generated truffle init project, and apply abi-gen to the stock contract, and then use tsc to compile the generated wrapper. It was not a straightforward process, so I wanted to provide help to anyone else who might want to do that.

How Has This Been Tested?

I manually verified all of the instructions I wrote, and also the template change, several times over, and even created a repo to show that verification (via the commit log), at https://github.com/feuGeneA/truffle-init-to-0xproject-abi-gen .

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Change requires a change to the documentation.
  • Added tests to cover my changes.
  • Added new entries to the relevant CHANGELOG.jsons.
  • Labeled this PR with the 'WIP' label if it is a work in progress.
  • Labeled this PR with the labels corresponding to the changed package.

feuGeneA added 2 commits June 13, 2018 23:09
before this change, TypeScript compilation of the generated contract
wrapper was giving me the following errors:

$ abi-gen --abis 'build/contracts/*.json' --out build/types --template contract_templates/contract.handlebars --partials 'contract_templates/partials/*.handlebars'
Found 7 partial templates
Found 1 ABI files
Processing: Migrations...
Created: build/types/migrations.ts
$ tsc
build/types/migrations.ts(81,23): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
build/types/migrations.ts(108,23): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
build/types/migrations.ts(130,23): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
build/types/migrations.ts(146,25): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
build/types/migrations.ts(173,25): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
build/types/migrations.ts(195,25): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Here is the generated code around the first error:

74:    public setCompleted = {
75:        async sendTransactionAsync(
76:            completed: BigNumber,
77:            txData: Partial<TxData> = {},
78:        ): Promise<string> {
79:            const self = this as any as MigrationsContract;
80:            const inputAbi = self._lookupAbi('setCompleted(uint256)').inputs;
81:            [completed,
82:    ] = BaseContract._formatABIDataItemList(inputAbi, [completed,
83:    ], BaseContract._bigNumberToString.bind(self));

All of the other errors are the same, a destructuring assignment with a
single element but with a trailing comma.

This is legal JavaScript but it is not allowed by the TypeScript
compiler, apparently per the bug described at
microsoft/TypeScript#24628 .

While awaiting the 3.0 version of TypeScript, it's a simple enough
change to have the template not append a trailing comma.
It must have been left over from when an abi-gen output was modified to
be the source of this template.
Copy link
Contributor

@LogvinovLeon LogvinovLeon left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. LGTM except for the removal of the comment.

@@ -1,7 +1,3 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you decide to remove that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because this file (with template code) was not generated using abi-gen, and you should indeed edit it directly, per https://github.com/0xProject/0x-monorepo/tree/v2-prototype/packages/abi-gen#how-to-write-custom-templates . I assumed that this contract.handlebars file was created by taking a wrapper (that was auto-generated using abi-gen) and editing it to add the template stuff, and that this comment was leftover from that process.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, abi-gen will prepend it itself. Either do the change in abi-gen or leave it there for now and I'll change it later

Copy link
Contributor Author

@feuGeneA feuGeneA Jun 14, 2018

Choose a reason for hiding this comment

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

I do think it's right for abi-gen to prepend itself to the generated file. But once you take the generated file and give it a life of its own by committing it with changes (here, the template additions), it's no longer a "generated file", and it makes sense to remove the comment from that file. Humbly but strongly, my opinion is that leaving abi-gen as is, and removing this comment, are both the right things to do. But I'm happy to defer to your opinion: if you disagree, then follow up one more time and tell me to remove it leave the comment, and it shall be done. :)

@LogvinovLeon
Copy link
Contributor

Please rebase it off the v2-prototype branch as all the work is happening there and it's gonna be merged into master soon.

@feuGeneA
Copy link
Contributor Author

Can't change the source branch of a PR. Opened new PR based on v2-prototype (#701). Closing this one.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants