switch to solcjs, fix formatting unit test, update chalk dependency #28
Conversation
| import { MalformedAbiError } from "./errors"; | ||
|
|
||
| const { yellow } = chalk; |
There was a problem hiding this comment.
please revert. Why do you prefer object destruction instead doing this on one go while importing?
There was a problem hiding this comment.
Unfortunately that format is no longer supported, see chalk/chalk#215 (comment)
There was a problem hiding this comment.
Uhh, that's horrifying but I guess we need to roll with it 👍
There was a problem hiding this comment.
Yeah, sorry about that. The maintainer of chalk seems quite adamant that it is the best way to do it :/
| @@ -1,6 +1,6 @@ | |||
| #!/usr/bin/env node | |||
| import { readFileSync, writeFileSync } from "fs"; | |||
| import { blue, red, green } from "chalk"; | |||
| import chalk from "chalk"; | |||
| @@ -11,6 +11,7 @@ import { parseArgs } from "./parseArgs"; | |||
| import { copyRuntime } from "./copyRuntime"; | |||
| import { extractAbi } from "./abiParser"; | |||
|
|
|||
| const { blue, red, green } = chalk; | |||
| @@ -31,12 +31,14 @@ | |||
| "web3-typescript-typings": "^0.7.2" | |||
| }, | |||
| "dependencies": { | |||
| "chalk": "^2.1.0", | |||
| "bignumber.js": "^5.0.0", | |||
There was a problem hiding this comment.
bignumber should be peer dependency
There was a problem hiding this comment.
I think it should be both as the project needs it and downstream projects need. Is that how peerDependencies work? You've got web3 as both a dependency and a peerDependency so I guess so.
There was a problem hiding this comment.
I will revisit this in a separate PR. But I think web3, bignumber etc should be only peer dependency since we generate code directly to users codebase (thus not using our dependency but user project's dependencies)
| @@ -2,7 +2,7 @@ import { expect } from "chai"; | |||
| import { deployContract } from "./utils/web3Contracts"; | |||
|
|
|||
| import { web3 } from "./web3"; | |||
| import { ContractWithOverloads } from "./abis/ContractWithOverloads"; | |||
| import { __ContractWithOverloads_sol_ContractWithOverloads as ContractWithOverloads } from "./abis/__ContractWithOverloads_sol_ContractWithOverloads"; | |||
There was a problem hiding this comment.
isn't there a way to make solcjs generate more readable file names?
There was a problem hiding this comment.
It pollutes a lot of code otherwise
There was a problem hiding this comment.
I didn't find one. I cd into that directory to stop it generating even more noise. I will dig a bit more and get back to you.
|
@krzkaczor I think this is as ready as it's ever going to be. I didn't find a way to tidy up the contract filenames (apart from manually renaming them). |
|
@linusnorton thanks! Sorry, i was busy. I will make a review of this and the other branch before monday! |
| @@ -31,12 +31,14 @@ | |||
| "web3-typescript-typings": "^0.7.2" | |||
| }, | |||
| "dependencies": { | |||
| "chalk": "^2.1.0", | |||
| "bignumber.js": "^5.0.0", | |||
There was a problem hiding this comment.
I will revisit this in a separate PR. But I think web3, bignumber etc should be only peer dependency since we generate code directly to users codebase (thus not using our dependency but user project's dependencies)
Unfortunately I also had to update chalk to get it to build correctly.