diff --git a/resources/dist/index.js b/resources/dist/index.js index 441987179..60ba86e47 100644 Binary files a/resources/dist/index.js and b/resources/dist/index.js differ diff --git a/resources/dist/index.js.json b/resources/dist/index.js.json index 6e7afaa16..f2df52d0e 100644 Binary files a/resources/dist/index.js.json and b/resources/dist/index.js.json differ diff --git a/src/actions.js b/src/actions.js index 60c7b1b6c..a92740af3 100644 --- a/src/actions.js +++ b/src/actions.js @@ -142,7 +142,7 @@ export function fetch( gateway, title, el, token ) { token: token } ); } ) - .fail( function ( data, result ) { + .catch( function ( data, result ) { // All failures, except those due to being offline or network error, // should present "There was an issue displaying this preview". // e.g.: @@ -213,7 +213,7 @@ export function linkDwell( title, el, event, gateway, generateToken ) { var previewState = getState().preview; if ( previewState.enabled && isNewInteraction() ) { - dispatch( fetch( gateway, title, el, token ) ); + return dispatch( fetch( gateway, title, el, token ) ); } } ); }; diff --git a/tests/node-qunit/actions.test.js b/tests/node-qunit/actions.test.js index 4ffee7312..138e1ac13 100644 --- a/tests/node-qunit/actions.test.js +++ b/tests/node-qunit/actions.test.js @@ -67,19 +67,10 @@ QUnit.test( '#boot', function ( assert ) { * @param {Object} module */ function setupWait( module ) { - module.waitDeferreds = []; - module.waitPromises = []; - - module.wait = module.sandbox.spy( function () { - var deferred = $.Deferred(), - promise = deferred.promise(); - - module.waitDeferreds.push( deferred ); - module.waitPromises.push( promise ); - return promise; + module.waitPromise = $.Deferred().resolve().promise(); + module.wait = module.sandbox.stub( WaitModule, 'default' ).callsFake( function () { + return module.waitPromise; } ); - - module.sandbox.stub( WaitModule, 'default' ).callsFake( module.wait ); } /** @@ -95,14 +86,10 @@ function setupEl( module ) { QUnit.module( 'ext.popups/actions#linkDwell @integration', { beforeEach: function () { - var that = this; - this.state = { preview: {} }; - this.getState = function () { - return that.state; - }; + this.getState = () => this.state; // The worst-case implementation of mw.now. mw.now = function () { return Date.now(); }; @@ -112,10 +99,12 @@ QUnit.module( 'ext.popups/actions#linkDwell @integration', { } ); QUnit.test( '#linkDwell', function ( assert ) { - var p, + var linkDwelled, event = {}, dispatch = this.sandbox.spy(); + assert.expect( 2 ); + this.sandbox.stub( mw, 'now' ).returns( new Date() ); this.sandbox.stub( actions, 'fetch' ); @@ -124,7 +113,7 @@ QUnit.test( '#linkDwell', function ( assert ) { activeToken: generateToken() }; - p = actions.linkDwell( + linkDwelled = actions.linkDwell( this.title, this.el, event, /* gateway = */ null, generateToken )( dispatch, @@ -150,21 +139,22 @@ QUnit.test( '#linkDwell', function ( assert ) { // --- - p.then( function () { + return linkDwelled.then( function () { assert.strictEqual( dispatch.callCount, 2, 'The fetch action is dispatched after FETCH_COMPLETE milliseconds.' ); } ); - return p; } ); QUnit.test( '#linkDwell doesn\'t continue when previews are disabled', function ( assert ) { - var p, + var linkDwelled, event = {}, dispatch = this.sandbox.spy(); + assert.expect( 2 ); + // Stub the state tree being updated by the LINK_DWELL action. this.state.preview = { enabled: false, @@ -172,7 +162,7 @@ QUnit.test( '#linkDwell doesn\'t continue when previews are disabled', function activeToken: generateToken() }; - p = actions.linkDwell( + linkDwelled = actions.linkDwell( this.title, this.el, event, /* gateway = */ null, generateToken )( dispatch, @@ -181,18 +171,18 @@ QUnit.test( '#linkDwell doesn\'t continue when previews are disabled', function assert.strictEqual( dispatch.callCount, 1 ); - p.then( function () { + return linkDwelled.then( function () { assert.strictEqual( dispatch.callCount, 1 ); } ); - - return p; } ); QUnit.test( '#linkDwell doesn\'t continue if the token has changed', function ( assert ) { - var p, + var linkDwelled, event = {}, dispatch = this.sandbox.spy(); + assert.expect( 1 ); + // Stub the state tree being updated by a LINK_DWELL action. this.state.preview = { enabled: true, @@ -200,7 +190,7 @@ QUnit.test( '#linkDwell doesn\'t continue if the token has changed', function ( activeToken: generateToken() }; - p = actions.linkDwell( + linkDwelled = actions.linkDwell( this.title, this.el, event, /* gateway = */ null, generateToken )( dispatch, @@ -219,24 +209,23 @@ QUnit.test( '#linkDwell doesn\'t continue if the token has changed', function ( activeToken: 'banana' }; - p.then( function () { + return linkDwelled.then( function () { assert.strictEqual( dispatch.callCount, 1 ); } ); - - return p; } ); QUnit.test( '#linkDwell dispatches the fetch action', function ( assert ) { - var p, - event = {}, + var event = {}, dispatch = this.sandbox.spy(); + assert.expect( 1 ); + this.state.preview = { enabled: true, activeToken: generateToken() }; - p = actions.linkDwell( + return actions.linkDwell( this.title, this.el, event, /* gateway = */ null, generateToken )( dispatch, @@ -244,28 +233,22 @@ QUnit.test( '#linkDwell dispatches the fetch action', function ( assert ) { ).then( function () { assert.strictEqual( dispatch.callCount, 2 ); } ); - - return p; } ); QUnit.module( 'ext.popups/actions#fetch', { beforeEach: function () { - var that = this; - - // Setup the mw.now stub before actions is re-required in setupWait this.now = 0; - this.sandbox.stub( mw, 'now' ).callsFake( function () { - return that.now; - } ); + this.sandbox.stub( mw, 'now' ).callsFake( () => this.now ); setupWait( this ); setupEl( this ); this.gatewayDeferred = $.Deferred(); - this.gatewayPromise = this.gatewayDeferred.promise(); this.gateway = { - getPageSummary: this.sandbox.stub().returns( this.gatewayPromise ) + getPageSummary: this.sandbox.stub().returns( + this.gatewayDeferred.promise() + ) }; this.dispatch = this.sandbox.spy(); @@ -273,15 +256,17 @@ QUnit.module( 'ext.popups/actions#fetch', { this.token = '1234567890'; // Sugar. - this.fetch = function () { + this.fetch = () => { return actions.fetch( - that.gateway, that.title, that.el, that.token - )( that.dispatch ); + this.gateway, this.title, this.el, this.token + )( this.dispatch ); }; } } ); QUnit.test( 'it should fetch data from the gateway immediately', function ( assert ) { + assert.expect( 3 ); + this.fetch(); assert.ok( this.gateway.getPageSummary.calledWith( 'Foo' ) ); @@ -301,19 +286,21 @@ QUnit.test( 'it should fetch data from the gateway immediately', function ( asse } ); QUnit.test( 'it should dispatch the FETCH_END action when the API request ends', function ( assert ) { - var that = this; + var fetched; - this.fetch(); + assert.expect( 1 ); + + fetched = this.fetch(); this.now += 115; this.gatewayDeferred.resolve( {} ); - return this.gatewayPromise.then( function () { + return fetched.then( () => { assert.deepEqual( - that.dispatch.getCall( 1 ).args[ 0 ], + this.dispatch.getCall( 1 ).args[ 0 ], { type: 'FETCH_END', - el: that.el, + el: this.el, timestamp: 115 } ); @@ -321,112 +308,82 @@ QUnit.test( 'it should dispatch the FETCH_END action when the API request ends', } ); QUnit.test( 'it should delay dispatching the FETCH_COMPLETE action', function ( assert ) { - var whenDeferred = $.Deferred(), - whenSpy, - args, - result = {}, - that = this; + var result = {}, + fetched = this.fetch(); - whenSpy = this.sandbox.stub( $, 'when' ) - .returns( whenDeferred.promise() ); - - this.fetch(); + assert.expect( 2 ); assert.strictEqual( this.wait.getCall( 0 ).args[ 0 ], 350, 'It waits for FETCH_COMPLETE_TARGET_DELAY - FETCH_START_DELAY milliseconds.' ); + this.gatewayDeferred.resolve( result ); - // --- - args = whenSpy.getCall( 0 ).args; - - // This assertion is disabled due to $.Promise#then and #fail returning a new - // instance of $.Promise. - // assert.strictEqual( args[ 0 ], this.gatewayPromise ); - - assert.strictEqual( args[ 1 ], this.waitPromises[ 0 ] ); - - // --- - whenDeferred.resolve( result ); - - return whenDeferred.then( function () { - - // Ensure the following assertions are made after all callbacks have been - // executed. Use setTimeout( _, 0 ) since it's not critical that these - // assertions are run before I/O is processed, i.e. we don't require - // process.nextTick. - setTimeout( function () { - assert.deepEqual( - that.dispatch.getCall( 1 ).args[ 0 ], - { - type: 'FETCH_COMPLETE', - el: that.el, - result: result, - token: that.token - } - ); - }, 0 ); + return fetched.then( () => { + assert.deepEqual( + this.dispatch.getCall( 2 ).args[ 0 ], + { + type: 'FETCH_COMPLETE', + el: this.el, + result: result, + token: this.token + } + ); } ); } ); QUnit.test( 'it should dispatch the FETCH_FAILED action when the request fails', function ( assert ) { - var that = this; + var fetched = this.fetch(); assert.expect( 2 ); this.gatewayDeferred.reject( new Error( 'API req failed' ) ); - const fetch = this.fetch().catch( function () { + this.now += 115; + + return fetched.then( () => { assert.equal( - that.dispatch.callCount, 3, + this.dispatch.callCount, 3, 'dispatch called thrice, START, FAILED, and COMPLETE' ); assert.deepEqual( - that.dispatch.getCall( 1 ).args[ 0 ], + this.dispatch.getCall( 1 ).args[ 0 ], { type: 'FETCH_FAILED', - el: that.el + el: this.el } ); } ); - - this.now += 115; - - return fetch; } ); QUnit.test( 'it should dispatch the FETCH_FAILED action when the request fails even after the wait timeout', function ( assert ) { - var that = this; + var fetched = this.fetch(); assert.expect( 2 ); - const fetch = this.fetch().catch( function () { + // After the wait interval happens, resolve the gateway request + return this.waitPromise.then( () => { + this.gatewayDeferred.reject( new Error( 'API req failed' ) ); + return fetched; + } ).then( () => { assert.equal( - that.dispatch.callCount, 3, + this.dispatch.callCount, 3, 'dispatch called thrice, START, FAILED, and COMPLETE' ); assert.deepEqual( - that.dispatch.getCall( 1 ).args[ 0 ], + this.dispatch.getCall( 1 ).args[ 0 ], { type: 'FETCH_FAILED', - el: that.el + el: this.el } ); } ); - - // After the wait interval happens, resolve the gateway request - this.waitPromises[ 0 ].then( function () { - that.gatewayDeferred.reject( new Error( 'API req failed' ) ); - } ); - - this.waitDeferreds[ 0 ].resolve(); - - return fetch; } ); QUnit.module( 'ext.popups/actions#abandon', { beforeEach: function () { + setupWait( this ); setupEl( this ); } } ); @@ -440,13 +397,13 @@ QUnit.test( 'it should dispatch start and end actions', function ( assert ) { activeToken: token } }; - }, p; + }, abandoned; + + assert.expect( 3 ); this.sandbox.stub( mw, 'now' ).returns( new Date() ); - setupWait( this ); - - actions.abandon( this.el )( dispatch, getState ); + abandoned = actions.abandon( this.el )( dispatch, getState ); assert.ok( dispatch.calledWith( { type: 'ABANDON_START', @@ -461,7 +418,7 @@ QUnit.test( 'it should dispatch start and end actions', function ( assert ) { 'Have you spoken with #Design about changing this value?' ); - p = this.waitPromises[ 0 ].then( function () { + return abandoned.then( function () { assert.ok( dispatch.calledWith( { type: 'ABANDON_END', @@ -470,10 +427,6 @@ QUnit.test( 'it should dispatch start and end actions', function ( assert ) { 'ABANDON_* share the same token.' ); } ); - - // After ABANDON_END_DELAY milliseconds... - this.waitDeferreds[ 0 ].resolve(); - return p; } ); QUnit.test( 'it shouldn\'t dispatch under certain conditions', function ( assert ) { diff --git a/tests/node-qunit/container.test.js b/tests/node-qunit/container.test.js index 05cc0e34e..b1e3684ab 100644 --- a/tests/node-qunit/container.test.js +++ b/tests/node-qunit/container.test.js @@ -14,8 +14,7 @@ QUnit.test( '#has', function ( assert ) { } ); QUnit.test( '#get', function ( assert ) { - var service = {}, - that = this; + var service = {}; this.factory.returns( service ); @@ -39,9 +38,7 @@ QUnit.test( '#get', function ( assert ) { // --- assert.throws( - function () { - that.container.get( 'bar' ); - }, + () => { this.container.get( 'bar' ); }, /The service "bar" hasn't been defined./ ); } ); diff --git a/tests/node-qunit/integration.test.js b/tests/node-qunit/integration.test.js index 1017abca7..008746ac5 100644 --- a/tests/node-qunit/integration.test.js +++ b/tests/node-qunit/integration.test.js @@ -8,9 +8,6 @@ import registerChangeListener from '../../src/changeListener'; var mw = mediaWiki, $ = jQuery, - // Store the real wait to be actually used in tests - wait = WaitModule.default, - /** * Whether Gateway#getPageSummary is resolved or rejected. * @enum {number} @@ -56,15 +53,13 @@ function constant( x ) { return function () { return x; }; } QUnit.module( 'ext.popups preview @integration', { beforeEach: function () { - var that = this; - // The worst-case implementation of mw.now. mw.now = function () { return Date.now(); }; - this.resetWait = function () { - that.waitDeferred = $.Deferred(); - that.waitPromise = that.waitDeferred.promise(); - that.wait.returns( that.waitPromise ); + this.resetWait = () => { + this.waitDeferred = $.Deferred(); + this.waitPromise = this.waitDeferred.promise(); + this.wait.returns( this.waitPromise ); }; this.wait = this.sandbox.stub( WaitModule, 'default' ); @@ -80,8 +75,8 @@ QUnit.module( 'ext.popups preview @integration', { this.store.dispatch ); - this.registerChangeListener = function ( fn ) { - return registerChangeListener( that.store, fn ); + this.registerChangeListener = ( fn ) => { + return registerChangeListener( this.store, fn ); }; this.title = stubs.createStubTitle( 1, 'Foo' ); @@ -102,71 +97,55 @@ QUnit.module( 'ext.popups preview @integration', { { get: identity } ); - this.dwell = function ( + this.dwell = ( title, el, ev, fetchResponse, resolution = FETCH_RESOLUTION.RESOLVE - ) { - that.resetWait(); - that.actions.linkDwell( title, el, ev, { + ) => { + this.resetWait(); + return this.actions.linkDwell( title, el, ev, { getPageSummary: function () { var method = resolution === FETCH_RESOLUTION.RESOLVE ? 'resolve' : 'reject'; return $.Deferred()[ method ]( fetchResponse ).promise(); } }, function () { return 'pagetoken'; } ); - return that.waitPromise; }; - this.dwellAndShowPreview = function ( + this.dwellAndShowPreview = ( title, el, ev, fetchResponse, reject = FETCH_RESOLUTION.RESOLVE - ) { - that.dwell( title, el, ev, fetchResponse, reject ); - that.waitDeferred.resolve(); - - // Wait for the next tick to resolve pending callbacks. N.B. that the - // fetch action invokes wait twice. - return wait( 0 ) - .then( function () { - that.waitDeferred.resolve(); - - return that.waitPromise; - } ); + ) => { + var dwelled = this.dwell( title, el, ev, fetchResponse, reject ); + // Resolve the wait timeout for the linkDwell and the fetch action + this.waitDeferred.resolve(); + return dwelled; }; - this.abandon = function () { - that.resetWait(); - that.actions.abandon(); - return that.waitPromise; + this.abandon = () => { + this.resetWait(); + return this.actions.abandon(); }; - this.abandonAndWait = function () { - that.abandon(); - that.waitDeferred.resolve(); - return wait( 0 ); // Wait for next tick to resolve pending callbacks + this.abandonAndWait = () => { + var abandoned = this.abandon(); + this.waitDeferred.resolve(); + return abandoned; }; - this.dwellAndPreviewDwell = function ( title, el, ev, res ) { - return that.dwellAndShowPreview( title, el, ev, res ).then( function () { + this.dwellAndPreviewDwell = ( title, el, ev, res ) => { + return this.dwellAndShowPreview( title, el, ev, res ).then( () => { // Get out of the link, and before the delay ends... - var abandonPromise = that.abandon( el ), - abandonDeferred = that.waitDeferred; + var abandonPromise = this.abandon(), + abandonWaitDeferred = this.waitDeferred; // Dwell over the preview - that.actions.previewDwell( el ); + this.actions.previewDwell( el ); // Then the abandon delay finishes - abandonDeferred.resolve(); + abandonWaitDeferred.resolve(); return abandonPromise; } ); }; - - this.abandonPreview = function () { - that.resetWait(); - that.actions.abandon(); - - return that.waitPromise; - }; } } ); @@ -224,56 +203,54 @@ QUnit.test( 'in ACTIVE state, fetch fail switches it to DATA', function ( assert } ); QUnit.test( 'in ACTIVE state, abandon start, and then end, switch it to INACTIVE', function ( assert ) { - var that = this, - el = this.el; + var el = this.el; return this.dwellAndShowPreview( this.title, el, 'event', 42 ) - .then( function () { - return that.abandonAndWait( el ); - } ).then( function () { - var state = that.store.getState(); + .then( () => { + return this.abandonAndWait( el ); + } ).then( () => { + var state = this.store.getState(); assert.equal( state.preview.activeLink, undefined, 'After abandoning, preview is back to INACTIVE' ); } ); } ); QUnit.test( 'in ACTIVE state, abandon link, and then dwell preview, should keep it active after all delays', function ( assert ) { - var that = this, - el = this.el; + var el = this.el; return this.dwellAndPreviewDwell( this.title, el, 'event', 42 ) - .then( function () { - var state = that.store.getState(); + .then( () => { + var state = this.store.getState(); assert.equal( state.preview.activeLink, el ); } ); } ); QUnit.test( 'in ACTIVE state, abandon link, hover preview, back to link, should keep it active after all delays', function ( assert ) { - var that = this, - el = this.el; + var el = this.el; + // Dwell link, abandon it & hover preview return this.dwellAndPreviewDwell( this.title, el, 'event', 42 ) - .then( function () { - var abandonPreviewDeferred, dwellPromise, dwellDeferred; + .then( () => { + var abandonedPreview, abandonWaitDeferred, dwelled, dwellWaitDeferred; // Start abandoning the preview - that.abandonPreview( el ); + abandonedPreview = this.abandon(); + abandonWaitDeferred = this.waitDeferred; - abandonPreviewDeferred = that.waitDeferred; - // Dwell back into the link, new event is triggered - dwellPromise = that.dwell( that.title, el, 'event2', 42 ); - dwellDeferred = that.waitDeferred; + // Dwell back into the link, new event ('event2') is triggered + dwelled = this.dwell( this.title, el, 'event2', 42 ); + dwellWaitDeferred = this.waitDeferred; // Preview abandon happens next, before the fetch - abandonPreviewDeferred.resolve(); + abandonWaitDeferred.resolve(); - // Then fetch happens - dwellDeferred.resolve(); + // Then dwell wait & fetch happens + dwellWaitDeferred.resolve(); - return dwellPromise; + return $.when( abandonedPreview, dwelled ); } ) - .then( function () { - var state = that.store.getState(); + .then( () => { + var state = this.store.getState(); assert.equal( state.preview.activeLink, el ); } ); } ); diff --git a/tests/node-qunit/previewBehavior.test.js b/tests/node-qunit/previewBehavior.test.js index 68bd63023..635cad8ee 100644 --- a/tests/node-qunit/previewBehavior.test.js +++ b/tests/node-qunit/previewBehavior.test.js @@ -16,8 +16,7 @@ QUnit.module( 'ext.popups.preview.settingsBehavior', { } ); QUnit.test( 'it should set the settingsUrl on wgPopupsBetaFeature', function ( assert ) { - var that = this, - user = createStubUser( /* isAnon = */ false ), + var user = createStubUser( /* isAnon = */ false ), actions = {}, cases; @@ -26,12 +25,12 @@ QUnit.test( 'it should set the settingsUrl on wgPopupsBetaFeature', function ( a [ false, 'Special:Preferences#mw-prefsection-rendering' ] ]; - $.each( cases, function ( i, testCase ) { + cases.forEach( ( testCase ) => { var behavior; - that.config.set( 'wgPopupsBetaFeature', testCase[ 0 ] ); + this.config.set( 'wgPopupsBetaFeature', testCase[ 0 ] ); - behavior = createPreviewBehavior( that.config, user, actions ); + behavior = createPreviewBehavior( this.config, user, actions ); assert.deepEqual( behavior.settingsUrl,