[Bug] Return Type of Methods
Opened this issue · 2 comments
Hi,
Thanks for this great library.
I'm developing a subclass by extending BigNumber
to add my custom utility methods. However, when I tried to access superclass methods, I got type errors from TypeScript. (The code works as expected; I just got TypeScript typing errors).
class ExtendedNumber extends BigNumber {
foo(): number {
return 1000;
}
bar(x: number): this {
const c = this.plus(x);
const d = c.foo(); // <--------------- ERROR: Property 'foo' does not exist on type 'BigNumber'. ts(2339)
return this;
}
}
const value = new ExtendedNumber("10.1");
console.log(value.foo());
console.log(value.bar(1));
The problem is that the return type of methods is hard-coded as BigNumber
. They should be this
instead of BigNumber
.
For example see the original code from bignumber.d.ts
file below:
plus(n: BigNumber.Value, base?: number): BigNumber;
It should be:
plus(n: BigNumber.Value, base?: number): this;
Kind Regards,
After working for a while, I realized the problem was deeper than I thought. The code contains 56 hard-coded new BigNumber()
calls.
```class ExtendedNumber extends BigNumber {
bar(): this {
return this.add(1).decimalPlaces(2);ts(2339)
}
}
let value = new ExtendedNumber("10.1"); // value is instance of ExtendedNumber
value = value.bar(); // value is instance of BigNumber !!!!!
I tried to replace all new BigNumber()
with new this.constructor()
. However, in some places, this
is not available, so I tracked the calling code and replaced, for example, parseNumeric(...)
with parseNumeric.call(this, ...)
to make this
available in the parseNumeric()
function.
Unfortunately, when I changed maxOrMin.call(this, ...)
, tests failed to the degree that I needed to investigate much more deeply, and I couldn't continue.
In the current situation, BigNumber.js seems hard to extend in a subclass. I'll be happy if you can change that, but it may need much more effort than you want to spend.
Nonetheless, this is a very good library.
Kind regards,
Hi!
BigNumber.js doesn't work the way you think it does.
Pretty much every method you call on an instance of BigNumber (e.g. .plus()
, .minus()
, .multipliedBy
, etc) does not modify the original instance, it returns a brand new instance.
Each instance is effectively immutable, which is on purpose, it prevents you from writing buggy code (for example - if you have some global constant BigNumber - you can safely do all kinds of stuff with it and don't worry about mutating it accidentally).
So the return type of a method is actually never this
, it is always BigNumber
(unless of course it's something else like with .toFixed()
, isEqualTo()
, etc).
If you really want to do return this
in your extended class methods you will have to work around that by creating a new instance of ExtendedNumber when you need it and the continue to use your custom methods on it.
bar(x: number) {
const c = new ExtendedNumber(this.plus(x));
const d = c.foo();
return this;
}
I would strongly discourage this though, it's way easier to overlook something and write buggy code this way.
It might seem like constantly creating instances is a waste of memory and CPU cycles, but:
- This is JavaScript, your app probably eats up 1000x memory in many other parts of the code, optimize those parts, not this.
- Unless you're handling literally billions of BigNumber objects every second - whatever you're doing with BigNumber probably takes <1ms to execute, even with constant creation of new objects.