Typescript: isNaN is restricted to number as arguments only

Created on 21 Oct 2019 · 8Comments · Source: microsoft/TypeScript

TypeScript Version: 3.3.3333

Search Terms: isNaN

Code

// A *self-contained* demonstration of the problem follows...
// Test this by running `tsc` on the command-line, rather than through another build tool such as Gulp, Webpack, etc.

let someUnionType:string|number = "42";
if(isNaN(someUnionType)){
  // ..do something here if true
}else{
  // ..do something else if false
}

Expected behavior:
I expect isNaN to test a value to determine if it is NaN or not NaN, and return a boolean value accordingly.

I don't feel as though I should have to convert the union value to a number first and then test the result of that value, since isNaN will do the same without the added step. Converting a value to NaN or number using Number(x) then requires passing the result to isNaN(Number(x)) to get a true or false result.

According to some examples found at isNaN() - JavaScript | MDN, isNaN seems to handle such tests appropriately without first converting to number first.

Also, the polyfill for isNaN is:

credit: MDN isNaN polyfill

var isNaN = function(value) {
    var n = Number(value);
    return n !== n;
};

Since Number is permitted to take a value of type any, then it would seems as though isNaN should be permitted to do the same since it is basically just a different way to use Number. Also, I feel like it just breaks how isNaN was intended to be used. See this - "Confusing_special-case_behavior" for more details.

One last note, please see Useful special-case behavior for one final reason why isNaN should accept the same type parameters as Number.

Actual behavior:
Typescript errors out at before test if argument isn't of type number.
TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.

Playground Link:
Playground Link
Related Issues:

Duplicate

Source

bpazo

Most helpful comment

Please do respect the "Search Terms: isNaN" you wrote.

trotyl on 21 Oct 2019

👍4

All 8 comments

Just realized there is a newer version of TypeScript v.3.6.3
Still get the same results. Here is a playground link

bpazo on 21 Oct 2019

The problem you're encountering is essentially,

Just because a function can accept an argument of type T, should we allow it during compile-time?

For example,

function is23 (a : number) : boolean {
  return a == 23;
}

You can make the argument that is23() will work fine with is23("23").
So, the parameter should be changed to (a : number|string).

Actually, you can make the argument (har) that it will work fine with... An argument of any type.
So, the parameter should be changed to (a : any).

But...
Is that really what is intended?

My opinion is that isNaN (n : number) : boolean is the right signature.
It would bring me great displeasure to see it changed to something more permissive.

I think passing a not number type to isNaN() is a code smell.
It's on par with using logical operators on non-boolean operands.
Or using logical operators as if-statements.
Or returning a value in a void function.
etc.

If you really want to do this, though, you can take advantage of declaration merging,

declare function isNaN(x: string | number): boolean;

let someUnknownType: string | number = "42";
if(isNaN(someUnknownType)){ //OK
  // ..do something here
}else{
  // ..do something else
}

Playground

AnyhowStep on 21 Oct 2019

👍1

@AnyhowStep

Please see this quote from MDN

Actually, isNaN(x), isNaN(x - 0), isNaN(Number(x)), Number.isNaN(x - 0), and Number.isNaN(Number(x)) always return the same and in JavaScript isNaN(x) is just the shortest possible form to express each of these terms.

Restricting isNaN isn't doing anything different than using isNaN(Number(x)). Code smell or not, the former is just a shorter way to do the same thing.

bpazo on 21 Oct 2019

Please do respect the "Search Terms: isNaN" you wrote.

trotyl on 21 Oct 2019

👍4

Declaration merging. You get what you want, and everyone else gets to keep better type-safety.

declare function isNaN(x: string | number): boolean;

let someUnknownType: string | number = "42";
if(isNaN(someUnknownType)){ //OK
  // ..do something here
}else{
  // ..do something else
}

Playground

AnyhowStep on 21 Oct 2019

@trotyl
My apologies. I did try to search for isNaN before posting, but after viewing your comment I tried again and now realize that there is a default filter set in the search input. Not a good excuse, but this is my first issue report that I've made for anything on github. Lesson learned.

I've read through all the various thoughts on the matter now. It seems that this issue is one that is debatable by many at best and will most likely never appease everyone. So, I'll make a work around to suit my needs. Thanks for the feedback.

bpazo on 21 Oct 2019

Yeah, it took me a while before I figured out (probably was told by @RyanCavanaugh now that I think about it) that there is a good and a bad way to search for issues. The general search bar at the top returns many more relevant results than the issue-specific search in the page itself, for whatever reason.

searches

jcalz on 21 Oct 2019

What's worse is that you want the 'bad' box if you're searching for Closed issues, since the ordering from the 'good' box strongly deprioritizes closed issues.