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 [[ https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/MobileFrontend/+/74aefb1610d8edbeb22e63a86c39f1a626222544%5E%21/ | 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:
```lines=5
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.
**Remaining Patches to be Merged**
[x] https://gerrit.wikimedia.org/r/453451
[x] https://gerrit.wikimedia.org/r/453461
[x] https://gerrit.wikimedia.org/r/453465
[x] https://gerrit.wikimedia.org/r/453466
[x] https://gerrit.wikimedia.org/r/453475
[] https://gerrit.wikimedia.org/r/453478
[] https://gerrit.wikimedia.org/r/454441