Rename variables in change listeners for clarity

This patch does nothing but rename a pair of variables:
"prevState/state" becomes "oldState/newState". Reasoning:

1. The abbreviated "prev" is confusing, especially because we
are in a codebase that is all about "previews".

2. We are in a context that is all about a state **change**.
Change listeners get notified about the change from one state
to another. While it would be possible to stick to the already
mentioned "previous/current" terminology, I find the word
"current" confusing. What is "current" in this context? Did
the state already change? Am I notified about a change that is
**going** to happen or already happened? Is this even relevant?
I don't think it is. Therefor "old/new".

Another possibility is "previous/next".

Change-Id: Id886e1a095967fe86fb9021f59e335c62da8994e

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 ) {
     // ...
   }
 };

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;
 		}
 	} );
 }

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 ) {

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();

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 );
 		}
 	};
 }

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();

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;
 		}

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 );
 		}
 	};
 }

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 );

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 );
 	}
 }

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.

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,

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' ),

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,

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.'
 	);
 } );

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,

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 ),