Incompatible BigNumber types across versions

[julientregoat]

hi @ricmoo! first off, thank you for all of your work on the ethers project. I really appreciate how you've laid the project out, seems like you've thought of everything. it has made it a lot more of a pleasure working with ethereum over the past 1.5 years i've been using ethers.

encountered some compile issues when loading up a React project on a new computer. it looks like there is a discrepancy with the BigNumber type between two different versions of ethers. a dependency is on a slightly older version of ethers.

I'm wondering if this has anything to do with my version of node, since I didn't have a problem on my other computer (node 10.16 I think). I quickly looked into the issue the error describes, but I couldn't find any difference in the Hexable interface or _hex properties - looked exactly the same. I didn't have time to continue digging but I'm happy to contribute a solution if you might be able to point me in the right direction.

For now, downgrading my project to 4.0.28 works.

• node 12.14.1
• ethers 4.0.42 (my project)
• ethers 4.0.28 (dependency)

Failed to compile.

Type 'import("/<repo>/node_modules/<dependency>/node_modules/ethers/utils/bignumber").BigNumber' is not assignable to type 'import("/<repo>/node_modules/ethers/utils/bignumber").BigNumber'.
  Types have separate declarations of a private property '_hex'.  TS2322

    104 | 
    105 |     const params = await this.contract.functions.params()
  > 106 |     this.config.paramA = params.A
        |     ^

[ricmoo]

Ugh... This is actually an issue with TypeScript that has no simple work around for v4. It won't even matter if the exact same version of the library was used; two separate instances/copies of the same library will emit this error.

Basically, in TypeScript if an object contains any private members, equivalence is compared "nominally"... See type compatibility.

This is one of the main reasons I no longer use private members in v5, since there are many ways this can come up... Such as your case, but also using npm link, various bundlers, Lerna and other useful tools which do not play well with TypeScript.

In v5, variables that are presumed private are prefixed with an underscore, which does remind me: I need to run a quick grep to pull any remaining private variables because they really blow things up.

Long story short, and I apologize for this response, there isn't a good solution. At one point there was a TypeScript issue open for adding a tsc flag to allow structural equivalence (instead of nominal equivalence) on classes with private members, but I don't know what ever happened to that...

You will probable have to case things to any or possible maybe you can cast things to ethers.utils.BigNumberish, which will still get you most of the type-checking you would want, but still let things work... :(

[julientregoat]

Thank you for the quick reply!

Interesting! I had a gut feeling it was TS since it didn't really make sense. I haven't read that part of the handbook - thanks for that. I couldn't find anything on the structural equivalence proposal either. I'm don't need the latest version so this isn't a problem for me.

Regardless, I couldn't help but start playing with some ideas. What if Hexable becomes an abstract class, _hex becomes a protected property on that class, and BigNumber extends instead of implements? Unfortunately I think this would still have the same problem with past versions and it's a non issue in v5 so maybe the point is moot, but wanted to share.

abstract class Hexable {
  protected _hex: string

  constructor() {
    this._hex = 'some default value never used'
  }
}

class BigNumberA extends Hexable {
  constructor(value: string) {
    super()

    this._hex = value
  }
}

class BigNumberB extends Hexable {
  constructor(value: string) {
    super()

    this._hex = value
  }
}

let a = new BigNumberA('a')
let b = new BigNumberB('b')
a = b

[ricmoo]

Here is a short query that has far too many people talking about it to really keep up. :)

Completely fair attempt, but your example doesn't quite match what would happen... :)

Keep in mind that the Hexable abstract class would then be duplicated across the two packages (named the same, but referring to two different locations), effectively making a HexableA and HexableB, so BigNumberA extends HexableA vs BigNumberB extends HexableB would result in the a = b failing with the same error. :)

Don't worry, TypeScript has been quite thorough in making sure things will fail unnecessarily...

The most annoying thing is it doesn't even protect against what they are trying to protect against very well, and it is protecting against something they don't even try to protect against in the rest of the language. It might make sense for this to be a warning, but certainly not an error... Le sigh...

I've also thought about post-processing the .d.ts files and just removing all private members from the definitions; I think that may work...

[julientregoat]

ahh I see, I was hoping interfaces were treated differently than classes for nominal equivalence! like you said..

Don't worry, TypeScript has been quite thorough in making sure things will fail unnecessarily...

hahah!

I've also thought about post-processing the .d.ts files and just removing all private members from the definitions

if that works, that's probably the best solution going forward. probably not worth too much effort vs pushing people to v5!

[ricmoo]

I'm going to close this now, as there isn't really much I can do about it. Please feel free to re-open though, or continue comments (I monitor closed issues), since it may be useful to someone in the future that comes across this issue with the same problem.

Thanks! :)

[julientregoat]

hopefully the latest update to the private properties proposal & reflections in typescript will prevent this problem from occurring again here or in other repos :D

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#-ecmascript-private-fields

[ricmoo]

@julientregoat Oh... That looks wonderful. I will investigate it further and may go through and update lots of the library to use that.

Edit: Ah zut... They require ES6. I was planning to move to WeakMap anyways in the next (v6) of the library; this just means the syntax will be nicer. But for v5, looks like we are stuck with underscore named variables...

[julientregoat]

hah! so close!