-
Notifications
You must be signed in to change notification settings - Fork 366
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
Support for web3 1.0.0 #25
Comments
I have created #26 for this |
Hey, thank you very much for working on this issue. When I started developing TypeChain I also wanted to target 1.0 release (promises ftw) but it turned out to be buggy and we switched to I agree that support for 1.0 is great but I would prefer either to introduce something like |
I'd recommend you maintain separate branches and pin the version to the same version as web3. I would also tag the 1.0 branch as beta (https://docs.npmjs.com/getting-started/using-tags) while web3 is in beta. Coincidentally this is what they should have done, but have not. Let me know if you don't want this and I will withdraw the PR and maintain a separate fork |
I would suggest following:
What do you think? As a side note: I thought about changing the way we generate implementation and during build time just create interfaces, implementation would be generated in runtime. This would allow to easily support multiple targets because the interface that TypeChain contracts provide wouldn't change — only implementation part. |
Yeah sounds like a good plan, I will see if I can tidy it up tomorrow. What do you think we should do with the BigNumbers? Return them as string or add some code to cast them to BigNumber? |
@linusnorton did you find anything about the reasoning for removing BigNumber? As I mentioned before, I would prefer that both targets share the very same API. So we will need to manually wrap strings in BigNumber instances. |
Unfortunately I only found out by trial and error. There is a mention of it here: It seems like it is so that people can choose to use BN or BigNumber.js |
That's so typical for web3js :D Yeah, I am still up for supporting BigNumber anyway. That at least should be straightforward. |
Great, I will try to take a look at revising the PR tomorrow evening. |
@krzkaczor the PR is now split into two parts. If you could make a next branch then I can re-target the web3 1.0 PR. I decided to switch BigNumbers to strings in order to reflect the changes made to web3 1.0. |
@linusnorton i have created the
I believe it won't work for a long term as we want to keep TypeChain interface stable between different web3 versions etc. For now, it's fine since it's the very beginning of work on support for web3 1.0 When it's merged I am gonna release it to npm with appropriate version/tag. I will let you know. First, Let's merge pr to master. Then I will update next branch and then we will merge the other pr. Thanks for your time! |
Okay, when you are happy with the solcjs branch and it gets merged, I'll update this PR and we can merge it too. |
@linusnorton thanks for the first pr, good work! I fixed docs, and updated travis file in separate PR (already merged). Also, |
Is there an expected release date for compatibility with web3 1.0? |
@pelsasser please check out I plan to rewrite TypeChain in a more modular way in a couple of next days. This will allow people to provide they own templates and generate typings for web3 1.0 or ethers.js or whatever is needed. |
awesome, that is great to know. Any estimated release date? |
@krzkaczor - thanks! sorry to bug you on vacation! If we can assist, please let us know. We are enjoying using typechain! |
@pelsasser no worries ;) Glad to hear that you like it! There will be some breaking changes when Btw. after the migration is done you would configure Typechain (and any others plugins) like this: https://github.com/krzkaczor/ts-gen/blob/master/example/ts-gen.json (and btw |
Hi @krzkaczor this issue has come up as a Gitcoin Request, which means @StevenJNPearce would like to work on the issue for $200. Would you mind if we fund it from our budget? |
@vs77bb I was acutally hoping to get this funded and have someone else pick it up, or @krzkaczor could take the funding, as personally I'm not entirely clear on the work that needs to be done to implement it. |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done This issue now has a funding of 200.0 DAI (200.0 USD @ $1.0/DAI) attached to it.
|
Thanks for clarifying @StevenJNPearce... my mistake, thanks for simply bringing this to our attention. Great candidate for a Gitcoin bounty. @krzkaczor Just funded this one! If you'd like a contributor to pick it up, I hope some Gitcoiners will be on the way soon. However, you could also 'Start Work' yourself and claim the bounty once this PR is merged. If you'd furthermore like to discuss funding options for TypeChain via Gitcoin, happy to discuss on Slack! Hope you're doing well 🙂 |
Hey, thanks! I will describe the details of what needs to be done when I work on my computer. |
Hello @krzkaczor I'm interested in this, would you mind telling the details to me ? (for what has to be done) |
Hi, guys.
It means that type of this issue is contest. I mean for somebody (for example: for me) this can be important. |
First of all, thanks for funding this issue @StevenJNPearce. As for what needs to be done:
I will mark this issue as something I work on till |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done Work has been started. These users each claimed they can complete the work by 11 months, 4 weeks from now.
Learn more on the Gitcoin Issue Details page. |
@krzkaczor Sounds great! @Naggertooth This is indeed a traditional bounty - one person works, one gets paid out. We just knew we would approve @krzkaczor to work on it first if he'd like, because he is the maintainer of the repo. Be on the lookout for more issues + info on the thread here as things progress 🙂 |
@krzkaczor Awesome, thanks for the update! CC @StevenJNPearce for his reference as well 👍 |
It's done and released as As I mentioned before, if you used typechain before |
WOOT! Nice work! cc: @MARKETProtocol/core |
@pelsasser great to hear that! Please, let me know if you encounter any problems using it. I am gonna submit changes to |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done Work for 200.0 DAI (200.0 USD @ $1.0/DAI) has been submitted by: @vs77bb please take a look at the submitted work:
|
Issue Status: 1. Open 2. Started 3. Submitted 4. Done The funding of 200.0 DAI (200.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @krzkaczor.
|
@krzkaczor Great work! This was a fun use of Gitcoin for us, please do feel free to use Gitcoin Requests in the future if something comes up on your repo and you could use the help externally 🙂 |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done The funding of 200.0 DAI (200.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @krzkaczor.
|
@vs77bb wow, thanks! It's the very first time that I used gitcoin and I am surprised how pleasant process it was 👍 |
Although web3 1.0.0 is still in beta it is now the default install candidate for npm so trying to use the default web3 with typechain doesn't work.
The specific problem seems to stem from a backwards incompatible API change they've made:
The text was updated successfully, but these errors were encountered: