diff --git a/docs/change_listener.md b/docs/change_listener.md index 215dc823c..7fa373eca 100644 --- a/docs/change_listener.md +++ b/docs/change_listener.md @@ -24,7 +24,7 @@ mw.popups.changeListeners.foo = function ( boundActions ) { .attr( 'href', '#' ) .click( boundActions.showSettings ); - return function ( prevState, state ) { + return function ( oldState, newState ) { // ... } }; diff --git a/resources/dist/index.js b/resources/dist/index.js index 4fcc05c77..5631ac302 100644 Binary files a/resources/dist/index.js and b/resources/dist/index.js differ diff --git a/resources/dist/index.js.map.json b/resources/dist/index.js.map.json index 26b80c6ec..0f056563e 100644 Binary files a/resources/dist/index.js.map.json and b/resources/dist/index.js.map.json differ diff --git a/src/changeListener.js b/src/changeListener.js index 9a4627737..6a89cb33f 100644 --- a/src/changeListener.js +++ b/src/changeListener.js @@ -4,8 +4,8 @@ /** * @typedef {Function} ext.popups.ChangeListener - * @param {Object} prevState The previous state - * @param {Object} state The current state + * @param {Object} oldState The previous state + * @param {Object} newState The current state */ /** @@ -29,15 +29,14 @@ export default function registerChangeListener( store, callback ) { // Store#subscribe](http://redux.js.org/docs/api/Store.html#subscribe), // which was written by Dan Abramov. - let state; + let previousState; store.subscribe( () => { - const prevState = state; + const state = store.getState(); - state = store.getState(); - - if ( prevState !== state ) { - callback( prevState, state ); + if ( previousState !== state ) { + callback( previousState, state ); + previousState = state; } } ); } diff --git a/src/changeListeners/eventLogging.js b/src/changeListeners/eventLogging.js index 1e63f2e9f..ff691425c 100644 --- a/src/changeListeners/eventLogging.js +++ b/src/changeListeners/eventLogging.js @@ -18,8 +18,8 @@ export default function eventLogging( boundActions, eventLoggingTracker, getCurrentTimestamp ) { - return ( _, state ) => { - const eventLoggingObj = state.eventLogging; + return ( oldState, newState ) => { + const eventLoggingObj = newState.eventLogging; let event = eventLoggingObj.event; if ( !event ) { diff --git a/src/changeListeners/footerLink.js b/src/changeListeners/footerLink.js index 2cbd7f221..193fe24bc 100644 --- a/src/changeListeners/footerLink.js +++ b/src/changeListeners/footerLink.js @@ -57,7 +57,7 @@ function createFooterLink() { export default function footerLink( boundActions ) { let $footerLink; - return ( prevState, state ) => { + return ( oldState, newState ) => { if ( $footerLink === undefined ) { $footerLink = createFooterLink(); $footerLink.on( 'click', ( e ) => { @@ -66,7 +66,7 @@ export default function footerLink( boundActions ) { } ); } - if ( state.settings.shouldShowFooterLink ) { + if ( newState.settings.shouldShowFooterLink ) { $footerLink.show(); } else { $footerLink.hide(); diff --git a/src/changeListeners/linkTitle.js b/src/changeListeners/linkTitle.js index ed955e8b8..915780729 100644 --- a/src/changeListeners/linkTitle.js +++ b/src/changeListeners/linkTitle.js @@ -43,10 +43,10 @@ export default function linkTitle() { title = undefined; } - return ( prevState, state ) => { - const hasPrevActiveLink = prevState && prevState.preview.activeLink; + return ( oldState, newState ) => { + const hasPrevActiveLink = oldState && oldState.preview.activeLink; - if ( !state.preview.enabled ) { + if ( !newState.preview.enabled ) { return; } @@ -54,13 +54,13 @@ export default function linkTitle() { // Has the user dwelled on a link immediately after abandoning another // (remembering that the ABANDON_END action is delayed by // ~100 ms). - if ( prevState.preview.activeLink !== state.preview.activeLink ) { - restoreTitleAttr( prevState.preview.activeLink ); + if ( oldState.preview.activeLink !== newState.preview.activeLink ) { + restoreTitleAttr( oldState.preview.activeLink ); } } - if ( state.preview.activeLink ) { - destroyTitleAttr( state.preview.activeLink ); + if ( newState.preview.activeLink ) { + destroyTitleAttr( newState.preview.activeLink ); } }; } diff --git a/src/changeListeners/pageviews.js b/src/changeListeners/pageviews.js index 8682f3a73..a0e799a84 100644 --- a/src/changeListeners/pageviews.js +++ b/src/changeListeners/pageviews.js @@ -16,10 +16,10 @@ export default function pageviews( boundActions, pageviewTracker ) { - return ( _, state ) => { + return ( oldState, newState ) => { let page; - if ( state.pageviews && state.pageviews.pageview && state.pageviews.page ) { - page = state.pageviews.page; + if ( newState.pageviews && newState.pageviews.pageview && newState.pageviews.page ) { + page = newState.pageviews.page; pageviewTracker( 'event.VirtualPageView', $.extend( {}, { /* eslint-disable camelcase */ @@ -29,7 +29,7 @@ export default function pageviews( source_url: page.url /* eslint-enable camelcase */ }, - state.pageviews.pageview ) + newState.pageviews.pageview ) ); // Clear the pageview now its been logged. boundActions.pageviewLogged(); diff --git a/src/changeListeners/render.js b/src/changeListeners/render.js index 1d47304c6..d803f29f6 100644 --- a/src/changeListeners/render.js +++ b/src/changeListeners/render.js @@ -13,15 +13,15 @@ import * as renderer from '../ui/renderer'; export default function render( previewBehavior ) { let preview; - return ( prevState, state ) => { - if ( state.preview.shouldShow && !preview ) { - preview = renderer.render( state.preview.fetchResponse ); + return ( oldState, newState ) => { + if ( newState.preview.shouldShow && !preview ) { + preview = renderer.render( newState.preview.fetchResponse ); preview.show( - state.preview.measures, + newState.preview.measures, previewBehavior, - state.preview.activeToken + newState.preview.activeToken ); - } else if ( !state.preview.shouldShow && preview ) { + } else if ( !newState.preview.shouldShow && preview ) { preview.hide(); preview = undefined; } diff --git a/src/changeListeners/settings.js b/src/changeListeners/settings.js index cab4ff042..1eec9dc03 100644 --- a/src/changeListeners/settings.js +++ b/src/changeListeners/settings.js @@ -8,16 +8,16 @@ export default function settings( boundActions, render ) { let settingsObj; - return ( prevState, state ) => { - if ( !prevState ) { + return ( oldState, newState ) => { + if ( !oldState ) { // Nothing to do on initialization return; } // Update global modal visibility if ( - prevState.settings.shouldShow === false && - state.settings.shouldShow === true + oldState.settings.shouldShow === false && + newState.settings.shouldShow === true ) { // Lazily instantiate the settings UI if ( !settingsObj ) { @@ -26,19 +26,19 @@ export default function settings( boundActions, render ) { } // Update the UI settings with the current settings - settingsObj.setEnabled( state.preview.enabled ); + settingsObj.setEnabled( newState.preview.enabled ); settingsObj.show(); } else if ( - prevState.settings.shouldShow === true && - state.settings.shouldShow === false + oldState.settings.shouldShow === true && + newState.settings.shouldShow === false ) { settingsObj.hide(); } // Update help visibility - if ( prevState.settings.showHelp !== state.settings.showHelp ) { - settingsObj.toggleHelp( state.settings.showHelp ); + if ( oldState.settings.showHelp !== newState.settings.showHelp ) { + settingsObj.toggleHelp( newState.settings.showHelp ); } }; } diff --git a/src/changeListeners/statsv.js b/src/changeListeners/statsv.js index 409ec89dd..8a91064c3 100644 --- a/src/changeListeners/statsv.js +++ b/src/changeListeners/statsv.js @@ -11,8 +11,8 @@ * @return {ext.popups.ChangeListener} */ export default function statsv( boundActions, track ) { - return ( _, state ) => { - const statsvObj = state.statsv; + return ( oldState, newState ) => { + const statsvObj = newState.statsv; if ( statsvObj.action ) { track( statsvObj.action, statsvObj.data ); diff --git a/src/changeListeners/syncUserSettings.js b/src/changeListeners/syncUserSettings.js index 83c6d20c2..98f16896a 100644 --- a/src/changeListeners/syncUserSettings.js +++ b/src/changeListeners/syncUserSettings.js @@ -21,19 +21,19 @@ import { previewTypes } from '../preview/model'; * @return {ext.popups.ChangeListener} */ export default function syncUserSettings( userSettings ) { - return ( prevState, state ) => { + return ( oldState, newState ) => { syncIfChanged( - prevState, state, 'eventLogging.previewCount', + oldState, newState, 'eventLogging.previewCount', userSettings.storePreviewCount ); syncIfChanged( - prevState, state, 'preview.enabled', + oldState, newState, 'preview.enabled', userSettings.storePagePreviewsEnabled ); syncIfChanged( // TODO: This property currently doesn't exist in the state, see reducers/preview.js // TODO: This is currently not covered by a test case, see syncUserSettings.test.js - prevState, state, 'preview.enabled.' + previewTypes.TYPE_REFERENCE, + oldState, newState, 'preview.enabled.' + previewTypes.TYPE_REFERENCE, userSettings.storeReferencePreviewsEnabled ); }; @@ -57,16 +57,16 @@ function get( state, path ) { * Calls a sync function if the property prop on the property reducer on * the state trees has changed value. * - * @param {Object} prevState - * @param {Object} state + * @param {Object} oldState + * @param {Object} newState * @param {string} path dot-separated path in the state tree * @param {Function} sync function to be called with the newest value if * changed * @return {void} */ -function syncIfChanged( prevState, state, path, sync ) { - const current = get( state, path ); - if ( prevState && ( get( prevState, path ) !== current ) ) { +function syncIfChanged( oldState, newState, path, sync ) { + const current = get( newState, path ); + if ( oldState && ( get( oldState, path ) !== current ) ) { sync( current ); } } diff --git a/src/reducers/nextState.js b/src/reducers/nextState.js index 195c09f17..c55285f13 100644 --- a/src/reducers/nextState.js +++ b/src/reducers/nextState.js @@ -9,7 +9,7 @@ * hasOwnProperty to copy over to the new state. * * In [change listeners](/docs/change_listener.md), for example, we talk about - * the previous state and the current state (the `prevState` and `state` + * the previous state and the current state (the `oldState` and `newState` * parameters, respectively). Since * [reducers](http://redux.js.org/docs/basics/Reducers.html) take the current * state and an action and make updates, "next state" seems appropriate. diff --git a/tests/node-qunit/changeListeners/eventLogging.test.js b/tests/node-qunit/changeListeners/eventLogging.test.js index f66856ae5..68ced0125 100644 --- a/tests/node-qunit/changeListeners/eventLogging.test.js +++ b/tests/node-qunit/changeListeners/eventLogging.test.js @@ -34,11 +34,11 @@ QUnit.test( 'it should log the queued event', function ( assert ) { baz: 'qux' }; - const state = createState( baseData, { + const newState = createState( baseData, { action: 'pageLoaded' } ); - this.changeListener( undefined, state ); + this.changeListener( undefined, newState ); assert.ok( this.eventLoggingTracker.calledWith( @@ -55,20 +55,20 @@ QUnit.test( 'it should log the queued event', function ( assert ) { } ); QUnit.test( 'it should call the eventLogged bound action creator', function ( assert ) { - const state = createState( {}, undefined ); + const newState = createState( {}, undefined ); - this.changeListener( undefined, state ); + this.changeListener( undefined, newState ); assert.notOk( this.boundActions.eventLogged.called, 'It shouldn\'t call the eventLogged bound action creator if there\'s no queued event.' ); - state.eventLogging.event = { + newState.eventLogging.event = { action: 'pageLoaded' }; - this.changeListener( undefined, state ); + this.changeListener( undefined, newState ); assert.ok( this.boundActions.eventLogged.called, diff --git a/tests/node-qunit/changeListeners/footerLink.test.js b/tests/node-qunit/changeListeners/footerLink.test.js index f811100ce..e67354241 100644 --- a/tests/node-qunit/changeListeners/footerLink.test.js +++ b/tests/node-qunit/changeListeners/footerLink.test.js @@ -65,10 +65,10 @@ QUnit.test( 'it should show and hide the link', function ( assert ) { // --- - const prevState = $.extend( true, {}, this.state ); + const oldState = $.extend( true, {}, this.state ); this.state.settings.shouldShowFooterLink = false; - this.footerLinkChangeListener( prevState, this.state ); + this.footerLinkChangeListener( oldState, this.state ); assert.strictEqual( $link.css( 'display' ), diff --git a/tests/node-qunit/changeListeners/pageviews.test.js b/tests/node-qunit/changeListeners/pageviews.test.js index 4fd0068b9..b3661b89c 100644 --- a/tests/node-qunit/changeListeners/pageviews.test.js +++ b/tests/node-qunit/changeListeners/pageviews.test.js @@ -38,9 +38,9 @@ function createState( title ) { } QUnit.test( 'it should log the queued event', function ( assert ) { - const state = createState( 'Rainbows' ); + const newState = createState( 'Rainbows' ); - this.changeListener( undefined, state ); + this.changeListener( undefined, newState ); assert.ok( this.pageviewTracker.calledWith( @@ -64,9 +64,9 @@ QUnit.test( 'it should log the queued event', function ( assert ) { } ); QUnit.test( 'it should not log something that is not a pageview', function ( assert ) { - const state = createState(); + const newState = createState(); - this.changeListener( undefined, state ); + this.changeListener( undefined, newState ); assert.notOk( this.pageviewTracker.called, diff --git a/tests/node-qunit/changeListeners/render.test.js b/tests/node-qunit/changeListeners/render.test.js index c4aa2be64..97cdbed4c 100644 --- a/tests/node-qunit/changeListeners/render.test.js +++ b/tests/node-qunit/changeListeners/render.test.js @@ -18,7 +18,7 @@ QUnit.test( function ( assert ) { const previewBehavior = {}; - const state = { + const newState = { preview: { shouldShow: true, measures: {}, @@ -27,13 +27,13 @@ QUnit.test( }; const changeListener = render( previewBehavior ); - changeListener( undefined, state ); + changeListener( undefined, newState ); assert.ok( this.preview.show.calledWith( - state.preview.measures, + newState.preview.measures, previewBehavior, - state.preview.activeToken + newState.preview.activeToken ), 'The preview is shown with correct arguments.' ); @@ -41,7 +41,7 @@ QUnit.test( ); QUnit.test( 'it should render the preview', ( assert ) => { - const state = { + const newState = { preview: { shouldShow: true, fetchResponse: {} @@ -49,10 +49,10 @@ QUnit.test( 'it should render the preview', ( assert ) => { }; const changeListener = render( /* previewBehavior = undefined */ ); - changeListener( undefined, state ); + changeListener( undefined, newState ); assert.ok( - RendererModule.render.calledWith( state.preview.fetchResponse ), + RendererModule.render.calledWith( newState.preview.fetchResponse ), 'It should use the data from the gateway.' ); } ); diff --git a/tests/node-qunit/changeListeners/statsv.test.js b/tests/node-qunit/changeListeners/statsv.test.js index 9445051ae..aadfeba83 100644 --- a/tests/node-qunit/changeListeners/statsv.test.js +++ b/tests/node-qunit/changeListeners/statsv.test.js @@ -11,14 +11,14 @@ QUnit.module( 'ext.popups/changeListeners/statsv', { } ); QUnit.test( 'it should log the queued event', function ( assert ) { - const state = { + const newState = { statsv: { action: 'myAction', data: 123 } }; const changeListener = statsv( this.boundActions, this.track ); - changeListener( undefined, state ); + changeListener( undefined, newState ); assert.ok( this.track.calledWith( 'myAction', 123 ), @@ -32,13 +32,13 @@ QUnit.test( 'it should log the queued event', function ( assert ) { } ); QUnit.test( 'it should not log when no action is given', function ( assert ) { - const state = { + const newState = { statsv: { data: 123 } }; const changeListener = statsv( this.boundActions, this.track ); - changeListener( undefined, state ); + changeListener( undefined, newState ); assert.ok( this.track.notCalled, diff --git a/tests/node-qunit/changeListeners/syncUserSettings.test.js b/tests/node-qunit/changeListeners/syncUserSettings.test.js index 40918f679..8c314a414 100644 --- a/tests/node-qunit/changeListeners/syncUserSettings.test.js +++ b/tests/node-qunit/changeListeners/syncUserSettings.test.js @@ -15,19 +15,19 @@ QUnit.module( 'ext.popups/changeListeners/syncUserSettings', { QUnit.test( 'it shouldn\'t update the storage if the preview count hasn\'t changed', function ( assert ) { - const state = { + const newState = { eventLogging: { previewCount: 222 } }; - this.changeListener( undefined, state ); + this.changeListener( undefined, newState ); // --- - const prevState = $.extend( true, {}, state ); + const oldState = $.extend( true, {}, newState ); - this.changeListener( prevState, state ); + this.changeListener( oldState, newState ); assert.notOk( this.userSettings.storePreviewCount.called, @@ -37,16 +37,16 @@ QUnit.test( ); QUnit.test( 'it should update the storage if the previewCount has changed', function ( assert ) { - const prevState = { + const oldState = { eventLogging: { previewCount: 222 } }; - const state = $.extend( true, {}, prevState ); - ++state.eventLogging.previewCount; + const newState = $.extend( true, {}, oldState ); + ++newState.eventLogging.previewCount; - this.changeListener( prevState, state ); + this.changeListener( oldState, newState ); assert.ok( this.userSettings.storePreviewCount.calledWith( 223 ), @@ -57,19 +57,19 @@ QUnit.test( 'it should update the storage if the previewCount has changed', func QUnit.test( 'it shouldn\'t update the storage if the enabled state hasn\'t changed', function ( assert ) { - const state = { + const newState = { preview: { enabled: true } }; - this.changeListener( undefined, state ); + this.changeListener( undefined, newState ); // --- - const prevState = $.extend( true, {}, state ); + const oldState = $.extend( true, {}, newState ); - this.changeListener( prevState, state ); + this.changeListener( oldState, newState ); assert.notOk( this.userSettings.storePagePreviewsEnabled.called, @@ -79,16 +79,16 @@ QUnit.test( ); QUnit.test( 'it should update the storage if the enabled flag has changed', function ( assert ) { - const prevState = { + const oldState = { preview: { enabled: true } }; - const state = $.extend( true, {}, prevState ); - state.preview.enabled = false; + const newState = $.extend( true, {}, oldState ); + newState.preview.enabled = false; - this.changeListener( prevState, state ); + this.changeListener( oldState, newState ); assert.ok( this.userSettings.storePagePreviewsEnabled.calledWith( false ),