From 4521b50cc767657c01aa8d78c5fc51a331a1bc6d Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew@andrewduthie.com>
Date: Thu, 1 Mar 2018 21:18:19 -0500
Subject: [PATCH] Data: Avoid calling listeners on unchanging state

---
 data/index.js      | 14 ++++++++++++-
 data/test/index.js | 51 ++++++++++++++++++++++++++++++++++++----------
 2 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/data/index.js b/data/index.js
index 69fe38b89a2a80..844700ee5efc89 100644
--- a/data/index.js
+++ b/data/index.js
@@ -72,7 +72,19 @@ export function registerReducer( reducerKey, reducer ) {
 	}
 	const store = createStore( reducer, flowRight( enhancers ) );
 	stores[ reducerKey ] = store;
-	store.subscribe( globalListener );
+
+	// Customize subscribe behavior to call listeners only on effective change,
+	// not on every dispatch.
+	let lastState = store.getState();
+	store.subscribe( () => {
+		const state = store.getState();
+		const hasChanged = state !== lastState;
+		lastState = state;
+
+		if ( hasChanged ) {
+			globalListener();
+		}
+	} );
 
 	return store;
 }
diff --git a/data/test/index.js b/data/test/index.js
index 010387b0c5ccdb..268b4e3d1ab978 100644
--- a/data/test/index.js
+++ b/data/test/index.js
@@ -100,12 +100,19 @@ describe( 'select', () => {
 } );
 
 describe( 'withSelect', () => {
+	let wrapper;
+
 	const unsubscribes = [];
 	afterEach( () => {
 		let unsubscribe;
 		while ( ( unsubscribe = unsubscribes.shift() ) ) {
 			unsubscribe();
 		}
+
+		if ( wrapper ) {
+			wrapper.unmount();
+			wrapper = null;
+		}
 	} );
 
 	function subscribeWithUnsubscribe( ...args ) {
@@ -131,7 +138,7 @@ describe( 'withSelect', () => {
 			data: _select( 'reactReducer' ).reactSelector( ownProps.keyName ),
 		} ) )( ( props ) => <div>{ props.data }</div> );
 
-		const wrapper = mount( <Component keyName="reactKey" /> );
+		wrapper = mount( <Component keyName="reactKey" /> );
 
 		// Wrapper is the enhanced component. Find props on the rendered child.
 		const child = wrapper.childAt( 0 );
@@ -140,8 +147,6 @@ describe( 'withSelect', () => {
 			data: 'reactState',
 		} );
 		expect( wrapper.text() ).toBe( 'reactState' );
-
-		wrapper.unmount();
 	} );
 
 	it( 'should rerun selection on state changes', () => {
@@ -174,15 +179,33 @@ describe( 'withSelect', () => {
 			</button>
 		) );
 
-		const wrapper = mount( <Component /> );
+		wrapper = mount( <Component /> );
 
 		const button = wrapper.find( 'button' );
 
 		button.simulate( 'click' );
 
 		expect( button.text() ).toBe( '1' );
+	} );
+
+	it( 'should not rerun selection on unchanging state', () => {
+		const store = registerReducer( 'unchanging', ( state = {} ) => state );
+
+		registerSelectors( 'unchanging', {
+			getState: ( state ) => state,
+		} );
+
+		const mapSelectToProps = jest.fn();
+
+		const Component = compose( [
+			withSelect( mapSelectToProps ),
+		] )( () => <div /> );
+
+		wrapper = mount( <Component /> );
+
+		store.dispatch( { type: 'dummy' } );
 
-		wrapper.unmount();
+		expect( mapSelectToProps ).toHaveBeenCalledTimes( 1 );
 	} );
 
 	it( 'should rerun selection on props changes', () => {
@@ -202,13 +225,11 @@ describe( 'withSelect', () => {
 			count: _select( 'counter' ).getCount( ownProps.offset ),
 		} ) )( ( props ) => <div>{ props.count }</div> );
 
-		const wrapper = mount( <Component offset={ 0 } /> );
+		wrapper = mount( <Component offset={ 0 } /> );
 
 		wrapper.setProps( { offset: 10 } );
 
 		expect( wrapper.childAt( 0 ).text() ).toBe( '10' );
-
-		wrapper.unmount();
 	} );
 
 	it( 'ensures component is still mounted before setting state', () => {
@@ -236,7 +257,7 @@ describe( 'withSelect', () => {
 			count: _select( 'counter' ).getCount( ownProps.offset ),
 		} ) )( ( props ) => <div>{ props.count }</div> );
 
-		const wrapper = mount( <Component offset={ 0 } /> );
+		wrapper = mount( <Component offset={ 0 } /> );
 
 		store.dispatch( { type: 'increment' } );
 	} );
@@ -281,8 +302,6 @@ describe( 'withDispatch', () => {
 		wrapper.find( 'button' ).simulate( 'click' );
 
 		expect( store.getState() ).toBe( 2 );
-
-		wrapper.unmount();
 	} );
 } );
 
@@ -354,6 +373,16 @@ describe( 'subscribe', () => {
 
 		expect( secondListener ).toHaveBeenCalled();
 	} );
+
+	it( 'does not call listeners if state has not changed', () => {
+		const store = registerReducer( 'unchanging', ( state = {} ) => state );
+		const listener = jest.fn();
+		subscribeWithUnsubscribe( listener );
+
+		store.dispatch( { type: 'dummy' } );
+
+		expect( listener ).not.toHaveBeenCalled();
+	} );
 } );
 
 describe( 'dispatch', () => {