Skip to content

parseInt and parseFloat should accept any and not just strings #38471

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.

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

Closed
MoritzKn opened this issue May 11, 2020 · 5 comments
Closed

parseInt and parseFloat should accept any and not just strings #38471

MoritzKn opened this issue May 11, 2020 · 5 comments
Labels
Duplicate An existing issue was already created

Comments

@MoritzKn
Copy link

MoritzKn commented May 11, 2020

TypeScript Version: 3.8.3 (playground)

Search Terms:

  • "parseInt any"
  • "parseInit type"
  • "parseInit string"
  • "parseFloat any"
  • "parseFloat type"
  • "parseFloat string"

Code

const maybeNumber: number | string = 1;

// The issue: parseInt only accepts strings so this fails,
// even though the runtime happily accepts any value...
const asNumber1 = parseInt(maybeNumber, 10)

// So the solution would be:
// Convert the number toString() and the string toString() so we can be sure that we never 
// pass numbers into the parseInt function...
const asNumber2 = parseInt(maybeNumber.toString(), 10)

// Or, better do a type check. But we are still duplicating a type check that already exits in the runtime... :/
const asNumber3 = typeof maybeNumber === 'string' ? parseInt(maybeNumber, 10) : maybeNumber

// Or simply accept defeat and out out
const asNumber4 = parseInt(maybeNumber as string, 10)

Expected behavior:

parseInt and parseFloat accepts any.

The spec gives the function the signature parseInt ( string, radix ) but also states that the argument will be converted to string internally (Let inputString be ? ToString(string).).

In my opinion, the type system should allow relaying on the behavior:

  1. It improves usability
  2. Adding runtime checks creates bigger bundles and slower code (even if only by a tiny margin)

IMHO duplicate type checks should not be encouraged.

Actual behavior:

The first parseInt call returns the type error:

Argument of type 'string | number' is not assignable to parameter of type 'string'.
  Type 'number' is not assignable to type 'string'.(2345)

Playground Link: here

@MartinJohns
Copy link
Contributor

Duplicate of #17203. Search terms: parseFloat.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label May 11, 2020
@MoritzKn
Copy link
Author

Sorry, I missed that one. If the desition was made that this behavior is intended, I accept it and don't want to repeat the argument. But if you are willing to revisit this, IMHO it would be an improvement.

I already made my argument above but I have one small addition:
In my experience types like string | number (or any but it's probably a number) are common when taking input from outside TS where you are not 100% sure what the types are. In these cases adding a runtime conversion seems more appropriate to me than a type assertion (as number) and a type check using typeof seems overly verbose.

@RyanCavanaugh
Copy link
Member

parseInt(3e20) is 3e20 but parseInt(3e21) is 3. This is not a process you should put arbitrary data through. If you really want to, add a type assertion, because this is a very dangerous process.

@MoritzKn
Copy link
Author

Fair point. Though I'd like to point out that parseFloat(Number.MAX_VALUE) === Number.MAX_VALUE works and parseInt(Number.MAX_SAFE_INTEGER) === Number.MAX_SAFE_INTEGER also works. It's just that when you get out of the integer range that you start getting unexpected behavior.

But perhaps is going to be enough for me to actually start writing typeof foo === 'number' ? foo : parseInt(foo, 10).

@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants