MikeMcl / bignumber.js

A JavaScript library for arbitrary-precision decimal and non-decimal arithmetic
http://mikemcl.github.io/bignumber.js
MIT License
6.64k stars 741 forks source link

'BN' refers to a value, but is being used as a type here. Did you mean 'BN'? #315

Closed formula1 closed 2 years ago

formula1 commented 2 years ago

Reproduction

export const WholeNumber = BigNumber.clone({ DECIMAL_PLACES: 0 });

function clone(n: WholeNumber){
  return new WholeNumber(n);  
}

Results in

'BN' refers to a value, but is being used as a type here. Did you mean 'BN'?

My Environment

node:      16.13.0
typecript: 4.5.4

My Editor

Atom    : 1.58.0
Electron: 9.4.4
Chrome  : 83.0.4103.122
Node    : 12.14.1
MikeMcl commented 2 years ago

I don't see BN anywhere, and it is not used in the library itself, so the reproduction is lacking.

Anyway, presumably you need to define the WholeNumber type. How? By learning TypeScript, and this is not the place to do that.

Maybe you can just use:

function clone(n: typeof WholeNumber){
formula1 commented 2 years ago

I tried that, it didn't seem to work. While this may not be the place to learn typescript, if others run into a similar issue at least they may find that the issue already exists. I can use BigNumber as a type no problem. And I imagine that WholeNumber is a class that extends it? Can I just extend BigNumber and pass it the precision to the super? That would be convenient

shuckster commented 2 years ago

Just wanted to say that BN is a different library, and I've noticed that some others have had this confusion before.

MikeMcl commented 2 years ago

Just using

class WholeNumber extends BigNumber {}

will declare the WholeNumber type and should stop the error.

Bear in mind though that If BigNumber and WholeNumber have the same shape then they are considered the same type by TypeScript, so maybe you could add an extra property, for example:

class WholeNumber extends BigNumber {
  type = WholeNumber;
}
formula1 commented 2 years ago

@shuckster Should I rename my title? In the documentation they set the clone to BN so I figured it would be a good way to try to adhere to the example they provided

// Set DECIMAL_PLACES for the original BigNumber constructor
BigNumber.set({ DECIMAL_PLACES: 10 })

// Create another BigNumber constructor, optionally passing in a configuration object
BN = BigNumber.clone({ DECIMAL_PLACES: 5 })

x = new BigNumber(1)
y = new BN(1)

x.div(3)                            // '0.3333333333'
y.div(3)                            // '0.33333'

So, looking at the source code I found

BigNumber.set(configObject);

So I suppose I can extend BigNumber, then set the config immediately afterward. set() seems to be a confusing name for a method because it sounds like your modifying the number but I suppose its a static method so it can't be used by the instance and changing it may break systems which would be a major update for something so arbitrary.

after installing ts-node and creating a simple example app with files ./src/ExtendedNumber.ts and ./src/index.ts

// file = ./src/ExtendedNumber.ts

import BigNumber from "bignumber.js";

export class ExtendedNumber extends BigNumber {
  type = ExtendedNumber;
}
// I don't understand the export system perfectly, expecially when it comes to tree shaking
// But I expect that any methods run on exported files are run before the value gets exported
// Just in case I run the config function underneath the class to see if it works as I hope
ExtendedNumber.config({
  EXPONENTIAL_AT: 1e+9,
  DECIMAL_PLACES: 1
})
// file = ./index.ts
import BigNumber from "bignumber.js";
import { ExtendedNumber } from "./ExtendedNumber";

var one = new ExtendedNumber(1);
var frac = one.dividedBy(3);

console.log("should return 0.3:", frac.toString());

ExtendedNumber.config({
  EXPONENTIAL_AT: 1e+9,
  DECIMAL_PLACES: 2
});

frac = one.dividedBy(3);
console.log("should return 0.33:", frac.toString());

delete ExtendedNumber.config
delete ExtendedNumber.set

try {
  ExtendedNumber.config({
    EXPONENTIAL_AT: 1e+9,
    DECIMAL_PLACES: 3
  })
  throw new Error("shouldn't be able to config")
}catch(e){
  console.log("can't config again")
}
try {
  ExtendedNumber.set({
    EXPONENTIAL_AT: 1e+9,
    DECIMAL_PLACES: 3
  })
  throw new Error("shouldn't be able to set")
}catch(e){
  console.log("can't set again")
}

BigNumber.config.call(ExtendedNumber, {
    EXPONENTIAL_AT: 1e+9,
    DECIMAL_PLACES: 3
})

frac = one.dividedBy(3);
console.log("should return 0.333:", frac.toString());
console.log("is one a ExtendedNumber?", one instanceof ExtendedNumber);
console.log("is one a BigNumber?", one instanceof BigNumber);
console.log("is frac a ExtendedNumber?", frac instanceof ExtendedNumber);
console.log("is frac a BigNumber?", frac instanceof BigNumber);

I did the initial config in the ./src/ExtendedNumber.ts because i was worried that the export would happen before ExtendedNumber gets sent out. Fortunately it work how I wanted.

Something interesting is that

I don't know how to explain it since technically one is a ExtendedNumber, yet, due too the immutabiliy i presume, frac is constructed as a BigNumber. Strange stuff.

But we can consider this issue closed since I got what I wanted.

Btw @shuckster, Thanks for showing me BN.js. Was very impressed with their work and they even have typescript definitions on DefinitelyTyped. Lots of bitwise operations, I don't understand most of it but it looks like some fun stuff. Additionally they hold the numbers in an array so I suppose there may be no limit although, after a quick search, there is a limit. Perhaps they can support nested arrays at some point when things get crazy. Although after looking at the source code of bignumber.js it appears they also store the numbers in an array. So I suppose this is standard practice and makes sense because an array can be much larger than a number. The fact than BN.js supports mutability, though it sounds like a bad idea, has its benefits like not making the garbage collector have to work overtime.

However, I don't think it's a perfect fit for my application. For the library I've been working on, I want to give the users the option to "simplify a result" as well as export it to json which is meant to keep all the intricacies of the equation intact. This may mean division, irrational numbers or both which means decimals will be important. As a result, it's nice to see bignumber.js support the decimal system even though I store things internally as whole numbers. Perhaps when the time is right I can convert the exponents, numerators and denominators from BN into bignumber and run opertations from there. Thats far down the road but thanks for letting me know about BN.js :)

