From c2e25190ef05fca434dc9b74c8806424288b984d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Go=C5=82e=CC=A8biowski-Owczarek?= Date: Tue, 19 Nov 2024 22:13:41 +0100 Subject: [PATCH] Widget: Increase performance for large sortable/draggable collections Fixes gh-2062 --- tests/unit/widget/classes.js | 18 ++++++------ ui/.eslintrc.json | 1 + ui/widget.js | 56 +++++++++++++++++++++++------------- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/tests/unit/widget/classes.js b/tests/unit/widget/classes.js index 609f9903202..4a595b0f143 100644 --- a/tests/unit/widget/classes.js +++ b/tests/unit/widget/classes.js @@ -155,27 +155,27 @@ QUnit.test( "Classes elements are untracked as they are removed from the DOM", f var widget = $( "#widget" ).classesWidget(); var instance = widget.classesWidget( "instance" ); - assert.equal( instance.classesElementLookup[ "ui-classes-span" ].length, 3, + assert.equal( instance.classesElementLookup[ "ui-classes-span" ].size, 3, "Widget is tracking 3 ui-classes-span elements" ); - assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].length, 3, + assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].size, 3, "Widget is tracking 3 ui-core-span-null elements" ); - assert.equal( instance.classesElementLookup[ "ui-core-span" ].length, 3, + assert.equal( instance.classesElementLookup[ "ui-core-span" ].size, 3, "Widget is tracking 3 ui-core-span elements" ); widget.find( "span" ).eq( 0 ).remove(); - assert.equal( instance.classesElementLookup[ "ui-classes-span" ].length, 2, + assert.equal( instance.classesElementLookup[ "ui-classes-span" ].size, 2, "After removing 1 span from dom 2 ui-classes-span elements are tracked" ); - assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].length, 2, + assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].size, 2, "After removing 1 span from dom 2 ui-core-span-null elements are tracked" ); - assert.equal( instance.classesElementLookup[ "ui-core-span" ].length, 2, + assert.equal( instance.classesElementLookup[ "ui-core-span" ].size, 2, "After removing 1 span from dom 2 ui-core-span elements are tracked" ); widget.find( "span" ).remove(); - assert.equal( instance.classesElementLookup[ "ui-classes-span" ].length, 0, + assert.equal( instance.classesElementLookup[ "ui-classes-span" ].size, 0, "No ui-classes-span elements are tracked after removing all spans" ); - assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].length, 0, + assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].size, 0, "No ui-core-span-null elements are tracked after removing all spans" ); - assert.equal( instance.classesElementLookup[ "ui-core-span" ].length, 0, + assert.equal( instance.classesElementLookup[ "ui-core-span" ].size, 0, "No ui-core-span elements are tracked after removing all spans" ); } ); diff --git a/ui/.eslintrc.json b/ui/.eslintrc.json index eaba81af2a2..b9f196756ae 100644 --- a/ui/.eslintrc.json +++ b/ui/.eslintrc.json @@ -8,6 +8,7 @@ }, "env": { + "es6": true, "browser": true, "jquery": true, "node": false diff --git a/ui/widget.js b/ui/widget.js index d5fbd885cf0..e5114eb1b97 100644 --- a/ui/widget.js +++ b/ui/widget.js @@ -301,10 +301,10 @@ $.Widget.prototype = { this.uuid = widgetUuid++; this.eventNamespace = "." + this.widgetName + this.uuid; - this.bindings = $(); + this.bindings = new Set(); this.hoverable = $(); this.focusable = $(); - this.classesElementLookup = {}; + this.classesElementLookup = Object.create( null ); if ( element !== this ) { $.data( element, this.widgetFullName, this ); @@ -355,7 +355,7 @@ $.Widget.prototype = { this._destroy(); $.each( this.classesElementLookup, function( key, value ) { - that._removeClass( value, key ); + that._removeClass( $( Array.from( value ) ), key ); } ); // We can probably remove the unbind calls in 2.0 @@ -368,7 +368,7 @@ $.Widget.prototype = { .removeAttr( "aria-disabled" ); // Clean up events and states - this.bindings.off( this.eventNamespace ); + $( Array.from( this.bindings ) ).off( this.eventNamespace ); }, _destroy: $.noop, @@ -450,16 +450,12 @@ $.Widget.prototype = { currentElements = this.classesElementLookup[ classKey ]; if ( value[ classKey ] === this.options.classes[ classKey ] || !currentElements || - !currentElements.length ) { + !currentElements.size ) { continue; } - // We are doing this to create a new jQuery object because the _removeClass() call - // on the next line is going to destroy the reference to the current elements being - // tracked. We need to save a copy of this collection so that we can add the new classes - // below. - elements = $( currentElements.get() ); - this._removeClass( currentElements, classKey ); + elements = $( Array.from( currentElements ) ); + this._removeClass( elements, classKey ); // We don't use _addClass() here, because that uses this.options.classes // for generating the string of classes. We want to use the value passed in from @@ -509,7 +505,7 @@ $.Widget.prototype = { return elements; } ) .some( function( elements ) { - return elements.is( element ); + return elements.has( element ); } ); if ( !isTracked ) { @@ -525,12 +521,24 @@ $.Widget.prototype = { function processClassString( classes, checkOption ) { var current, i; for ( i = 0; i < classes.length; i++ ) { - current = that.classesElementLookup[ classes[ i ] ] || $(); + current = that.classesElementLookup[ classes[ i ] ] || new Set(); if ( options.add ) { bindRemoveEvent(); - current = $( $.uniqueSort( current.get().concat( options.element.get() ) ) ); + + // This function is invoked synchronously, so the reference + // to `current` is not an issue. + // eslint-disable-next-line no-loop-func + $.each( options.element, function( _i, node ) { + current.add( node ); + } ); } else { - current = $( current.not( options.element ).get() ); + + // This function is invoked synchronously, so the reference + // to `current` is not an issue. + // eslint-disable-next-line no-loop-func + $.each( options.element, function( _i, node ) { + current.delete( node ); + } ); } that.classesElementLookup[ classes[ i ] ] = current; full.push( classes[ i ] ); @@ -553,9 +561,7 @@ $.Widget.prototype = { _untrackClassesElement: function( event ) { var that = this; $.each( that.classesElementLookup, function( key, value ) { - if ( $.inArray( event.target, value ) !== -1 ) { - that.classesElementLookup[ key ] = $( value.not( event.target ).get() ); - } + value.delete( event.target ); } ); this._off( $( event.target ) ); @@ -583,6 +589,7 @@ $.Widget.prototype = { }, _on: function( suppressDisabledCheck, element, handlers ) { + var i; var delegateElement; var instance = this; @@ -600,7 +607,11 @@ $.Widget.prototype = { delegateElement = this.widget(); } else { element = delegateElement = $( element ); - this.bindings = this.bindings.add( element ); + for ( i = 0; i < element.length; i++ ) { + $.each( element, function( _i, node ) { + instance.bindings.add( node ); + } ); + } } $.each( handlers, function( event, handler ) { @@ -637,12 +648,17 @@ $.Widget.prototype = { }, _off: function( element, eventName ) { + var that = this; + eventName = ( eventName || "" ).split( " " ).join( this.eventNamespace + " " ) + this.eventNamespace; element.off( eventName ); + $.each( element, function( _i, node ) { + that.bindings.delete( node ); + } ); + // Clear the stack to avoid memory leaks (#10056) - this.bindings = $( this.bindings.not( element ).get() ); this.focusable = $( this.focusable.not( element ).get() ); this.hoverable = $( this.hoverable.not( element ).get() ); },