diff --git a/packages/data/CHANGELOG.md b/packages/data/CHANGELOG.md
index 6dad7cf833a791..259755bf03cd9e 100644
--- a/packages/data/CHANGELOG.md
+++ b/packages/data/CHANGELOG.md
@@ -1,3 +1,9 @@
+## 3.1.3 (Unreleased)
+
+### Bug Fix
+
+- Resolve an issue where `withSelect`'s `mapSelectToProps` would not be rerun if the wrapped component had incurred a store change during its mount lifecycle.
+
## 3.1.2 (2018-11-09)
## 3.1.1 (2018-11-09)
diff --git a/packages/data/src/components/with-select/index.js b/packages/data/src/components/with-select/index.js
index de681581904c54..c471bce36388c5 100644
--- a/packages/data/src/components/with-select/index.js
+++ b/packages/data/src/components/with-select/index.js
@@ -48,6 +48,8 @@ const withSelect = ( mapSelectToProps ) => createHigherOrderComponent( ( Wrapped
constructor( props ) {
super( props );
+ this.onStoreChange = this.onStoreChange.bind( this );
+
this.subscribe( props.registry );
this.mergeProps = getNextMergeProps( props );
@@ -55,6 +57,14 @@ const withSelect = ( mapSelectToProps ) => createHigherOrderComponent( ( Wrapped
componentDidMount() {
this.canRunSelection = true;
+
+ // A state change may have occurred between the constructor and
+ // mount of the component (e.g. during the wrapped component's own
+ // constructor), in which case selection should be rerun.
+ if ( this.hasQueuedSelection ) {
+ this.hasQueuedSelection = false;
+ this.onStoreChange();
+ }
}
componentWillUnmount() {
@@ -103,29 +113,31 @@ const withSelect = ( mapSelectToProps ) => createHigherOrderComponent( ( Wrapped
return true;
}
- subscribe( registry ) {
- this.unsubscribe = registry.subscribe( () => {
- if ( ! this.canRunSelection ) {
- return;
- }
+ onStoreChange() {
+ if ( ! this.canRunSelection ) {
+ this.hasQueuedSelection = true;
+ return;
+ }
- const nextMergeProps = getNextMergeProps( this.props );
- if ( isShallowEqual( this.mergeProps, nextMergeProps ) ) {
- return;
- }
+ const nextMergeProps = getNextMergeProps( this.props );
+ if ( isShallowEqual( this.mergeProps, nextMergeProps ) ) {
+ return;
+ }
+
+ this.mergeProps = nextMergeProps;
- this.mergeProps = nextMergeProps;
-
- // Schedule an update. Merge props are not assigned to state
- // because derivation of merge props from incoming props occurs
- // within shouldComponentUpdate, where setState is not allowed.
- // setState is used here instead of forceUpdate because forceUpdate
- // bypasses shouldComponentUpdate altogether, which isn't desireable
- // if both state and props change within the same render.
- // Unfortunately this requires that next merge props are generated
- // twice.
- this.setState( {} );
- } );
+ // Schedule an update. Merge props are not assigned to state since
+ // derivation of merge props from incoming props occurs within
+ // shouldComponentUpdate, where setState is not allowed. setState
+ // is used here instead of forceUpdate because forceUpdate bypasses
+ // shouldComponentUpdate altogether, which isn't desireable if both
+ // state and props change within the same render. Unfortunately,
+ // this requires that next merge props are generated twice.
+ this.setState( {} );
+ }
+
+ subscribe( registry ) {
+ this.unsubscribe = registry.subscribe( this.onStoreChange );
}
render() {
diff --git a/packages/data/src/components/with-select/test/index.js b/packages/data/src/components/with-select/test/index.js
index 2af54df5170204..4a874872df032d 100644
--- a/packages/data/src/components/with-select/test/index.js
+++ b/packages/data/src/components/with-select/test/index.js
@@ -7,6 +7,7 @@ import TestRenderer from 'react-test-renderer';
* WordPress dependencies
*/
import { compose } from '@wordpress/compose';
+import { Component } from '@wordpress/element';
/**
* Internal dependencies
@@ -45,11 +46,11 @@ describe( 'withSelect', () => {
{ props.data }
) );
- const Component = withSelect( mapSelectToProps )( OriginalComponent );
+ const DataBoundComponent = withSelect( mapSelectToProps )( OriginalComponent );
const testRenderer = TestRenderer.create(
-
+
);
const testInstance = testRenderer.root;
@@ -94,14 +95,14 @@ describe( 'withSelect', () => {
) );
- const Component = compose( [
+ const DataBoundComponent = compose( [
withSelect( mapSelectToProps ),
withDispatch( mapDispatchToProps ),
] )( OriginalComponent );
const testRenderer = TestRenderer.create(
-
+
);
const testInstance = testRenderer.root;
@@ -123,6 +124,66 @@ describe( 'withSelect', () => {
expect( OriginalComponent ).toHaveBeenCalledTimes( 2 );
} );
+ it( 'should rerun if had dispatched action during mount', () => {
+ registry.registerStore( 'counter', {
+ reducer: ( state = 0, action ) => {
+ if ( action.type === 'increment' ) {
+ return state + 1;
+ }
+
+ return state;
+ },
+ selectors: {
+ getCount: ( state ) => state,
+ },
+ actions: {
+ increment: () => ( { type: 'increment' } ),
+ },
+ } );
+
+ class OriginalComponent extends Component {
+ constructor( props ) {
+ super( ...arguments );
+
+ props.increment();
+ }
+
+ componentDidMount() {
+ this.props.increment();
+ }
+
+ render() {
+ return { this.props.count }
;
+ }
+ }
+
+ jest.spyOn( OriginalComponent.prototype, 'render' );
+
+ const mapSelectToProps = jest.fn().mockImplementation( ( _select, ownProps ) => ( {
+ count: _select( 'counter' ).getCount( ownProps.offset ),
+ } ) );
+
+ const mapDispatchToProps = jest.fn().mockImplementation( ( _dispatch ) => ( {
+ increment: _dispatch( 'counter' ).increment,
+ } ) );
+
+ const DataBoundComponent = compose( [
+ withSelect( mapSelectToProps ),
+ withDispatch( mapDispatchToProps ),
+ ] )( OriginalComponent );
+
+ const testRenderer = TestRenderer.create(
+
+
+
+ );
+ const testInstance = testRenderer.root;
+
+ expect( testInstance.findByType( 'div' ).props.children ).toBe( 2 );
+ expect( mapSelectToProps ).toHaveBeenCalledTimes( 2 );
+ expect( OriginalComponent.prototype.render ).toHaveBeenCalledTimes( 2 );
+ } );
+
it( 'should rerun selection on props changes', () => {
registry.registerStore( 'counter', {
reducer: ( state = 0, action ) => {
@@ -145,11 +206,11 @@ describe( 'withSelect', () => {
{ props.count }
) );
- const Component = withSelect( mapSelectToProps )( OriginalComponent );
+ const DataBoundComponent = withSelect( mapSelectToProps )( OriginalComponent );
const testRenderer = TestRenderer.create(
-
+
);
const testInstance = testRenderer.root;
@@ -159,7 +220,7 @@ describe( 'withSelect', () => {
testRenderer.update(
-
+
);
@@ -180,11 +241,11 @@ describe( 'withSelect', () => {
const OriginalComponent = jest.fn().mockImplementation( () => );
- const Component = compose( [
+ const DataBoundComponent = compose( [
withSelect( mapSelectToProps ),
] )( OriginalComponent );
- const Parent = ( props ) => ;
+ const Parent = ( props ) => ;
const testRenderer = TestRenderer.create(
@@ -222,11 +283,11 @@ describe( 'withSelect', () => {
const OriginalComponent = jest.fn().mockImplementation( () => );
- const Component = withSelect( mapSelectToProps )( OriginalComponent );
+ const DataBoundComponent = withSelect( mapSelectToProps )( OriginalComponent );
TestRenderer.create(
-
+
);
@@ -251,13 +312,13 @@ describe( 'withSelect', () => {
const OriginalComponent = jest.fn().mockImplementation( () => );
- const Component = compose( [
+ const DataBoundComponent = compose( [
withSelect( mapSelectToProps ),
] )( OriginalComponent );
const testRenderer = TestRenderer.create(
-
+
);
@@ -266,7 +327,7 @@ describe( 'withSelect', () => {
testRenderer.update(
-
+
);
@@ -286,13 +347,13 @@ describe( 'withSelect', () => {
const OriginalComponent = jest.fn().mockImplementation( () => );
- const Component = compose( [
+ const DataBoundComponent = compose( [
withSelect( mapSelectToProps ),
] )( OriginalComponent );
TestRenderer.create(
-
+
);
@@ -322,11 +383,11 @@ describe( 'withSelect', () => {
const OriginalComponent = jest.fn()
.mockImplementation( ( props ) => { JSON.stringify( props ) }
);
- const Component = withSelect( mapSelectToProps )( OriginalComponent );
+ const DataBoundComponent = withSelect( mapSelectToProps )( OriginalComponent );
const testRenderer = TestRenderer.create(
-
+
);
const testInstance = testRenderer.root;
@@ -339,7 +400,7 @@ describe( 'withSelect', () => {
testRenderer.update(
-
+
);
@@ -369,11 +430,11 @@ describe( 'withSelect', () => {
( props ) => { props.count || 'Unknown' }
) );
- const Component = withSelect( mapSelectToProps )( OriginalComponent );
+ const DataBoundComponent = withSelect( mapSelectToProps )( OriginalComponent );
const testRenderer = TestRenderer.create(
-
+
);
const testInstance = testRenderer.root;
@@ -384,7 +445,7 @@ describe( 'withSelect', () => {
testRenderer.update(
-
+
);
@@ -394,7 +455,7 @@ describe( 'withSelect', () => {
testRenderer.update(
-
+
);
@@ -465,11 +526,11 @@ describe( 'withSelect', () => {
{ props.value }
) );
- const Component = withSelect( mapSelectToProps )( OriginalComponent );
+ const DataBoundComponent = withSelect( mapSelectToProps )( OriginalComponent );
const testRenderer = TestRenderer.create(
-
+
);
const testInstance = testRenderer.root;
@@ -491,7 +552,7 @@ describe( 'withSelect', () => {
testRenderer.update(
-
+
);