-
Notifications
You must be signed in to change notification settings - Fork 27.3k
feat(isNaN): isNumberNaN #11242
feat(isNaN): isNumberNaN #11242
Conversation
|
@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? |
|
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; |
|
@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? |
|
I think would be better if I name it |
c5c424d to
b688fff
Compare
|
and instead of defining a function that wraps both the var isNumberNaN = Number.isNaN || function(n) {
...
}; |
f8466b9 to
08c3477
Compare
|
good idea var _isNaN = window.isNaN;
var isNumberNaN = Number.isNaN || function isNumberNaN(num) {
return isNumber(num) ? _isNaN(num) : false;
}; |
|
Nice idea.
function isNumberNaN(value) {
return value !== value;
} |
src/ng/filter/limitTo.js
Outdated
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.
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
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
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
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
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
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
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
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
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
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