Skip to content

Commit

Permalink
Data: Fix data.query backwards compatibility with props (#5220)
Browse files Browse the repository at this point in the history
  • Loading branch information
@aduth
aduth authored Mar 2, 2018
1 parent 68394ce commit 6b47489
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 104 deletions.
4 changes: 1 addition & 3 deletions data/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,5 @@ export const query = ( mapSelectToProps ) => {
plugin: 'Gutenberg',
} );

return withSelect( ( props ) => {
return mapSelectToProps( select, props );
} );
return withSelect( mapSelectToProps );
};
228 changes: 127 additions & 101 deletions data/test/index.js
View file
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand All @@ -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', {
Expand Down Expand Up @@ -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 ) {
Expand All @@ -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' ) {
Expand All @@ -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 );
Expand All @@ -281,8 +309,6 @@ describe( 'withDispatch', () => {
wrapper.find( 'button' ).simulate( 'click' );

expect( store.getState() ).toBe( 2 );

wrapper.unmount();
} );
} );

Expand Down

0 comments on commit 6b47489

Please sign in to comment.