-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Remove typescript typings #1658
Comments
I created a first draft of typings for @types/web3. See DefinitelyTyped/DefinitelyTyped#26031 |
@levino -- I've seen that you've closed the PR at |
I just opened the PR on Friday. It is WIP, so I closed it until I am done. My idea is to have the typings only at definitelytyped and no mention of typescript in this repo over here. I am happy to get any further support on the matter. Maybe one can use the web3 repo here to gather issues for the typings over at definitelytyped. I will continue work on Monday and formulate some todos. The typings from the PR pass linting and work for me to compile my project which uses web3 1.0. please give it a spin too and see what is missing for you. |
@levino -- Got it. Probably it's better to get a sign-off from the repo owners regarding the plan of action here. I've added a comment over another PR asking for their thoughts on the matter. I'd be more than happy to help you with the porting of code from PS: I'm just getting started with |
There is a risk that this approach won't be approved, even though it follows the categorical imperative and as such is the correct approach ;). Even if the decision is taken to keep the typings here, the PR at dt will still be very valuable. Even more so if we use it to create very well readable and high quality typings. They can easily be ported back. I also just took the typings from here and fixed the linting errors they produced in a stricter setup like dt when I created the PR in the first place. I also "stole" the typings for |
@levino -- Understood. Let's try to get it approved though ;) Let me know how can I be of help here and also we can try asking around the folks at |
@eswarasai I added more work to it. Please review the PR at DT. DefinitelyTyped/DefinitelyTyped#26031 |
@levino -- Great work 👍This is looking amazing. Thanks for all the work. Hopefully we can get this merged soon :) |
I added a "Todos" section in the PR. If you have time, please feel free to address any one of them. For example if would be great if you could add tests. |
Mentioning @frozeman to get some feedback on the matter. |
Also I kinda got lost on the way and based my work on an outdated type definitions file. I will have to fix this tomorrow (and add a bunch of contributors). |
My changes to DefinitelyTyped have been merged. Now people can install and use https://www.npmjs.com/package/@types/web3. It wont work just like that at the moment as the broken type defintions still are part of the How to use
|
@levino When following these instructions on a React app boostrapped with latest @petejkim/react-scripts-ts-sass (latest npm version, 2.15.2), I get
|
Are you sure you use the exact code above for requiring the module? Copy and paste it. |
Can you create a minimal example repo to reproduce your issue? |
@levino Here's the repro, commit messages indicating steps taken: https://github.com/freeatnet/web3-1.0.0-beta.34-types-issue-repro. I appreciate your quick response and work on web3 types! |
I gave you two solutions to your issue in the repo via PRs. For reference the problem was usage of setting |
@levino Thank you for looking into it! I'm gonna experiment with Related-unrelated: if I feel that I found an issue in |
As an issue in the definitely typed repo. Highlight me if you want in the issue. |
Thanks for your great work @levino! |
@levino Can you please give an example how to use abi module with your @types/web3 package? Now it works for me only like this (of cause without typings): |
My package is for |
@levino I followed your instruction and created a simple application.
Seems 'import' syntax and the typing file is okay. My repo is https://github.com/tomo1026j/SampleWeb3 I'm sorry if this is not the right place to ask but appreciate if you can help me. |
I've understood finally how to work with
|
@levino There's another small issue with Have created a PR |
…escript specific things in the codebase.
…escript specific things in the codebase.
…escript specific things in the codebase.
* Fix #1658 by removing the typings files and all references to typescript specific things in the codebase. * Add documentation usage with TypeScript.
- Import it correctly as a CommonJS/AMD module in TypeScript: web3/web3.js#1658 (comment) - Delete the default types on postinstall
@levino How using |
@krzkaczor that is a syntax supported by TypeScript
https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require That said, I'm not sure that it's necessary... |
@zachlysobey I think its somehow deprecated, for example its not supported by babel: babel/babel#7062 This is why I really avoid it. |
We are not talking about Babel but typescript. You can use "synthetic default exports" and it will work in all kinds of ways. But the correct way to use this package in a typescript project is as described. Anyhow this is off topic. If you still feel some pain from this, please create a new issue with proper description. |
@levino thank you for your hard work on these typings & your explanations of how to get them working. Does anyone know if there are any plans to ship these working typings as a part of the web3 package? |
There are no plans because that would be an anti pattern. Please start reading from the top, including all links. |
How come there is a I feel like it's a regression as it creates issues similar to what was mentioned in this thread. |
Yeah. It was decided to do so. Type defs are now maintained here again |
No. Type definitions are now shipped with web3 |
@levino Maybe I'm missing something, but are types still being shipped with web3 as of 1.2.0? |
@levino might I be doing something wrong if types are being shipped?
|
Type defs are not shipped with web3@^1 at the moment. I think that @types/web3 work with web3@^1. web3@^2 is still beta, I think but it has type defs for TS. Currently @NVIDIA does not allow us to officially provide type defintions via DT and also does not provide type defintions himself. |
@levino I ended up using import Web3 from 'web3'
import { BlockHeader } from 'web3-eth' Some of the APIs (I know it's alpha) are still confusing though, and always have been with web3 I think, here's an example: function subNewBlockHeaders () {
web3ws.eth.subscribe('newBlockHeaders', null, (err: Error, res: BlockHeader) => {
if (err) {
console.log('error', err)
}
console.log('new block', res.number)
}).on('data', (data: BlockHeader) => {
console.log('this will never be reached, either directly with .on or by assignment from eth.subscribe')
})
}
// SUBSCRIBE
subNewBlockHeaders() Yet the docs in all versions past 1 (I believe) state you have to use |
@levino Could you please stop trying to restart the types discussion all the time. The 2.x branch does have the types for each module and there are way better then the types you have „maintained“ on DT. We will back-port all the bug fixes we already have done into the 1.x branch and the type definitions with the dtslint tests will get copied to the 1.x branch as well. (see issue #3070) |
* Fix web3#1658 by removing the typings files and all references to typescript specific things in the codebase. * Add documentation usage with TypeScript.
The files with types are non working and sloppy (lots of implicit any). Please remove them, they cause more harm than good.
The text was updated successfully, but these errors were encountered: