Web3.js: Lots of typings are incorrect or documented incorrectly in 1.2.2

[haltman-at]

Issue

Lots of typings are incorrect in 1.2.2. In some cases this is because the typing matches the documentation rather than the implementation, but in these cases both the typing and the documentation should be updated (especially because in at least one case the documented typing is completely unworkable).

A number of examples follow. This list is in no way intended to be exhaustive; it's simply the problems that I personally have run into. But given the existence of these problems, I think you may want to give all the typings a thorough review.

Examples

getStorageAt

Let's start with the worst case. The second argument to getStorageAt specifies the address of the storage slot from which to retrieve storage. Currently it is typed as a number. This is unworkable, because storage slot addresses are 256 bits long -- and yes, large ones, outside the range of a Javascript safe integer, are in fact commonly used.

Fortunately, the function does not actually only take numbers for that argument; numeric strings also work, as do BNs. (Possibly BigNumber as well; I didn't check.) But, currently, in order to use this function, one must perform a nonsensical coercion by means of unknown.

So, the type of that argument should be number | string | BN (or possibly something even more permissive depending on what the function actually takes), and the documentation should be updated to match.

Log.topics

The type of the topics field on the Log type is (string | string[])[]. This is incorrect, in that it is overly broad, which on a return type is a problem. A log returned by getPastLogs has, as its topic field, a simple array of strings (these strings being the topics). That is, it should have type string[]. Instead it has been typed as (string | string[])[].

It looks to me like there was a mixup here between the topics field on the Log type, and the topics field on the PastLogsOptions type. The latter should indeed be (string | string[])[], as you can pass in either an individual address or a list of addresses. However, there is no reason that these two topics fields on these two different types should have the same type. They work differently, and their individual types should reflect that.

More on PastLogsOptions

PastLogsOptions's topics field actually shouldn't be (string | string[])[] (or rather, Array<string | string[]>... I'm not sure what the difference here is between T[] and Array<T>, so I'm going to ignore the distinction if you don't mind), but rather (string | string[] | null)[], same as with LogsOptions.

The typings on fromBlock and toBlock are both number | string, which works, but is a bit permissive. What's actually accepted is number | "latest" | "pending", so you might want to restrict to that?

More on block typing

Let's talk some more about ways of specifying blocks. Currently, functions that allow one to specify a block (not counting getPastLogs) take that block as number | string. But actually, BN's and BigNumbers also work. The typing, and documentation, should be updated to reflect this.

In fact, for many of these functions in web3.eth, the documentation is even more wrong than that, because the docs don't make clear that you can use a block hash instead of a block number, even though you can, for all of them (except getPastLogs, and possibly subscribe which I haven't tried). So, this should also be fixed.

currentProvider

The web3.currentProvider object has type string. Frequently, it is not a string, but rather a complex object with methods such as send. A Provider type should be created to match how providers actually work, and should be documented.

[joshstevens19]

Hey thanks for this issue.

The typings where taken from the 2.x branch which were created from the code itself. That code was a lot different to the 1.x branch so we tried our best to sort as much out as we could but we knew there would be some issues for sure. The typings are a lot better then the ones on @types but not perfect that’s for sure.

When typings are not generated automatically it is very hard to get it correct all the time.

We are trying to get any open community members to help fixing any bad typings you see so we would be super open to a PR from you with those above and very thankful.

I think some of the ones above have already been addressed but not released but I’m happy to go through the ones you have raised and address them 👍

Appreciate the in-depth issue. If you do spot any more do raise another issue and we will be hot on fixing them as soon as possible for you.

[nivida]

Thanks for opening this issue and I apologize for those wrongly defined type definitions. The mentioned getStorageAt types are already fixed and will be merged asap (https://github.com/ethereum/web3.js/pull/3180). We will adjust the other mentioned types in the next days.

[haltman-at]

Thanks a bunch! Unfortunately I made some mistakes in my issue writeup above and I'd like to correct them here.

Actually just one mistake -- the PastLogsOption part. I forgot that, contrary to what I wrote, block numbers are also accepted if given as strings, and BNs and BigNumbers are also accepted.

So, above I said the type is too permissive, but actually it's too restrictive, in that BN and BigNumber should also be included.

I mean, ideally we'd be able to provide more specific types than string for things like numeric strings and hex strings, but we can't, so... yeah. :-/ This is probably worth pointing out in the documentation -- that while other functions (again, possibly excluding subscribe, I know nothing about that) will allow a block hash in place of a block number, and will accept "genesis", this one won't. (This actually kind of is documented, but I think it's worth making more explicit, especially given how many functions are in fact more permissive than documented.)

Or, alternatively, the function itself could be modified to accept these things. :) (If that's possible, anyway).

[nivida]

and BigNumbers are also accepted.

Oh! thanks for the hint! I've totally overseen the BigNumber support. I will add it to the BlockNumber type and documentation in PR #3199.