From 6b47489903380ab5f4c36ee5ebc901210cc8496a Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew@andrewduthie.com>
Date: Fri, 2 Mar 2018 09:00:47 -0500
Subject: [PATCH] Data: Fix data.query backwards compatibility with props
 (#5220)

---
 data/index.js      |   4 +-
 data/test/index.js | 228 +++++++++++++++++++++++++--------------------
 2 files changed, 128 insertions(+), 104 deletions(-)

diff --git a/data/index.js b/data/index.js
index 69fe38b89a2a8..85b67b7c3b247 100644
--- a/data/index.js
+++ b/data/index.js
@@ -288,7 +288,5 @@ export const query = ( mapSelectToProps ) => {
 		plugin: 'Gutenberg',
 	} );
 
-	return withSelect( ( props ) => {
-		return mapSelectToProps( select, props );
-	} );
+	return withSelect( mapSelectToProps );
 };
diff --git a/data/test/index.js b/data/test/index.js
index 010387b0c5ccd..12cfd573adbbf 100644
--- a/data/test/index.js
+++ b/data/test/index.js
@@ -1,11 +1,13 @@
 /**
  * External dependencies
  */
+import { noop, flowRight } from 'lodash';
 import { mount } from 'enzyme';
 
 /**
  * WordPress dependencies
  */
