Skip to content

Commit

Permalink
Core: Warn on jQuery.isNumeric and jQuery.type
Browse files Browse the repository at this point in the history
  • Loading branch information
dmethvin committed Jun 9, 2019
1 parent c076e19 commit 396ee60
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 46 deletions.
60 changes: 32 additions & 28 deletions src/core.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@

var oldInit = jQuery.fn.init,
oldIsNumeric = jQuery.isNumeric,
oldFind = jQuery.find,
rattrHashTest = /\[(\s*[-\w]+\s*)([~|^$*]?=)\s*([-\w#]*?#[-\w#]*)\s*\]/,
rattrHashGlob = /\[(\s*[-\w]+\s*)([~|^$*]?=)\s*([-\w#]*?#[-\w#]*)\s*\]/g;
Expand Down Expand Up @@ -71,33 +70,6 @@ jQuery.parseJSON = function() {
return JSON.parse.apply( null, arguments );
};

jQuery.isNumeric = function( val ) {

// The jQuery 2.2.3 implementation of isNumeric
function isNumeric2( obj ) {
var realStringObj = obj && obj.toString();
return !jQuery.isArray( obj ) && ( realStringObj - parseFloat( realStringObj ) + 1 ) >= 0;
}

var newValue = oldIsNumeric( val ),
oldValue = isNumeric2( val );

if ( newValue !== oldValue ) {
migrateWarn( "jQuery.isNumeric() should not be called on constructed objects" );
}

return oldValue;
};

if ( jQueryVersionSince( "3.3.0" ) ) {
migrateWarnFunc( jQuery, "isWindow",
function( obj ) {
return obj != null && obj === obj.window;
},
"jQuery.isWindow() is deprecated"
);
}

migrateWarnFunc( jQuery, "holdReady", jQuery.holdReady,
"jQuery.holdReady is deprecated" );

Expand All @@ -115,3 +87,35 @@ if ( jQueryVersionSince( "3.2.0" ) ) {
migrateWarnFunc( jQuery, "nodeName", jQuery.nodeName,
"jQuery.nodeName is deprecated" );
}

if ( jQueryVersionSince( "3.3.0" ) ) {

migrateWarnFunc( jQuery, "isNumeric", jQuery.isNumeric = function( obj ) {

// As of jQuery 3.0, isNumeric is limited to
// strings and numbers (primitives or objects)
// that can be coerced to finite numbers (gh-2662)
var type = jQuery.type( obj );
return ( type === "number" || type === "string" ) &&

// parseFloat NaNs numeric-cast false positives ("")
// ...but misinterprets leading-number strings, e.g. hex literals ("0x...")
// subtraction forces infinities to NaN
!isNaN( obj - parseFloat( obj ) );
},
"jQuery.isNumeric() is deprecated"
);

migrateWarnFunc( jQuery, "type", jQuery.type,
"jQuery.type is deprecated" );

migrateWarnFunc( jQuery, "isFunction", jQuery.isFunction,
"jQuery.isFunction() is deprecated" );

migrateWarnFunc( jQuery, "isWindow",
function( obj ) {
return obj != null && obj === obj.window;
},
"jQuery.isWindow() is deprecated"
);
}
109 changes: 94 additions & 15 deletions test/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,28 +226,88 @@ QUnit.test( "jQuery.parseJSON", function( assert ) {
} );
} );

QUnit.test( "jQuery.isNumeric", function( assert ) {
assert.expect( 8 );
QUnit.test( "isNumeric", function( assert ) {
assert.expect( 43 );

var ToString = function( value ) {
var t = jQuery.isNumeric,
ToString = function( value ) {
this.toString = function() {
return String( value );
};
};

expectWarning( assert, "changed cases", function() {
assert.equal( jQuery.isNumeric( new ToString( "42" ) ), true,
"Custom .toString returning number" );
} );
assert.ok( t( "-10" ), "Negative integer string" );
assert.ok( t( "0" ), "Zero string" );
assert.ok( t( "5" ), "Positive integer string" );
assert.ok( t( -16 ), "Negative integer number" );
assert.ok( t( 0 ), "Zero integer number" );
assert.ok( t( 32 ), "Positive integer number" );
assert.ok( t( "-1.6" ), "Negative floating point string" );
assert.ok( t( "4.536" ), "Positive floating point string" );
assert.ok( t( -2.6 ), "Negative floating point number" );
assert.ok( t( 3.1415 ), "Positive floating point number" );
assert.ok( t( 1.5999999999999999 ), "Very precise floating point number" );
assert.ok( t( 8e5 ), "Exponential notation" );
assert.ok( t( "123e-2" ), "Exponential notation string" );
assert.ok( t( "040" ), "Legacy octal integer literal string" );
assert.ok( t( "0xFF" ), "Hexadecimal integer literal string (0x...)" );
assert.ok( t( "0Xba" ), "Hexadecimal integer literal string (0X...)" );
assert.ok( t( 0xFFF ), "Hexadecimal integer literal" );

if ( +"0b1" === 1 ) {
assert.ok( t( "0b111110" ), "Binary integer literal string (0b...)" );
assert.ok( t( "0B111110" ), "Binary integer literal string (0B...)" );
} else {
assert.ok( true, "Browser does not support binary integer literal (0b...)" );
assert.ok( true, "Browser does not support binary integer literal (0B...)" );
}

expectNoWarning( assert, "unchanged cases", function() {
assert.equal( jQuery.isNumeric( 42 ), true, "number" );
assert.equal( jQuery.isNumeric( "42" ), true, "number string" );
assert.equal( jQuery.isNumeric( "devo" ), false, "non-numeric string" );
assert.equal( jQuery.isNumeric( [ 2, 4 ] ), false, "array" );
assert.equal( jQuery.isNumeric( new ToString( "devo" ) ), false,
"Custom .toString returning non-number" );
} );
if ( +"0o1" === 1 ) {
assert.ok( t( "0o76" ), "Octal integer literal string (0o...)" );
assert.ok( t( "0O76" ), "Octal integer literal string (0O...)" );
} else {
assert.ok( true, "Browser does not support octal integer literal (0o...)" );
assert.ok( true, "Browser does not support octal integer literal (0O...)" );
}

assert.equal( t( new ToString( "42" ) ), false, "Only limited to strings and numbers" );
assert.equal( t( "" ), false, "Empty string" );
assert.equal( t( " " ), false, "Whitespace characters string" );
assert.equal( t( "\t\t" ), false, "Tab characters string" );
assert.equal( t( "abcdefghijklm1234567890" ), false, "Alphanumeric character string" );
assert.equal( t( "xabcdefx" ), false, "Non-numeric character string" );
assert.equal( t( true ), false, "Boolean true literal" );
assert.equal( t( false ), false, "Boolean false literal" );
assert.equal( t( "bcfed5.2" ), false, "Number with preceding non-numeric characters" );
assert.equal( t( "7.2acdgs" ), false, "Number with trailing non-numeric characters" );
assert.equal( t( undefined ), false, "Undefined value" );
assert.equal( t( null ), false, "Null value" );
assert.equal( t( NaN ), false, "NaN value" );
assert.equal( t( Infinity ), false, "Infinity primitive" );
assert.equal( t( Number.POSITIVE_INFINITY ), false, "Positive Infinity" );
assert.equal( t( Number.NEGATIVE_INFINITY ), false, "Negative Infinity" );
assert.equal( t( new ToString( "Devo" ) ), false, "Custom .toString returning non-number" );
assert.equal( t( {} ), false, "Empty object" );
assert.equal( t( [] ), false, "Empty array" );
assert.equal( t( [ 42 ] ), false, "Array with one number" );
assert.equal( t( function() {} ), false, "Instance of a function" );
assert.equal( t( new Date() ), false, "Instance of a Date" );
} );

QUnit[ typeof Symbol === "function" ? "test" : "skip" ]( "isNumeric(Symbol)", function( assert ) {
assert.expect( 2 );

assert.equal( jQuery.isNumeric( Symbol() ), false, "Symbol" );
assert.equal( jQuery.isNumeric( Object( Symbol() ) ), false, "Symbol inside an object" );
} );

QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( ".isNumeric (warn)", function( assert ) {
assert.expect( 3 );

expectWarning( assert, "warning on isNumeric (and possibly type)", function() {
assert.equal( jQuery.isNumeric( 42 ), true, "isNumeric number" );
assert.equal( jQuery.isNumeric( "nope" ), false, "isNumeric non number" );
} );
} );

QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( "jQuery.isWindow", function( assert ) {
Expand Down Expand Up @@ -320,6 +380,25 @@ QUnit[ jQueryVersionSince( "3.2.0" ) ? "test" : "skip" ]( "jQuery.nodeName", fun
})
});

QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( "jQuery.isFunction", function( assert ) {
assert.expect( 4 );

expectWarning( assert, "jQuery.isFunction", function() {
assert.equal( jQuery.isFunction( function(){} ), true, "function is function" );
assert.equal( jQuery.isFunction( {} ), false, "object not function" );
assert.equal( jQuery.isFunction( 1 ), false, "number not function" );
} );
} );

QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( "jQuery.type (warn)", function( assert ) {
assert.expect( 2 );

expectWarning( assert, "jQuery.type", 1, function() {
assert.equal( jQuery.type( [] ) , "array", "[] is array" );
} );

} );

TestManager.runIframeTest( "old pre-3.0 jQuery", "core-jquery2.html",
function( assert, jQuery, window, document, log ) {
assert.expect( 1 );
Expand Down
12 changes: 9 additions & 3 deletions warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,17 @@ See jQuery-ui [commit](https://github.com/jquery/jquery-ui/commit/c0093b599fcd58

**Solution**: Replace any use of `jQuery.parseJSON` with `JSON.parse`.

### JQMIGRATE: jQuery.isNumeric() should not be called on constructed objects
### JQMIGRATE: jQuery.isNumeric() is deprecated

**Cause**: The intended use case of `jQuery.isNumeric` is to see if its argument is either already a number, or a string that can be converted to a number. In jQuery 3.0 some edge cases changed to not return the same values. In particular, a constructed object (one created with `new MyObject()`) that contains a `.toString()` method is never considered to be numeric, even if that method returns a string that could be converted to a number. Please do not taunt this method.
**Cause**: This method was used by jQuery to determine if certain string arguments could be converted to numbers, but the name led people to apply their own interpretations to what the method means. As a result, it often doesn't meet the needs of specific cases. For example, a 25-character string of only digits is technically a valid number, but JavaScript cannot represent it accurately. The string `"0x251D"` is a valid hexadecimal number but may not be acceptable numeric input to a web form.

**Solution**: Either use a different test for being numeric, or call the object's `.toString()` method before calling the jQuery method: `jQuery.isNumeric( myObject.toString() )`.
**Solution**: Use a test for being numeric that makes sense for the specific situation. For example, instead of `jQuery.isNumeric(string)`, use `isNan(parseFloat(string))` if a floating point number is expected, or `string.test(/^[0-9]{1,8}$/)` if a sequence of 1 to 8 digits is expected.

### JQMIGRATE: jQuery.type() is deprecated

**Cause**: This method returns a string that indicates the type of the argument, for example `"number"` or `"function"`. However, as the JavaScript language evolves this method has become problematic because new language constructs might require this function to either return a new string (potentially breaking existing code) or somehow map new constructs into existing strings (again, potentially breaking existing code). Examples of new recent JavaScript features include asynchronous functions, class constructors, `Symbol`s, or functions that act as iterators.

**Solution**: Review code that uses `jQuery.type()` and use a type check that is appropriate for the situation. For example. if the code expects a plain function, check for `typeof arg === "function"`.

### JQMIGRATE: jQuery.unique is deprecated; use jQuery.uniqueSort

Expand Down

0 comments on commit 396ee60

Please sign in to comment.