In future when Promise's are supported everywhere we will want to use this instead of $.Deferred. We warn against their usage in the code to prevent new introductions but we should fix the existing ones. This will ensure our code remains future proof.
https://caniuse.com/#feat=promises
In preparation for this however, we should deprecate our usage of done, fail and always inside MobileFrontend.
Replace all calls to jQuery.Deferred.done/fail/always(). Watch out:
- new mw.Api().get().done().abort is defined but new mw.Api().get().then().abort is undefined; XMLHttpRequests may have to be preserved as an intermediate to return an abortable Deferred. See this example.
- Return the final Deferred, not an intermediate Deferred. e.g.:
ag '\.(done|fail|always)\(' function promiseResultNotReturned() { var p = new Promise( (resolve, reject) => resolve() ); p.then( () => 'returning from then'); return p; } function promiseResultReturned() { var p = new Promise( (resolve, reject) => resolve() ); var p2 = p.then( () => 'returning from return of then' ); return p2; } var y = promiseResultReturned(); var n = promiseResultNotReturned(); y /* Promise (fulfilled: returning from return of then) */ n /* Promise (fulfilled: undefined) */
- Be careful when replacing .done/.fail, specially .fail, as it behaves differently than .catch. .fail will keep the promise rejected, but adding a .catch handler will make the promise resolved unless you throw inside the catch handler again! See https://github.com/wikimedia/mediawiki-extensions-Popups/blob/e351fd15500e1ab1eb8a7c2e3b975a0d528ece6c/src/actions.js#L137-L138 for an example in popups (that was just a .fail before)
- done returns the original promise, and then returns a new promise
- from https://jquery.com/upgrade-guide/3.0/#breaking-change-and-feature-jquery-deferred-is-now-promises-a-compatible:
- done() calls are abortable, then are not. E.g., new mw.Api().get().done().abort is defined new mw.Api().get().then().abort is undefined. Beware of functions returning API Deferreds-- abortion may be attempted by clients.
Deferred methods such as .done(), .fail(), and .pipe() retain their old behavior and so are not Promises/A+ compliant. If you require synchronous resolution, do not want exceptions converted to rejection values or rejection callback returns converted to fulfillment values, or want thrown errors to bubble out of the function where they occur, you can use these methods instead of .then() and .catch()
Current usage:
resources/mobile.editor.api/EditorGateway.js 189: self.api.postWithToken( 'edit', apiOptions ).done( function ( data ) { 252: } ).fail( function () { 300: this._pending = this.api.post( options ).done( function ( resp ) { 317: } ).fail( function () { resources/mobile.editor.common/EditorOverlayBase.js 356: return OO.ui.confirm( mw.msg( 'mobile-frontend-editor-cancel-confirm' ) ).done( function ( confirmed ) { resources/mobile.editor.overlay/EditorOverlay.js 310: this.gateway.getPreview( params ).done( function ( result ) { 320: } ).fail( function () { 322: } ).always( function () { 469: .done( function ( result ) { 492: .fail( function () { 578: .done( function () { 590: .fail( function ( data, code, response ) { resources/mobile.editor.ve/ve.init.mw.MobileFrontendArticleTarget.js 221: OO.ui.confirm( mw.msg( 'mobile-frontend-editor-switch-confirm' ) ).done( function ( confirmed ) { resources/mobile.scrollEndEventEmitter/ScrollEndEventEmitter.js 47: * this.gateway.getPhotos().done( function ( photos ) { resources/mobile.search/SearchOverlay.js 234: this.router.back().done( function () { resources/mobile.special.mobilediff.scripts/init.js 36: } ).done( function ( data ) { 55: } ).fail( function ( error ) { resources/mobile.special.nearby.scripts/nearby.js 66: loader.using( 'mobile.foreignApi' ).done( function () { resources/mobile.startup/OverlayManager.js 110: factoryResult.done( function ( overlay ) { resources/mobile.startup/rlModuleLoader.js 30: } ).always( function () { resources/mobile.startup/Skin.js 311: .done( function () { 330: gateway.getReferencesList( data.page, id ).done( function ( refListElements ) { 348: .fail( function () { 353: .always( function () { resources/mobile.startup/toast.js 46: this.notification.done( function ( notif ) { resources/mobile.talk.overlays/TalkOverlay.js 110: this.pageGateway.getPage( options.title ).fail( function ( resp ) { 130: } ).done( function ( pageData ) { resources/mobile.talk.overlays/TalkSectionAddOverlay.js 133: this.save().done( function ( status ) { 144: } ).fail( function ( error ) { resources/mobile.talk.overlays/TalkSectionOverlay.js 98: this.pageGateway.getPage( options.title ).done( function ( pageData ) { 135: } ).done( function () { 141: } ).fail( function ( data, response ) { 164: } ).always( function () { resources/mobile.watchstar/Watchstar.js 172: gateway.postStatusesByTitle( [ page.getTitle() ], postWatched ).always( function () { 174: } ).done( function () { 193: } ).fail( function () { tests/qunit/mobile.editor.api/test_EditorGateway.js 55: return gateway.getContent().done( function ( resp ) { 73: return gateway.getContent().done( function ( resp ) { 94: gateway.getContent().done( doneSpy ).fail( function ( error ) { 266: } ).done( doneSpy ).fail( failSpy ).always( function () { 294: gateway.getContent().done( function () { 296: gateway.save().done( doneSpy ).fail( failSpy ).always( function () { 332: gateway.save().done( doneSpy ).fail( failSpy ).always( function () { 363: gateway.save().done( doneSpy ).fail( failSpy ).always( function () { 398: gateway.save().done( doneSpy ).fail( failSpy ).always( function () { 433: gateway.save().done( doneSpy ).fail( failSpy ).always( function () { 466: gateway.save().done( doneSpy ).fail( failSpy ).always( function () { 492: gateway.save().done( doneSpy ).fail( failSpy ).always( function () { 561: } ).done( doneSpy ); 590: } ).done( function ( section ) { 620: } ).done( function ( section ) { 646: gateway.save().always( function () { tests/qunit/mobile.pagelist.scripts/test_WatchstarPageList.js 50: pageList.getPages( [], [] ).done( function () { 80: pl.getPages( [ 30, 50 ], [] ).done( function () { tests/qunit/mobile.references.gateway/test_ReferencesHtmlScraperGateway.js 23: return this.referencesGateway.getReference( '#cite_note-1', this.page ).done( function ( ref ) { 30: this.referencesGateway.getReference( '#cite_note-bad', this.page ).fail( function ( err ) { tests/qunit/mobile.references.gateway/test_ReferencesMobileViewGateway.js 60: return this.referencesGateway.getReference( '#cite_note-1', this.page ).done( function ( ref ) { 78: this.referencesGateway.getReference( '#cite_note-bad', this.page ).fail( function ( err ) { 87: this.referencesGatewayEmpty.getReference( '#cite_note-bad', this.page ).fail( function ( err ) { 96: this.referencesGatewayRejector.getReference( '#cite_note-bad-2', this.page ).fail( function ( err ) { tests/qunit/mobile.startup/test_Skin.js 37: ] ).done( function () { 52: ] ).fail( function () { 55: } ).always( function () { 61: return this.skin.loadImagesList( [] ).done( function () { 73: } ).done( function () { tests/qunit/mobile.startup/test_PageGateway.js 65: return pageGateway.getPage( 'Test' ).done( function ( resp ) { 178: pageGateway.getPage( 'Test' ).done( function ( resp ) { 314: return pageGateway.getPageLanguages( 'Test' ).done( function ( resp ) { 458: pageGateway.getPage( 'Err' ).fail( function ( msg ) { 493: return pageGateway.getPage( 'Test' ).done( function ( resp ) { tests/qunit/mobile.talk.overlays/test_TalkSectionAddOverlay.js 24: overlay.save().done( function ( status ) { 26: } ).fail( function ( error ) {
notes
Careful when replacing .done/.fail, specially .fail, as it behaves differently than .catch.
.fail will keep the promise rejected, but adding a .catch` handler will make the promise resolved unless `you throw inside the catch handler again!
See https://github.com/wikimedia/mediawiki-extensions-Popups/blob/e351fd15500e1ab1eb8a7c2e3b975a0d528ece6c/src/actions.js#L137-L138 for an example in popups (that was just a .fail before)
It would be amazing if the owner of this task generates a migration checklist that we can refer to whenever making these changes in other repos.
Migration Checklist
- For consistency, prefer to use .then( successFn, failFn ) when replacing .done( successFn ).fail( failFn ) or .done( successFn ).catch( failFn ). Be aware of the return value in your callbacks though!
- In general, .done( function () { ... } ).fail( function () { ... } ).always( alwaysFn ) can be replaced with .then( function () { ... alwaysFn() }, function () { ... alwaysFn() } )
- Be aware of Deferred handlers with multiple parameters. A Deferred can resolve with multiple arguments passed to its callback functions while a ES6 promise can only resolve with only one argument.
- When changing the return value of a function, keep in mind that $.Deferred().done() returns a JQuery.Deferred, but $.Deferred().then() returns a JQuery.Promise so you may have to update the JSDocs.
- You may need to convert tests from sync to async if the tests cover code that previously relied on .done/.fail. This is because .done/.fail callbacks may be called synchronously if the Deferred has already been resolved (which is often the case with test stubs), but .then/.catch callbacks are always called asynchronously. See https://jquery.com/upgrade-guide/3.0/#backwards-compatibility for more info and https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/commit/d42523e609082c300081149f5b9dde1ad6b4ced1#diff-057379d6c79b6705c76c0090f5685011 for an example
Remaining Patches to be Merged
- https://gerrit.wikimedia.org/r/453451
- https://gerrit.wikimedia.org/r/453461
- https://gerrit.wikimedia.org/r/453465
- https://gerrit.wikimedia.org/r/453466
- https://gerrit.wikimedia.org/r/453475
- https://gerrit.wikimedia.org/r/453478
- https://gerrit.wikimedia.org/r/454441
- https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/454439/1