Skip to content

Core: Fill in & warn against $.fn.{push,sort,splice} #529

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

Merged
merged 1 commit into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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