-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Chore (fix):removed shorthand type conversions #12457
Conversation
@@ -20,7 +20,7 @@ assert(process.argv[3], 'Pass the number of iterations'); | |||
|
|||
const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); | |||
const method = process.argv[2]; | |||
const calls = +process.argv[3]; | |||
const calls = Number(process.argv[3]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep in mind that Number, Boolean and String can be overriden with something faulty, so +'string'
, !!
and "" + number
are mostly safest approach
but I guess in jest repo it doesn't matter much
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a lint rule we can activate for this?
@@ -20,7 +20,7 @@ assert(process.argv[3], 'Pass the number of iterations'); | |||
|
|||
const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); | |||
const method = process.argv[2]; | |||
const calls = +process.argv[3]; | |||
const calls = Number(process.argv[3]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should use parseInt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes parseInt seems more appropraite here
Let me see, if there is, than we can activate a😀 |
You need to rebase btw, after #12447 |
@SimenB as you asked if there's a lint rule and it does exist - https://eslint.org/docs/rules/no-implicit-coercion I will add this and then we will get other occurrence , do i need to pull a seperare PR for the rule? or do it here |
@SimenB i feel i have messed up this one, 😟😟, is it ok i pull a fresh one and then also add the linting rule ? |
Just reset from |
@SimenB idk its not working , its already a mess ☹☹, better would be to close this? |
i am really really sorrry , for the last time let me pull a new one and fix the occurunces |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
In JavaScript, there are a lot of different ways to convert value types. Some of them might be hard to read and understand. specially if we try to use shorthand type conversion .