Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Bug] Return Type of Methods #368

Open
ozum opened this issue Mar 12, 2024 · 1 comment · May be fixed by #369
Open

[Bug] Return Type of Methods #368

ozum opened this issue Mar 12, 2024 · 1 comment · May be fixed by #369

Comments

@ozum
Copy link

ozum commented Mar 12, 2024 •
edited

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,
ozum added a commit to ozum/bignumber.js that referenced this issue Mar 12, 2024
@ozum
change return type from "BigNumber" to "this"
e555a49 
The hard-coded return type "BigNumber" prevents subclasses from working correctly with TypeScript types when super class methods are called.

See MikeMcl#368, this PR closes MikeMcl#368
@ozum ozum linked a pull request Mar 12, 2024 that will close this issue
Open
@ozum
Copy link
Author

ozum commented Mar 12, 2024

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,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

change return type from "BigNumber" to "this" ozum/bignumber.js
1 participant
@ozum