sinonjs / formatio

The cheesy object formatter
https://sinonjs.github.io/formatio/
Other
36 stars 11 forks source link

ascii(0n) throws, but other BigInts work fine #30

Closed martianboy closed 5 years ago

martianboy commented 5 years ago

What did you expect to happen?

I expected this to work with BigInts just fine. It seems to be the case for most BigInts, except 0n.

What actually happens

It throws TypeError: Cannot mix BigInt and other types, use explicit conversions.

How to reproduce

formatio.ascii(0n)

produces this error and stacktrace:

> formatio.ascii(0n)
Thrown:
TypeError: Cannot mix BigInt and other types, use explicit conversions
    at ascii (.../node_modules/@sinonjs/formatio/lib/formatio.js:77:37)
    at Object.ascii (.../node_modules/@sinonjs/formatio/lib/formatio.js:227:16)
mgred commented 5 years ago

@martianboy thanks for filing this issue. The error occures due to this line:

if (!object) { return String((1 / object) === -Infinity ? "-0" : object); }

Since 0n is falsy we run into that branch but it throws when trying to convert BigInts by operation (1/0n). For every other BigInt value it works fine and we call the built-in toString-method.

I prepared a PR to support BigInt in formatio but am not sure if this makes sense, because this feature is not yet supported by all platforms nor is it in a final stage as of the Specification

Any opinions @mroderick @mantoni @fatso83 ?

fatso83 commented 5 years ago

this feature is not yet supported by all platforms nor is it in a final stage as of the Specification

I am not sure what feature you refer to, @mgred. If you are talking about BigInt's, then this issue wouldn't be a problem in any case, since 0n would impart a syntactical error long before hitting formatio. So ... are you talking about some other feature?

In any case, we could do some feature detection for a specific branch logic that only deals with bigInts

const hasBigInt = (typeof BigInt !== 'undefined');
if(hasBigInt && someBigIntSpecificHandlingForABigIntInThisBlock) {
}
martianboy commented 5 years ago

@mgred Thanks for the reply and the PR too. I tend to agree with @fatso83, if BigInt is not supported there will be no way for a bigint to hit formatio in the first place. So I guess a simple feature detection for BigInt and special handling of it makes perfect sense.

mgred commented 5 years ago

this feature is not yet supported by all platforms nor is it in a final stage as of the Specification

I am not sure what feature you refer to, @mgred. If you are talking about BigInt's, then this issue wouldn't be a problem in any case, since 0n would impart a syntactical error long before hitting formatio. So ... are you talking about some other feature?

I refer to BigInt as feature here. 😃

In any case, we could do some feature detection for a specific branch logic that only deals with bigInts

const hasBigInt = (typeof BigInt !== 'undefined');
if(hasBigInt && someBigIntSpecificHandlingForABigIntInThisBlock) {
}

Hmm, that makes sense. I did it more or less that way in #31:

    if (typeof BigInt !== "undefined" && typeof object === "bigint") {
        return object.toString();
    }

Thank for sharing your thoughts on this 👍