-
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
Web3 typescript definition has no default export #1248
Comments
Have you seen #1184 ? |
the type definition reflects the current module implementation line 78
Based on typescript document, it is recommended to adopt module-class, which is why the type is exposed with "=" and not default. We have been going back and forth on this issue as you can see with #1184, which was exactly to undo the default import that undo the "=" earlier... I am sure we don't want to keep repeating this cycle, will the following import syntax work for you?
|
Using "require" keyword, I got this error on compilation (module: ES6 and target: ES5)
|
web3.js is still using es5 module system, so it's best to use the compiler option that supports it... try module: "commonjs" |
Doing I could use the lower version but i noticed that the method for viewing pass events, which is what i want to do, has changed in .27. In version *.26 the passed events can be viewed by invoking Please resolve soon, thanks |
seems like @nicolaslawdune has a PR for this |
here is the bottom line:
If you guys look at the commit history, you can see that we have tried both export styles multiple times because every time you switch to one the other camp complained. Giving the module can't be exported both "=" and "default", we have to choose one... but which one? I think we all agree that the type definition should closely reflect the original package implementation: current web3's js implementation uses "export =", according to typescript official doc, it is recommended to adopt type definition with "export =" (see here). This tips the scale for |
PR1249 made it "export default" again in version 1.0.0-beta.30 . Doesn't work with commonjs anymore. |
Just use ES Modules since it's JavaScript's official module system and stop changing it all the time. It's a breaking change and breaking changes are never welcome. |
What I ended up doing to stop typescript from complaining and still have type definition and autocompletion is
|
@AlexBeauchemin was able to make it work as follows:
|
This should be fixed with the PR #2000 thanks to @joshstevens19 who is writing the typings! |
@nivida Which typings are you talking about? |
@levino He has currently just implemented the typings for web-utils and there is an open PR for web3-bzz maybe you can help him or move the typings from DefinitelyTyped into the web3.js repository. I would like to have them directly in the repository because this way I can guarantee that they are always up to date. The issue above should be fixed because of the refactoring and the typings Josh is writing but I will proof it before I merge the PR into the 1.0 branch. |
I repeatedly said and it is best practice to only have the type definitions at definitely typed. You should never again bring type definitions back into this very code base right here. The only exception for this rule is, if you actually write the code in typescript. |
@levino Hey dude, I have to say I completely disagree. I do agree types say quote At the moment the typing's NEVER get kept in date, someone does a PR in If it was actually in the repo itself you can enforce contribution instruction to make sure the typing keep in date all the time. I seen other repos like BigNumber -(https://github.com/MikeMcl/bignumber.js/blob/master/bignumber.d.ts) do this due to how impossible it was to manage it being in 2 separate repos. At the current time the This solution will mean updating methods in the future and updating it's typing will be super easy for all the maintainers as it's in 1 repo. It will also mean the typing's will never be out of sync due to contribution guidelines but still versioned nicely. I know a lot of people may just use I just trying to make this seamless, bringing easier development for the TypeScript devs that's all. Like i said i can tell you a thousands times where the types reflected are so out of date. This would solve a lot of issues i see raised with typing's and bring it all in with 1 install command. This does no harm to any Let me know your thoughts :) |
I removed the type defintions after a long discussion because they were broken (as in "You were not able to use There are two possible and acceptable ways to proceed:
All other approaches are going to fail and only cause misery. |
One other thought to keep in mind: This library is written in Javascript, because the people contributing to it are not able to write Typescript code. If they were, the library would be written in Typescript. Now you want them to review type definitions for a usage of this library in Typescript projects. You see the problem? |
Thanks for your quick response @levino. I completely understand what you are saying.
I think this would be a step in the correct direction and maybe in the future Not sure how it will cause misery either? If we promote a approach of updating both i think the community would be fine with this. It will actually cause a better developer experience for people who install it. This will add a little bit more work for the contributors but make the whole experience better for everyone downloading it and using it in projects as the types will always be correct.
|
And one more thing: PRs to DT get merged pretty quickly when they are good. Most of them aren't though. |
This is a false assumption. First you code javascript. Then you code typescript. Then you learn how to produce type definitions. Devs who are able to write type defintions are a subset of the devs who have mastered Typescript, not the other way round. This is an opinion of mine and we reached a point were we have two different opinions and I have not further arguments to give. I made my point. |
@levino but we have under 10 regular contributors.. do you not think this would bring more good then bad? Say yes some This could be the first step and maybe in the future web3 is rewritten into Thanks a lot for your view and taking time to discuss! |
I get your argument: These 10 contributors should be able to learn typescript. I disagree with your conclusion: Lets make them learn typescript and then maintain the type definitions for this JS code base. I think the only conclusion can be: Lets make them learn typescript and write everything in typescript. With |
I think some other milestones have to be reached first before this gets done like actually release of Really appreciate your view though! |
And one thing about the argument with BigNumber.js: The pain you experienced was not because the type defs where maintained in another repository, but because bignumber.js is not written in typescript. |
A popular example of a lib that's not written in TS and maintains its own type defs is Redux - it seems to work fine for them. Redux is way simpler than web3.js, but it also has way more contributors and downloads than web3.js. |
That might be. But it is against best practice and I explained in detail why the categorical imperative says it is a bad idea. Only because some people do it does not mean that we have to do it too. |
Who defined the best practice you're talking about? One of the most important libs in the industry does it differently without problems, so I don't really see how it's against "best practice". TypeScript docs don't discourage such approach either, so I think that it's just your opinion and not a sacred truth that we should follow. I've used web3.js with TypeScript and each time I've written my own typings for things I needed, because the official ones were just plain wrong and not helpful at all. Bigger Ethereum projects like 0x also have picked this route (as seen here) to avoid the hassle of working with official web3 typings from DefinitelyTyped. |
We are in a world where Anyway discussion over - see ya! |
@joshstevens19 I'd suggest locking this thread to avoid further flamewars. |
Please read the docs more carefully. I quote (again and again and again):
|
@levino not trying to pick sides here, but you must agree that breaking web3.js for typescript users on a regular basis is not a good thing? Regardless of what the TypeScrypt docs say. For better or worse, a lot of people are using TypeScript that also use web3.js. It sounds to me like the only option here to prevent the regular breakage is to include the ts file in this repo so it's always in sync. If the contributors to this repo don't want to update the types file, let some of the people on this thread help whenever a PR comes in to ensure it gets updated properly before merging, it would probably only take a few minutes of a "TypeScript pros" time to check the types file for each PR. |
You all want to have typings that are up to date with the current interface of the library, right? Guess what: There is only one (just one!) way to achieve this: Implement the library in typescript and generate the typings automatically. Even if you maintain the typings here, they will be out of sync all the time. As I said, the typings in this repo were broken. There are several reasons why this cannot work:
So my point is: You all push for maintaining the typings here but you will not achieve what you want to achieve. The type definitions will be out of sync all the time not matter what until this is rewritten in typescript. |
Unfortunately in the typescript docs they do not explain why you should publish the typings via DT when you write your package in Javascript, but I think I gave enough reasoning. Additionally to that: Why do you think that they recommend that? |
I think I do understand TS it is not that rocket science thing to do types.
Maybe we could test them in our CI too.
This should not happen if I review a PR.
I think you should answer this question :) |
So you will from now on personally review all PRs? That is going to scale well.
Yes. That is quite easy. What is more easy though is to rewrite the whole codebase in typescript.
It will happen. Or you will drive away open source contributors because they create a valid PR which never gets merged because "we are still waiting for the typings".
I did explain my thinking. Three times. I repeat myself. People say that I am not right. I want to here their train of thoughts. |
And another question for you all: What about flow? Should we add flow typings too? Facebook is using flow! |
Yes thats what I'm doing currently and will do as long as I work for this lib.
I think we can test them with for example https://github.com/Microsoft/dtslint
Then I have to write the types by my self and I think that will not happen so often.
No you didn't you were just quoting the TS documenation but you never really explained the thoughts behind it. The TS documentation does not explain it either.
I think I could add for example this: https://github.com/joarwilk/flowgen to the publish process to generate them. Idk how good this is but maybe that could be a solution. I'm not saying that I like one of the solutions more then the other but until now I can't see a reall killer argument that we should have them on DT. I think thats more an ethical thing. 😅 |
I really am trying to find it hard to understand why you are so passionate in not having them in the web3 lib itself. We do not have many contributors as it is and we can make sure its part of the contribution guidelines. I am sure a few types is not going to make people fear or stop contributing at all, does that stop all of 500 contributors to Redux? We can also make sure it's tested in our CLI so its just as good and safe as
As i keep saying Redux do this due to making it a lot nicer for the developers to just install and do it, they also have
We are at a point where
We need to be making this stuff mandatory like Redux do, allowing TypeScript don't explicitly say |
Just do what you want if you do not want to listen to reason. This is going to be a fuckup! At least I told you so. |
oh and https://github.com/moment/moment/blob/develop/moment.d.ts are |
I understand your argumentation. You will from now on review every pull request and your super powers will make sure that every change to the API of the library or any of the sub-packages will be reflected 100% in the typings here in the repo. I say: You will fail! And you say: No I have my superpowers. Which is fine. But I will not waste more time on this. Do it then. It will be painful and hard and you will fail. Lets talk again in a year. |
And btw: If at some point you decide to rewrite the whole codebase in TS, that does not count as a success, but a fail too, right? Because that is what you want to avoid with your approach. |
Ah and I forgot to predict something: You will have hundreds of issues from Windows-using script kiddies who cannot use Typescript right but will blame you for "incorrect typings". They will all go on the backlog and jam it. |
This is already happening @levino , this is what this discussion is all about. Apparently all of us TypeScript users are "script kiddies" and yes, we're complaining about this library breaking every time you update it. If you want us "script kiddies" to stop complaining, then just do the right thing and fix the problem. You have a bunch of people saying they'll even help fix the problem, which you don't seem to want to accept. 🤦♂️ |
I maintain (with others of course) the And I repeat: If you all want that this library supports typescript itself, then JUST WRITE IT IN TYPESCRIPT! |
Well, i've been having this page opened for the whole afternoon and i must admit i'm quite amazed by how this has gone to some flames. As JS & TS developper, in normal times, i would have been supporting separated repo for providing types, as requested by @levino, as i rather prefer avoiding to have any broken type provided by the original repo, third-party packages being easy to remove without touching web3 original code. However, as i read the answers it seems quite technically feasible to ensure types are correctly written before merging any PR, as @nivida provided tools & examples of others projects doing so. Comment from @jpeletier here : #1302 (comment) also makes me think that rewriting web3 into a TS form might be quite faster as it could be expected. If web3 dev team commits to review type definitions while a typescript migration is being built, i'll definitely support it, they made their point.
@levino : i don't think it's about winning or failing here but rather having a collective conversation about how to do it the best way possible. Too bad to have thrown useless flames for that. In any case, and still, thanks for providing |
@levino I think people who do this like
Very open to rewrite it in Thanks for the good discussion though @levino i do respect your views and no hard feelings 👍 |
If everybody here speaks Typescript then there is no reason not to write this library in Typescript. I repeat again: One can incrementally migrate a library to typescript. But that requires some Typescript skills. |
I will lock this issue now because I think we already seen all the arguments. |
The usage of Web3.js with TypeScript is explained in the root README.md of the 2.x and 1.x branch. |
Hello,
Web3 Typescript definition on branch 1.0 doesn't contain "default export".
Context
After importing :
import Web3 from 'web3';
Compilation Error
Module '"xxxx/node_modules/web3/index"' has no default export.
Update 1
After importing like this
import Web3 = require('web3');
I got a compilation error :This error is due to Module mode from tsconfig which is ES6.
Update 2
I remove module mode from tsconfig and it works now.
The text was updated successfully, but these errors were encountered: