Stop recommending Number.isNaN over isNaN

[folderol4]

I like the idea of no-restricted-globals, but I've noticed that this particular recommendation comes with a behavior change:
https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb-base/rules/best-practices.js#L231-L234

isNaN(undefined) -> true
isNaN('state') -> true
Number.isNaN(undefined) -> false
Number.isNaN('state') -> false

To me if the behavior is different it should be listed as a recommended alternative and may cause people to introduce bugs.

[ljharb]

That's the whole reason to do it (and the whole reason Number.isNaN was added to the language 5 years ago); you shouldn't be passing non-number types into isNaN in the first place.

[folderol4]

Ok so The isNaN() function determines whether a value is NaN or not. I guess if you are using it to explicitly check for NaN as intended then the recommendation is correct. I'll close this.

[apieceofbart]

I completely agree with the OP - the fact the rule suggestion says Unexpected use of 'isNaN'. Use Number.isNaN instead means that people will blindly replace isNaN for Number.isNaN.In fact, I'm just fixing code that was most probably written because of this suggestion.
I think the best way would be to ditch suggestion or to replace the second part with "User Number.isNaN(Number(x)" instead.

[ljharb]

@apieceofbart the suggestion doesn't tell you to blindly make a replacement, because the replacement isn't in backticks - and if it were trying to suggest that, it'd just do an autofix.

If someone's going to do a naive replacement then they're probably doomed to screw up no matter what the message says.

[apieceofbart]

All I am saying is I see the message: "Unexpected use of 'isNaN'. Use Number.isNaN instead" with a link that is hardly visible and not clickable. To me "use X instead" means I can replace one with the other. Of course I won't argue with you but I think this issue and other issues on the same topics + a fact that I am now fixing code that was caused exactly because the person interpreted the rule as a replacement, is a proof that the message is misleading.

[ljharb]

If tests weren’t covering it there’s not much to be done.

[kevbost]

@ljharb @apieceofbart I just became that doomed to screw up no matter what the message says person while instantiating eslint on a legacy codebase.

4 instances of this, easy fix, just prepend with Number. I have plenty of experience and still blindly fell for this suggestion. It's misleading -- full stop -- and is going to continue to mislead people and cause bugs, not prevent them.

Unexpected use of 'isNaN'. Use Number.isNaN instead. eslint(no-restricted-globals)

[ljharb]

@kevbost I'm happy to improve the message if that would clear it up, but not having tests is the cause of your issue, not the linter.