formula1 commented 2 years ago

@MikeMcl Sorry about that, didn't mean to leave you hanging. So, While its all well and good extending the library, I wanted to set the config and the example way created a class that I couldn't use as a type. While your example was a good start, the config file still was not getting set in the constructor or anything. After a bit of research, I found i can just run the .config static method on the new class after I've extended it. Completely bypassing the need to use Clone which, while a nice convenience function, doesn't work well with typescript from my experience.

formula1 commented 2 years ago

Just in case anyone is interested, I can use ExtendedNumber as a type

const two = new ExtendedNumber(2);
const three = new BigNumber(3);

function runLog(arg: ExtendedNumber){
  console.log("running in a function:", arg);
}
runLog(two);
runLog(three);

Argument of type 'BigNumber' is not assignable to parameter of type 'ExtendedNumber'. Property 'type' is missing in type 'BigNumber' but required in type 'ExtendedNumber'.

Pretty much what I was looking for

note: like @MikeMcl mentioned, you need to add the type parameter to your class definition. Otherwise Typescript will allow you to use big numbers as extended numbers which I don't think is desirable

shuckster commented 2 years ago

Should I rename my title? In the documentation they set the clone to BN so I figured it would be a good way to try to adhere to the example they provided

Apologies! I did not realise this was copy-pasted from the README. I remember that BN.js was being used alongside BigNumber seemingly unknowingly in those other threads, and just thought that might also be your own case. Pay no attention to the man behind the curtain. 👀

formula1 commented 2 years ago

Was just running some more tests and ran into another issue. It seems as though config function is updating the base BigNumber class instead of the child class. Looking at the sourcecode I don't even see where the class is getting mutated. So I have no idea how config really works. Am I on the wrong branch? There also seems to be a typescript bug or maybe it's expected behavior but I'm going to create an issue over there as well.

in the below example, here is the log

wN is Whole? true
wN is Dec? false
wN is Big? true
dN is Whole? false
dN is Dec? true
dN is Big? true
bN is Whole? false
bN is Dec? false
bN is Big? true
wN div 3: 0.333
dN div 3: 0.333
bN div 3: 0.333

here is the code

import BigNumber from "bignumber.js";

BigNumber.config({
  DECIMAL_PLACES: 5,
})

class WholeNumber extends BigNumber {}

WholeNumber.config({
  DECIMAL_PLACES: 0
})

class DecNumber extends BigNumber {}

DecNumber.config({
  DECIMAL_PLACES: 3
})

const wholeOne = new WholeNumber(1);
console.log("wN is Whole?", wholeOne instanceof WholeNumber);       // true
console.log("wN is Dec?", wholeOne instanceof DecNumber);           // false
console.log("wN is Big?", wholeOne instanceof BigNumber);           // true

const decOne = new DecNumber(1);
console.log("dN is Whole?", decOne instanceof WholeNumber);         // false
console.log("dN is Dec?", decOne instanceof DecNumber);             // true
console.log("dN is Big?", decOne instanceof BigNumber);             // true

const bigOne = new BigNumber(1);
console.log("bN is Whole?", bigOne instanceof WholeNumber);         // false
console.log("bN is Dec?", bigOne instanceof DecNumber);             // false
console.log("bN is Big?", bigOne instanceof BigNumber);             // true

function divideByThree(n: WholeNumber): WholeNumber {
  return n.div(3);
}

console.log("wN div 3:", divideByThree(wholeOne).toString());    // 0.333
console.log("dN div 3:", divideByThree(decOne).toString())       // 0.333
console.log("bN div 3:", divideByThree(bigOne).toString());      // 0.333
shuckster commented 2 years ago

Use .clone() to isolate the configs:

class WholeNumber extends BigNumber.clone() {}
class DecNumber extends BigNumber.clone() {}

console.log:

wN div 3: 0 
dN div 3: 0.333 
bN div 3: 0.33333 
formula1 commented 2 years ago

@shuckster Solid.

formula1 commented 2 years ago

I am curious, is that expected behaviour though? Shouldn't the setting the config of the child class not affect the parent or siblings?

shuckster commented 2 years ago

I can't speak for the author, but with a method like clone perhaps it was easier to offer global mutable configs as the default, rather than the exception.

I've certainly used clone to solve this problem myself.