Skip to content

ParseInt and ParseFloat should accept numbers and BigInt as Input #50828

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
rafikalid opened this issue Sep 18, 2022 · 8 comments
Closed

ParseInt and ParseFloat should accept numbers and BigInt as Input #50828

rafikalid opened this issue Sep 18, 2022 · 8 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@rafikalid
Copy link

Bug Report

parseInt and parseFloat only accept string. therefore it should accept number, Number and bigInt too

🔎 Search Terms

  • parseInt
  • parseFloat

🕗 Version & Regression Information

all versions

Please keep and fill in the line that best applies:
-->

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about parseInt and parseFloat

⏯ Playground Link

Workbench Repro

💻 Code

parseInt(854n); // Should work with bigInt
parseInt(548.25); // Should work too with numbers

parseFloat(854n); // Should work with bigInt
parseFloat(548.25); // Should work too with numbers

// Using wrappers
const n = new Number(55);
parseInt(n); // Should work

Workbench Repro

🙁 Actual behavior

Those functions accept only string to parse

🙂 Expected behavior

Should accept number, Number and bigInt too

Solution

I send a PR for this. please review and merge ❤️

@fatcerberus
Copy link

fatcerberus commented Sep 18, 2022

Duplicate of #17203 / #38471.

Quote @RyanCavanaugh:

parseInt(3e20) is 3e20 but parseInt(3e21) is 3. This is not a process you should put arbitrary data through.

@rafikalid
Copy link
Author

fix: #50829

@rafikalid
Copy link
Author

When I need to convert a number into an integer, I use this :
const myInteger= parseInt(542.33)
the same If I want to convert a big Integer into int.
I don't have a good reason why limit this to string when the real API allows any type of data?

@fatcerberus
Copy link

fatcerberus commented Sep 18, 2022

That's what Math.trunc is for. parseInt will do the wrong thing if your numbers are too large, e.g. parseInt(3e21) returns 3 (Have you actually read through the linked issues? This was discussed to death.) So while it does allow any type of data (by converting it to a string first), it's not guaranteed to do what you intended. The purpose of the type checker is to detect bugs like that.

I don't have a reason why limit this to string when to real API allows any type of data?

You could say this about most built-ins because automatic type coercion runs rampant in JS (i.e. half of the reason TS even exists). See https://stackoverflow.com/questions/41750390/what-does-all-legal-javascript-is-legal-typescript-mean

@MartinJohns
Copy link
Contributor

Besides what fatcerberus said, you should really not be using the types String and Number in place for string and number.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Sep 19, 2022
@jakebailey
Copy link
Member

Passing non-strings through parseInt and parseFloat almost assuredly will behave in an unexpected way. If you want to drop the non-integer portion of a number, use Math.trunc. If you want to convert a bigint to a number, use Number(myBigInt).

@fatcerberus
Copy link

Indeed. The key isn't that it accepts some type of data, but that it's well-behaved when passed that type. parseInt is well-behaved when passed a string; it tries to convert the string to an integer (it might not always succeed, but you know that upfront). It's not well-behaved when passed a number--it might give you back the integer portion of the number, or it might just totally mangle it, and you have no way of knowing which will happen unless you examine the number's string representation before you pass it in.

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' 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
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

6 participants