Skip to content

Commit

Permalink
Core: Fill in & warn against $.fn.{push,sort,splice}
Browse files Browse the repository at this point in the history
Fixes gh-473
Closes gh-529
  • Loading branch information
mgol authored Sep 16, 2024
1 parent 5845951 commit 95d05ce
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 0 deletions.
16 changes: 16 additions & 0 deletions src/jquery/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import {
import "../disablePatches.js";

var findProp,
arr = [],
push = arr.push,
sort = arr.sort,
splice = arr.splice,
class2type = {},
oldInit = jQuery.fn.init,
oldFind = jQuery.find,
Expand Down Expand Up @@ -177,3 +181,15 @@ if ( jQueryVersionSince( "3.3.0" ) ) {
"jQuery.isWindow() is deprecated"
);
}

if ( jQueryVersionSince( "4.0.0" ) ) {

// `push`, `sort` & `splice` are used internally by jQuery <4, so we only
// warn in jQuery 4+.
migrateWarnProp( jQuery.fn, "push", push, "push",
"jQuery.fn.push() is deprecated and removed; use .add or convert to an array" );
migrateWarnProp( jQuery.fn, "sort", sort, "sort",
"jQuery.fn.sort() is deprecated and removed; convert to an array before sorting" );
migrateWarnProp( jQuery.fn, "splice", splice, "splice",
"jQuery.fn.splice() is deprecated and removed; use .slice or .not with .eq" );
}
56 changes: 56 additions & 0 deletions test/unit/jquery/core.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@

QUnit.module( "core" );

function getTagNames( elem ) {
return elem.toArray().map( function( node ) {
return node.tagName.toLowerCase();
} );
}

QUnit.test( "jQuery(html, props)", function( assert ) {
assert.expect( 2 );

Expand Down Expand Up @@ -449,3 +455,53 @@ TestManager.runIframeTest( "old pre-3.0 jQuery", "core-jquery2.html",

assert.ok( /jQuery 3/.test( log ), "logged: " + log );
} );

QUnit[ jQueryVersionSince( "4.0.0" ) ? "test" : "skip" ]( "jQuery.fn.push", function( assert ) {
assert.expect( 2 );

expectWarning( assert, "jQuery.fn.push", 1, function() {
var node = jQuery( "<div></div>" )[ 0 ],
elem = jQuery( "<p></p><span></span>" );

elem.push( node );

assert.deepEqual( getTagNames( elem ), [ "p", "span", "div" ],
"div added in-place" );
} );
} );

QUnit[ jQueryVersionSince( "4.0.0" ) ? "test" : "skip" ]( "jQuery.fn.sort", function( assert ) {
assert.expect( 2 );

expectWarning( assert, "jQuery.fn.sort", 1, function() {
var elem = jQuery( "<span></span><div></div><p></p>" );

elem.sort( function( node1, node2 ) {
const tag1 = node1.tagName.toLowerCase();
const tag2 = node2.tagName.toLowerCase();
if ( tag1 < tag2 ) {
return -1;
}
if ( tag1 > tag2 ) {
return 1;
}
return 0;
} );

assert.deepEqual( getTagNames( elem ), [ "div", "p", "span" ],
"element sorted in-place" );
} );
} );

QUnit[ jQueryVersionSince( "4.0.0" ) ? "test" : "skip" ]( "jQuery.fn.splice", function( assert ) {
assert.expect( 2 );

expectWarning( assert, "jQuery.fn.splice", 1, function() {
var elem = jQuery( "<span></span><div></div><p></p>" );

elem.splice( 1, 1, jQuery( "<i></i>" )[ 0 ], jQuery( "<b></b>" )[ 0 ] );

assert.deepEqual( getTagNames( elem ), [ "span", "i", "b", "p" ],
"splice removed & added in-place" );
} );
} );
8 changes: 8 additions & 0 deletions warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,14 @@ See jQuery-ui [commit](https://github.com/jquery/jquery-ui/commit/c0093b599fcd58

**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"`.

### \[push\] JQMIGRATE: jQuery.fn.push() is deprecated and removed; use .add or convert to an array
### \[sort\] JQMIGRATE: jQuery.fn.sort() is deprecated and removed; convert to an array before sorting
### \[splice\] JQMIGRATE: jQuery.fn.splice() is deprecated and removed; use .slice or .not with .eq

**Cause**: jQuery used to add the Array `push`, `sort` & `splice` methods to the jQuery prototype. They behaved differently to other jQuery APIs - they modify the jQuery collections in place, they don't play nice with APIs like `.end()`, they were also never documented.

**Solution**: Replace `.push( node )` with `.add( node )`, `.splice( index )` with `.not( elem.eq( index ) )`. In more complex cases, call `.toArray()` first, manipulate the resulting array and convert back to the jQuery object by passing the resulting array to `$()`.

### \[unique\] JQMIGRATE: jQuery.unique is deprecated; use jQuery.uniqueSort

**Cause**: The fact that `jQuery.unique` sorted its results in DOM order was surprising to many who did not read the documentation carefully. As of jQuery 3.0 this function is being renamed to make it clear.
Expand Down

0 comments on commit 95d05ce

Please sign in to comment.