Skip to content

Core: Patch push, sort & splice in jQuery >=3.7.0 #568

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

Open
wants to merge 1 commit into
base: 3.x-stable
Choose a base branch
from
Open
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
15 changes: 14 additions & 1 deletion src/jquery/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import { jQueryVersionSince } from "../compareVersions.js";
import { migratePatchAndWarnFunc } from "../main.js";
import "../disablePatches.js";

var class2type = {},
var arr = [],
push = arr.push,
sort = arr.sort,
splice = arr.splice,
class2type = {},

// Require that the "whitespace run" starts from a non-whitespace
// to avoid O(N^2) behavior when the engine would try matching "\s+$" at each space position.
Expand Down Expand Up @@ -118,3 +122,12 @@ if ( jQueryVersionSince( "3.3.0" ) ) {
"proxy", "DEPRECATED: jQuery.proxy()" );

}

if ( jQueryVersionSince( "3.7.0" ) ) {
migratePatchAndWarnFunc( jQuery.fn, "push", push, "push",
"jQuery.fn.push() is deprecated; use .add() or convert to an array" );
migratePatchAndWarnFunc( jQuery.fn, "sort", sort, "sort",
"jQuery.fn.sort() is deprecated; convert to an array before sorting" );
migratePatchAndWarnFunc( jQuery.fn, "splice", splice, "splice",
"jQuery.fn.splice() is deprecated; use .slice() or .not() with .eq()" );
}
57 changes: 57 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 @@ -318,6 +324,57 @@ TestManager.runIframeTest( "old pre-3.0 jQuery", "core-jquery2.html",
assert.ok( /jQuery 3/.test( log ), "logged: " + log );
} );

QUnit[ jQueryVersionSince( "3.7.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( "3.7.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 ) {
var tag1 = node1.tagName.toLowerCase(),
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( "3.7.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" );
} );
} );


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

Expand Down
8 changes: 8 additions & 0 deletions warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ This is _not_ a warning, but a console log message the plugin shows when it firs

**Solution:** Put a [valid doctype](http://www.w3.org/QA/2002/04/valid-dtd-list.html) in the document and ensure that the document is rendering in standards mode. The simplest valid doctype is the HTML5 one, which we highly recommend: `<!doctype html>` . The jQuery Migrate plugin does not attempt to fix issues related to quirks mode.

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

**Cause**: jQuery adds the Array `push`, `sort` & `splice` methods to the jQuery prototype. They behave 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 `$()`.

### \[jqXHR-methods\] JQMIGRATE: jQXHR.success is deprecated and removed
### \[jqXHR-methods\] JQMIGRATE: jQXHR.error is deprecated and removed
### \[jqXHR-methods\] JQMIGRATE: jQXHR.complete is deprecated and removed
Expand Down
Loading