From bbef4902f933c7221de83d76ead61b0ac44b5794 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 25 Mar 2016 00:35:39 +0000 Subject: [PATCH] mediawiki.widgets: Remove use of bind() for lexical 'this' binding Follows-up 4636ac79dd. Bind can be useful when needing to pass an instance method elewhere. However when nesting closures, use the scope directly instead of binding 'this' several layers deep. This is fragile at best and doesn't make it less confusing. Leave the natural 'this' unchanged. Change this can go wrong both ways and results in unpredictable behaviour and confusing code that is hard to review. Sometimes one means the outer 'this' but gets the inner one, and sometimes you need the inner one (e.g. inside callbacks for jQuery). Consistently assign a variable and use scope to access objects. Besides, one can't escape it when you need both. This avoids an entire class of potential errors. It also performs marginally better without a binding but that's besides the point as there are other valid uses of bind(). Change-Id: I1fcfdbd8fa7c52e150cadd8a520591e700c5bfa9 --- .../mw.widgets.CategoryCapsuleItemWidget.js | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/resources/src/mediawiki.widgets/mw.widgets.CategoryCapsuleItemWidget.js b/resources/src/mediawiki.widgets/mw.widgets.CategoryCapsuleItemWidget.js index 5369d35e71..58115c3140 100644 --- a/resources/src/mediawiki.widgets/mw.widgets.CategoryCapsuleItemWidget.js +++ b/resources/src/mediawiki.widgets/mw.widgets.CategoryCapsuleItemWidget.js @@ -25,23 +25,24 @@ * @private */ PageExistenceCache.prototype.processExistenceCheckQueue = function () { - var queue, titles; + var queue, titles, + cache = this; if ( this.currentRequest ) { // Don't fire off a million requests at the same time this.currentRequest.always( function () { - this.currentRequest = null; - this.processExistenceCheckQueueDebounced(); - }.bind( this ) ); + cache.currentRequest = null; + cache.processExistenceCheckQueueDebounced(); + } ); return; } queue = this.existenceCheckQueue; this.existenceCheckQueue = {}; titles = Object.keys( queue ).filter( function ( title ) { - if ( this.existenceCache.hasOwnProperty( title ) ) { - queue[ title ].resolve( this.existenceCache[ title ] ); + if ( cache.existenceCache.hasOwnProperty( title ) ) { + queue[ title ].resolve( cache.existenceCache[ title ] ); } - return !this.existenceCache.hasOwnProperty( title ); - }.bind( this ) ); + return !cache.existenceCache.hasOwnProperty( title ); + } ); if ( !titles.length ) { return; } @@ -53,10 +54,10 @@ } ).done( function ( response ) { $.each( response.query.pages, function ( index, page ) { var title = new ForeignTitle( page.title ).getPrefixedText(); - this.existenceCache[ title ] = !page.missing; - queue[ title ].resolve( this.existenceCache[ title ] ); - }.bind( this ) ); - }.bind( this ) ); + cache.existenceCache[ title ] = !page.missing; + queue[ title ].resolve( cache.existenceCache[ title ] ); + } ); + } ); }; /** @@ -107,6 +108,7 @@ * @cfg {string} [apiUrl] API URL, if not the current wiki's API */ mw.widgets.CategoryCapsuleItemWidget = function MWWCategoryCapsuleItemWidget( config ) { + var widget = this; // Parent constructor mw.widgets.CategoryCapsuleItemWidget.parent.call( this, $.extend( { data: config.title.getMainText(), @@ -137,8 +139,8 @@ this.constructor.static.pageExistenceCaches[ this.apiUrl ] .checkPageExistence( new ForeignTitle( this.title.getPrefixedText() ) ) .done( function ( exists ) { - this.setMissing( !exists ); - }.bind( this ) ); + widget.setMissing( !exists ); + } ); /*jshint +W024*/ }; -- 2.20.1