+import { deprecated } from '@wordpress/utils';
 import { compose } from '@wordpress/element';
 
 /**
@@ -21,8 +23,13 @@ import {
 	withSelect,
 	withDispatch,
 	subscribe,
+	query,
 } from '../';
 
+jest.mock( '@wordpress/utils', () => ( {
+	deprecated: jest.fn(),
+} ) );
+
 describe( 'registerStore', () => {
 	it( 'should be shorthand for reducer, actions, selectors registration', () => {
 		const store = registerStore( 'butcher', {
@@ -95,17 +102,24 @@ describe( 'select', () => {
 
 		expect( select( 'reducer', 'selector', 'arg' ) ).toEqual( 'result' );
 		expect( selector ).toBeCalledWith( store.getState(), 'arg' );
-		expect( console ).toHaveWarned();
+		expect( deprecated ).toHaveBeenCalled();
 	} );
 } );
 
 describe( 'withSelect', () => {
+	let wrapper;
+
 	const unsubscribes = [];
 	afterEach( () => {
 		let unsubscribe;
 		while ( ( unsubscribe = unsubscribes.shift() ) ) {
 			unsubscribe();
 		}
+
+		if ( wrapper ) {
+			wrapper.unmount();
+			wrapper = null;
+		}
 	} );
 
 	function subscribeWithUnsubscribe( ...args ) {
@@ -114,135 +128,149 @@ describe( 'withSelect', () => {
 		return unsubscribe;
 	}
 
-	it( 'passes the relevant data to the component', () => {
-		registerReducer( 'reactReducer', () => ( { reactKey: 'reactState' } ) );
-		registerSelectors( 'reactReducer', {
-			reactSelector: ( state, key ) => state[ key ],
-		} );
-
-		// In normal circumstances, the fact that we have to add an arbitrary
-		// prefix to the variable name would be concerning, and perhaps an
-		// argument that we ought to expect developer to use select from the
-		// wp.data export. But in-fact, this serves as a good deterrent for
-		// including both `withSelect` and `select` in the same scope, which
-		// shouldn't occur for a typical component, and if it did might wrongly
-		// encourage the developer to use `select` within the component itself.
-		const Component = withSelect( ( _select, ownProps ) => ( {
-			data: _select( 'reactReducer' ).reactSelector( ownProps.keyName ),
-		} ) )( ( props ) => <div>{ props.data }</div> );
-
-		const wrapper = mount( <Component keyName="reactKey" /> );
+	function cases( withSelectImplementation, extraAssertions = noop ) {
+		function itWithExtraAssertions( description, test ) {
+			return it( description, flowRight( test, extraAssertions ) );
+		}
 
-		// Wrapper is the enhanced component. Find props on the rendered child.
-		const child = wrapper.childAt( 0 );
-		expect( child.props() ).toEqual( {
-			keyName: 'reactKey',
-			data: 'reactState',
+		itWithExtraAssertions( 'passes the relevant data to the component', () => {
+			registerReducer( 'reactReducer', () => ( { reactKey: 'reactState' } ) );
+			registerSelectors( 'reactReducer', {
+				reactSelector: ( state, key ) => state[ key ],
+			} );
+
+			// In normal circumstances, the fact that we have to add an arbitrary
+			// prefix to the variable name would be concerning, and perhaps an
+			// argument that we ought to expect developer to use select from the
+			// wp.data export. But in-fact, this serves as a good deterrent for
+			// including both `withSelect` and `select` in the same scope, which
+			// shouldn't occur for a typical component, and if it did might wrongly
+			// encourage the developer to use `select` within the component itself.
+			const Component = withSelectImplementation( ( _select, ownProps ) => ( {
+				data: _select( 'reactReducer' ).reactSelector( ownProps.keyName ),
+			} ) )( ( props ) => <div>{ props.data }</div> );
+
+			wrapper = mount( <Component keyName="reactKey" /> );
+
+			// Wrapper is the enhanced component. Find props on the rendered child.
+			const child = wrapper.childAt( 0 );
+			expect( child.props() ).toEqual( {
+				keyName: 'reactKey',
+				data: 'reactState',
+			} );
+			expect( wrapper.text() ).toBe( 'reactState' );
 		} );
-		expect( wrapper.text() ).toBe( 'reactState' );
 
-		wrapper.unmount();
-	} );
+		itWithExtraAssertions( 'should rerun selection on state changes', () => {
+			registerReducer( 'counter', ( state = 0, action ) => {
+				if ( action.type === 'increment' ) {
+					return state + 1;
+				}
 
-	it( 'should rerun selection on state changes', () => {
-		registerReducer( 'counter', ( state = 0, action ) => {
-			if ( action.type === 'increment' ) {
-				return state + 1;
-			}
+				return state;
+			} );
 
-			return state;
-		} );
+			registerSelectors( 'counter', {
+				getCount: ( state ) => state,
+			} );
 
-		registerSelectors( 'counter', {
-			getCount: ( state ) => state,
-		} );
+			registerActions( 'counter', {
+				increment: () => ( { type: 'increment' } ),
+			} );
 
-		registerActions( 'counter', {
-			increment: () => ( { type: 'increment' } ),
-		} );
+			const Component = compose( [
+				withSelectImplementation( ( _select ) => ( {
+					count: _select( 'counter' ).getCount(),
+				} ) ),
+				withDispatch( ( _dispatch ) => ( {
+					increment: _dispatch( 'counter' ).increment,
+				} ) ),
+			] )( ( props ) => (
+				<button onClick={ props.increment }>
+					{ props.count }
+				</button>
+			) );
 
-		const Component = compose( [
-			withSelect( ( _select ) => ( {
-				count: _select( 'counter' ).getCount(),
-			} ) ),
-			withDispatch( ( _dispatch ) => ( {
-				increment: _dispatch( 'counter' ).increment,
-			} ) ),
-		] )( ( props ) => (
-			<button onClick={ props.increment }>
-				{ props.count }
-			</button>
-		) );
+			wrapper = mount( <Component /> );
 
-		const wrapper = mount( <Component /> );
+			const button = wrapper.find( 'button' );
 
-		const button = wrapper.find( 'button' );
+			button.simulate( 'click' );
 
-		button.simulate( 'click' );
+			expect( button.text() ).toBe( '1' );
+		} );
 
-		expect( button.text() ).toBe( '1' );
+		itWithExtraAssertions( 'should rerun selection on props changes', () => {
+			registerReducer( 'counter', ( state = 0, action ) => {
+				if ( action.type === 'increment' ) {
+					return state + 1;
+				}
 
-		wrapper.unmount();
-	} );
+				return state;
+			} );
 
-	it( 'should rerun selection on props changes', () => {
-		registerReducer( 'counter', ( state = 0, action ) => {
-			if ( action.type === 'increment' ) {
-				return state + 1;
-			}
+			registerSelectors( 'counter', {
+				getCount: ( state, offset ) => state + offset,
+			} );
 
-			return state;
-		} );
+			const Component = withSelectImplementation( ( _select, ownProps ) => ( {
+				count: _select( 'counter' ).getCount( ownProps.offset ),
+			} ) )( ( props ) => <div>{ props.count }</div> );
 
-		registerSelectors( 'counter', {
-			getCount: ( state, offset ) => state + offset,
-		} );
+			wrapper = mount( <Component offset={ 0 } /> );
 
-		const Component = withSelect( ( _select, ownProps ) => ( {
-			count: _select( 'counter' ).getCount( ownProps.offset ),
-		} ) )( ( props ) => <div>{ props.count }</div> );
+			wrapper.setProps( { offset: 10 } );
 
-		const wrapper = mount( <Component offset={ 0 } /> );
+			expect( wrapper.childAt( 0 ).text() ).toBe( '10' );
+		} );
 
-		wrapper.setProps( { offset: 10 } );
+		itWithExtraAssertions( 'ensures component is still mounted before setting state', () => {
+			// This test verifies that even though unsubscribe doesn't take effect
+			// until after the current listener stack is called, we don't attempt
+			// to setState on an unmounting `withSelect` component. It will fail if
+			// an attempt is made to `setState` on an unmounted component.
+			const store = registerReducer( 'counter', ( state = 0, action ) => {
+				if ( action.type === 'increment' ) {
+					return state + 1;
+				}
 
-		expect( wrapper.childAt( 0 ).text() ).toBe( '10' );
+				return state;
+			} );
 
-		wrapper.unmount();
-	} );
+			registerSelectors( 'counter', {
+				getCount: ( state, offset ) => state + offset,
+			} );
 
-	it( 'ensures component is still mounted before setting state', () => {
-		// This test verifies that even though unsubscribe doesn't take effect
-		// until after the current listener stack is called, we don't attempt
-		// to setState on an unmounting `withSelect` component. It will fail if
-		// an attempt is made to `setState` on an unmounted component.
-		const store = registerReducer( 'counter', ( state = 0, action ) => {
-			if ( action.type === 'increment' ) {
-				return state + 1;
-			}
+			subscribeWithUnsubscribe( () => {
+				wrapper.unmount();
+			} );
 
-			return state;
-		} );
+			const Component = withSelectImplementation( ( _select, ownProps ) => ( {
+				count: _select( 'counter' ).getCount( ownProps.offset ),
+			} ) )( ( props ) => <div>{ props.count }</div> );
 
-		registerSelectors( 'counter', {
-			getCount: ( state, offset ) => state + offset,
-		} );
+			wrapper = mount( <Component offset={ 0 } /> );
 
-		subscribeWithUnsubscribe( () => {
-			wrapper.unmount();
+			store.dispatch( { type: 'increment' } );
 		} );
+	}
 
-		const Component = withSelect( ( _select, ownProps ) => ( {
-			count: _select( 'counter' ).getCount( ownProps.offset ),
-		} ) )( ( props ) => <div>{ props.count }</div> );
-
-		const wrapper = mount( <Component offset={ 0 } /> );
+	cases( withSelect );
 
-		store.dispatch( { type: 'increment' } );
+	describe( 'query backwards-compatibility', () => {
+		cases( query, () => expect( deprecated ).toHaveBeenCalled() );
 	} );
 } );
 
 describe( 'withDispatch', () => {
+	let wrapper;
+	afterEach( () => {
+		if ( wrapper ) {
+			wrapper.unmount();
+			wrapper = null;
+		}
+	} );
+
 	it( 'passes the relevant data to the component', () => {
 		const store = registerReducer( 'counter', ( state = 0, action ) => {
 			if ( action.type === 'increment' ) {
@@ -264,7 +292,7 @@ describe( 'withDispatch', () => {
 			};
 		} )( ( props ) => <button onClick={ props.increment } /> );
 
-		const wrapper = mount( <Component count={ 0 } /> );
+		wrapper = mount( <Component count={ 0 } /> );
 
 		// Wrapper is the enhanced component. Find props on the rendered child.
 		const child = wrapper.childAt( 0 );
@@ -281,8 +309,6 @@ describe( 'withDispatch', () => {
 		wrapper.find( 'button' ).simulate( 'click' );
 
 		expect( store.getState() ).toBe( 2 );
-
-		wrapper.unmount();
 	} );
 } );