Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat(isNaN): isNumberNaN #11242

Closed
wants to merge 1 commit into from
Closed

Conversation

PatrickJS
Copy link
Contributor

window.isNaN(‘lol’); //=> true
Number.isNaN(‘lol’); //=> false

isNaN converts it’s arguments into a Number before checking if it’s NaN. Number.isNaN fixes this but we can also polyfill it

@pkozlowski-opensource
Copy link
Member

@gdi2290 is there any reason to polyfill this particular ES6 method inside the core? Shouldn't we rather advice people to include a full / partial polyfill if they need it?

@PatrickJS
Copy link
Contributor Author

well I didn't keep this global on the angular object so it's for internal use since it's inside of the closure. Throughout the code base the team is checking if it is a number before running isNaN. Adding this would save a few lines and from this js gotcha

if (isNumber(ctrl.$modelValue) && isNaN(ctrl.$modelValue)) {
// would be
if (isNaN(ctrl.$modelValue) {
//
minVal = isNumber(val) && !isNaN(val) ? val : undefined;
// would be
minVal = !isNaN(val) ? val : undefined;

@pkozlowski-opensource
Copy link
Member

@gdi2290 so, if you believe that the changed isNaN version can be used to improve existing code, could you please include those changes in this PR?

@PatrickJS
Copy link
Contributor Author

I think would be better if I name it isNumberNaN and replace all isNaN to isNumberNaN I'll update the PR

@PatrickJS PatrickJS force-pushed the fix-isNaN branch 3 times, most recently from c5c424d to b688fff Compare March 4, 2015 20:57
@PatrickJS PatrickJS changed the title feat(isNaN): polyfill isNaN feat(isNaN): isNumberNaN Mar 4, 2015
@jbedard
Copy link
Collaborator

jbedard commented Mar 4, 2015

and instead of defining a function that wraps both the Number.isNaN and pollyfill it should only provide a pollyfill in place of Number.isNaN. Something such as...

var isNumberNaN = Number.isNaN || function(n) {
   ...
};

@PatrickJS PatrickJS force-pushed the fix-isNaN branch 2 times, most recently from f8466b9 to 08c3477 Compare March 4, 2015 21:23
@PatrickJS
Copy link
Contributor Author

good idea

var _isNaN = window.isNaN;
var isNumberNaN = Number.isNaN || function isNumberNaN(num) {
  return isNumber(num) ? _isNaN(num) : false;
};

@realityking
Copy link
Contributor

Nice idea.

Number.isNaN can be more easily (and a bit faster) polyfilled like this

function isNumberNaN(value) {
  return value !== value;
}


if (isNumber(input)) input = input.toString();
if (!isArray(input) && !isString(input)) return input;

begin = (!begin || isNaN(begin)) ? 0 : toInt(begin);
begin = (!begin || isNumberNaN(begin)) ? 0 : toInt(begin);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this one should change. This one specifically wanted "123" to pass...

window.isNaN(‘lol’); //=> true
Number.isNaN(‘lol’); //=> false

isNaN converts it’s arguments into a Number before checking if it’s NaN
@Narretz Narretz self-assigned this Aug 28, 2016
@Narretz Narretz closed this in aa306c1 Sep 5, 2016
@PatrickJS PatrickJS deleted the fix-isNaN branch September 5, 2016 22:14
Narretz pushed a commit that referenced this pull request Sep 8, 2016
window.isNaN(‘lol’); //=> true
Number.isNaN(‘lol’); //=> false

isNaN converts it’s arguments into a Number before checking if it’s NaN.
In various places in the code base, we are checking if a variable is a Number and
NaN (or not), so this can be simplified with this new method (which is not exported on the
global Angular object).

Closes #11242
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
window.isNaN(‘lol’); //=> true
Number.isNaN(‘lol’); //=> false

isNaN converts it’s arguments into a Number before checking if it’s NaN.
In various places in the code base, we are checking if a variable is a Number and
NaN (or not), so this can be simplified with this new method (which is not exported on the
global Angular object).

Closes angular#11242
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
window.isNaN(‘lol’); //=> true
Number.isNaN(‘lol’); //=> false

isNaN converts it’s arguments into a Number before checking if it’s NaN.
In various places in the code base, we are checking if a variable is a Number and
NaN (or not), so this can be simplified with this new method (which is not exported on the
global Angular object).

Closes angular#11242
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
window.isNaN(‘lol’); //=> true
Number.isNaN(‘lol’); //=> false

isNaN converts it’s arguments into a Number before checking if it’s NaN.
In various places in the code base, we are checking if a variable is a Number and
NaN (or not), so this can be simplified with this new method (which is not exported on the
global Angular object).

Closes angular#11242
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
window.isNaN(‘lol’); //=> true
Number.isNaN(‘lol’); //=> false

isNaN converts it’s arguments into a Number before checking if it’s NaN.
In various places in the code base, we are checking if a variable is a Number and
NaN (or not), so this can be simplified with this new method (which is not exported on the
global Angular object).

Closes angular#11242
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
window.isNaN(‘lol’); //=> true
Number.isNaN(‘lol’); //=> false

isNaN converts it’s arguments into a Number before checking if it’s NaN.
In various places in the code base, we are checking if a variable is a Number and
NaN (or not), so this can be simplified with this new method (which is not exported on the
global Angular object).

Closes angular#11242
petebacondarwin pushed a commit that referenced this pull request Nov 23, 2016
window.isNaN(‘lol’); //=> true
Number.isNaN(‘lol’); //=> false

isNaN converts it’s arguments into a Number before checking if it’s NaN.
In various places in the code base, we are checking if a variable is a Number and
NaN (or not), so this can be simplified with this new method (which is not exported on the
global Angular object).

Closes #11242
petebacondarwin pushed a commit that referenced this pull request Nov 24, 2016
window.isNaN(‘lol’); //=> true
Number.isNaN(‘lol’); //=> false

isNaN converts it’s arguments into a Number before checking if it’s NaN.
In various places in the code base, we are checking if a variable is a Number and
NaN (or not), so this can be simplified with this new method (which is not exported on the
global Angular object).

Closes #11242
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants