Add registry param to withDispatch component

packages/data/CHANGELOG.md

@@ -1,3 +1,10 @@
+## 4.1.0 (Unreleased)
+
+### New Feature
+
+- `withDispatch`'s `mapDispatchToProps` function takes the `registry` object as the 3rd param ([#11851](https://github.com/WordPress/gutenberg/pull/11851)).
+- `withSelect`'s `mapSelectToProps` function takes the `registry` object as the 3rd param ([#11851](https://github.com/WordPress/gutenberg/pull/11851)).
+
 ## 4.0.1 (2018-11-20)
 
 ## 4.0.0 (2018-11-15)

packages/data/README.md

@@ -229,7 +229,7 @@ A higher-order component is a function which accepts a [component](https://githu
 
 #### `withSelect( mapSelectToProps: Function ): Function`
 
-Use `withSelect` to inject state-derived props into a component. Passed a function which returns an object mapping prop names to the subscribed data source, a higher-order component function is returned. The higher-order component can be used to enhance a presentational component, updating it automatically when state changes. The mapping function is passed the [`select` function](#select) and the props passed to the original component.
+Use `withSelect` to inject state-derived props into a component. Passed a function which returns an object mapping prop names to the subscribed data source, a higher-order component function is returned. The higher-order component can be used to enhance a presentational component, updating it automatically when state changes. The mapping function is passed the [`select` function](#select), the props passed to the original component and the `registry` object.
 
 _Example:_
 
@@ -261,7 +261,7 @@ In the above example, when `HammerPriceDisplay` is rendered into an application,
 
 #### `withDispatch( mapDispatchToProps: Function ): Function`
 
-Use `withDispatch` to inject dispatching action props into your component. Passed a function which returns an object mapping prop names to action dispatchers, a higher-order component function is returned. The higher-order component can be used to enhance a component. For example, you can define callback behaviors as props for responding to user interactions. The mapping function is passed the [`dispatch` function](#dispatch) and the props passed to the original component.
+Use `withDispatch` to inject dispatching action props into your component. Passed a function which returns an object mapping prop names to action dispatchers, a higher-order component function is returned. The higher-order component can be used to enhance a component. For example, you can define callback behaviors as props for responding to user interactions. The mapping function is passed the [`dispatch` function](#dispatch), the props passed to the original component and the `registry` object.
 
 ```jsx
 function Button( { onClick, children } ) {
@@ -272,7 +272,7 @@ const { withDispatch } = wp.data;
 
 const SaleButton = withDispatch( ( dispatch, ownProps ) => {
 	const { startSale } = dispatch( 'my-shop' );
-	const { discountPercent = 20 } = ownProps;
+	const { discountPercent } = ownProps;
 
 	return {
 		onClick() {
@@ -281,6 +281,33 @@ const SaleButton = withDispatch( ( dispatch, ownProps ) => {
 	};
 } )( Button );
 
+// Rendered in the application:
+//
+//  <SaleButton discountPercent="20">Start Sale!</SaleButton>
+```
+
+In the majority of cases, it will be sufficient to use only two first params passed to `mapDispatchToProps` as illustrated in the previous example. However, there might be some very advanced use cases where using the `registry` object might be used as a tool to optimize the performance of your component. Using `select` function from the registry might be useful when you need to fetch some dynamic data from the store at the time when the event is fired, but at the same time, you never use it to render your component. In such scenario, you can avoid using the `withSelect` higher order component to compute such prop, which might lead to unnecessary re-renders of you component caused by its frequent value change. Keep in mind, that `mapDispatchToProps` must return an object with functions only. 
+
+```jsx
+function Button( { onClick, children } ) {
+	return <button type="button" onClick={ onClick }>{ children }</button>;
+}
+
+const { withDispatch } = wp.data;
+
+const SaleButton = withDispatch( ( dispatch, ownProps, { select } ) => {
+	// Stock number changes frequently.
+	const { getStockNumber } = select( 'my-shop' );
+	const { startSale } = dispatch( 'my-shop' );
+
+	return {
+		onClick() {
+			const dicountPercent = getStockNumber() > 50 ? 10 : 20;
+			startSale( discountPercent );
+		},
+	};
+} )( Button );
+
 // Rendered in the application:
 //
 //  <SaleButton>Start Sale!</SaleButton>

packages/data/src/components/with-dispatch/index.js

@@ -39,7 +39,7 @@ const withDispatch = ( mapDispatchToProps ) => createHigherOrderComponent(
 
 			proxyDispatch( propName, ...args ) {
 				// Original dispatcher is a pre-bound (dispatching) action creator.
-				mapDispatchToProps( this.props.registry.dispatch, this.props.ownProps )[ propName ]( ...args );
+				mapDispatchToProps( this.props.registry.dispatch, this.props.ownProps, this.props.registry )[ propName ]( ...args );
 			}
 
 			setProxyProps( props ) {
@@ -49,8 +49,12 @@ const withDispatch = ( mapDispatchToProps ) => createHigherOrderComponent(
 				// called, it is done only to determine the keys for which
 				// proxy functions should be created. The actual registry
 				// dispatch does not occur until the function is called.
-				const propsToDispatchers = mapDispatchToProps( this.props.registry.dispatch, props.ownProps );
+				const propsToDispatchers = mapDispatchToProps( this.props.registry.dispatch, props.ownProps, this.props.registry );
 				this.proxyProps = mapValues( propsToDispatchers, ( dispatcher, propName ) => {
+					if ( typeof dispatcher !== 'function' ) {
+						// eslint-disable-next-line no-console
+						console.warn( `Property ${ propName } returned from mapDispatchToProps in withDispatch must be a function.` );
+					}
 					// Prebind with prop name so we have reference to the original
 					// dispatcher to invoke. Track between re-renders to avoid
 					// creating new function references every render.

packages/data/src/components/with-dispatch/test/index.js

@@ -119,4 +119,67 @@ describe( 'withDispatch', () => {
 		expect( firstRegistryAction ).toHaveBeenCalledTimes( 2 );
 		expect( secondRegistryAction ).toHaveBeenCalledTimes( 2 );
 	} );
+
+	it( 'always calls select with the latest state in the handler passed to the component', () => {
+		const store = registry.registerStore( 'counter', {
+			reducer: ( state = 0, action ) => {
+				if ( action.type === 'update' ) {
+					return action.count;
+				}
+				return state;
+			},
+			actions: {
+				update: ( count ) => ( { type: 'update', count } ),
+			},
+			selectors: {
+				getCount: ( state ) => state,
+			},
+		} );
+
+		const Component = withDispatch( ( _dispatch, ownProps, { select: _select } ) => {
+			const outerCount = _select( 'counter' ).getCount();
+			return {
+				update: () => {
+					const innerCount = _select( 'counter' ).getCount();
+					expect( innerCount ).toBe( outerCount );
+					const actionReturnedFromDispatch = _dispatch( 'counter' ).update( innerCount + 1 );
+					expect( actionReturnedFromDispatch ).toBe( undefined );
+				},
+			};
+		} )( ( props ) => <button onClick={ props.update } /> );
+
+		const testRenderer = TestRenderer.create(
+			<RegistryProvider value={ registry }>
+				<Component />
+			</RegistryProvider>
+		);
+
+		const counterUpdateHandler = testRenderer.root.findByType( 'button' ).props.onClick;
+
+		counterUpdateHandler();
+		expect( store.getState() ).toBe( 1 );
+
+		counterUpdateHandler();
+		expect( store.getState() ).toBe( 2 );
+
+		counterUpdateHandler();
+		expect( store.getState() ).toBe( 3 );
+	} );
+
+	it( 'warns when mapDispatchToProps returns non-function property', () => {
+		const Component = withDispatch( () => {
+			return {
+				count: 3,
+			};
+		} )( () => null );
+
+		TestRenderer.create(
+			<RegistryProvider value={ registry }>
+				<Component />
+			</RegistryProvider>
+		);
+		expect( console ).toHaveWarnedWith(
+			'Property count returned from mapDispatchToProps in withDispatch must be a function.'
+		);
+	} );
 } );

packages/data/src/components/with-select/index.js

@@ -39,7 +39,7 @@ const withSelect = ( mapSelectToProps ) => createHigherOrderComponent( ( Wrapped
 	 */
 	function getNextMergeProps( props ) {
 		return (
-			mapSelectToProps( props.registry.select, props.ownProps ) ||
+			mapSelectToProps( props.registry.select, props.ownProps, props.registry ) ||
 			DEFAULT_MERGE_PROPS
 		);
 	}

packages/edit-post/src/components/header/index.js

@@ -82,18 +82,17 @@ function Header( {
 export default compose(
 	withSelect( ( select ) => ( {
 		hasActiveMetaboxes: select( 'core/edit-post' ).hasMetaBoxes(),
-		hasBlockSelection: !! select( 'core/editor' ).getBlockSelectionStart(),
 		isEditorSidebarOpened: select( 'core/edit-post' ).isEditorSidebarOpened(),
 		isPublishSidebarOpened: select( 'core/edit-post' ).isPublishSidebarOpened(),
 		isSaving: select( 'core/edit-post' ).isSavingMetaBoxes(),
 	} ) ),
-	withDispatch( ( dispatch, { hasBlockSelection } ) => {
+	withDispatch( ( dispatch, ownProps, { select } ) => {
+		const { getBlockSelectionStart } = select( 'core/editor' );
 		const { openGeneralSidebar, closeGeneralSidebar } = dispatch( 'core/edit-post' );
-		const sidebarToOpen = hasBlockSelection ? 'edit-post/block' : 'edit-post/document';
+
 		return {
-			openGeneralSidebar: () => openGeneralSidebar( sidebarToOpen ),
+			openGeneralSidebar: () => openGeneralSidebar( getBlockSelectionStart() ? 'edit-post/block' : 'edit-post/document' ),
 			closeGeneralSidebar: closeGeneralSidebar,
-			hasBlockSelection: undefined,
 		};
 	} ),
 )( Header );

packages/editor/src/components/block-list/block.js

@@ -71,7 +71,6 @@ export class BlockListBlock extends Component {
 		this.onDragStart = this.onDragStart.bind( this );
 		this.onDragEnd = this.onDragEnd.bind( this );
 		this.selectOnOpen = this.selectOnOpen.bind( this );
-		this.onShiftSelection = this.onShiftSelection.bind( this );
 		this.hadTouchStart = false;
 
 		this.state = {
@@ -290,7 +289,7 @@ export class BlockListBlock extends Component {
 
 		if ( event.shiftKey ) {
 			if ( ! this.props.isSelected ) {
-				this.onShiftSelection();
+				this.props.onShiftSelection();
 				event.preventDefault();
 			}
 		} else {
@@ -362,20 +361,6 @@ export class BlockListBlock extends Component {
 		}
 	}
 
-	onShiftSelection() {
-		if ( ! this.props.isSelectionEnabled ) {
-			return;
-		}
-
-		const { getBlockSelectionStart, onMultiSelect, onSelect } = this.props;
-
-		if ( getBlockSelectionStart() ) {
-			onMultiSelect( getBlockSelectionStart(), this.props.clientId );
-		} else {
-			onSelect( this.props.clientId );
-		}
-	}
-
 	forceFocusedContextualToolbar() {
 		this.isForcingContextualToolbar = true;
 		// trigger a re-render
@@ -649,7 +634,6 @@ const applyWithSelect = withSelect( ( select, { clientId, rootClientId, isLargeV
 		getEditorSettings,
 		hasSelectedInnerBlock,
 		getTemplateLock,
-		getBlockSelectionStart,
 	} = select( 'core/editor' );
 	const isSelected = isBlockSelected( clientId );
 	const { hasFixedToolbar, focusMode } = getEditorSettings();
@@ -682,13 +666,11 @@ const applyWithSelect = withSelect( ( select, { clientId, rootClientId, isLargeV
 		block,
 		isSelected,
 		isParentOfSelectedBlock,
-		// We only care about this value when the shift key is pressed.
-		// We call it dynamically in the event handler to avoid unnecessary re-renders.
-		getBlockSelectionStart,
 	};
 } );
 
-const applyWithDispatch = withDispatch( ( dispatch, ownProps ) => {
+const applyWithDispatch = withDispatch( ( dispatch, ownProps, { select } ) => {
+	const { getBlockSelectionStart } = select( 'core/editor' );
 	const {
 		updateBlockAttributes,
 		selectBlock,
@@ -709,7 +691,6 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps ) => {
 		onSelect( clientId = ownProps.clientId, initialPosition ) {
 			selectBlock( clientId, initialPosition );
 		},
-		onMultiSelect: multiSelect,
 		onInsertBlocks( blocks, index ) {
 			const { rootClientId } = ownProps;
 			insertBlocks( blocks, index, rootClientId );
@@ -730,6 +711,17 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps ) => {
 		onMetaChange( meta ) {
 			editPost( { meta } );
 		},
+		onShiftSelection() {
+			if ( ! ownProps.isSelectionEnabled ) {
+				return;
+			}
+
+			if ( getBlockSelectionStart() ) {
+				multiSelect( getBlockSelectionStart(), ownProps.clientId );
+			} else {
+				selectBlock( ownProps.clientId );
+			}
+		},
 		toggleSelection( selectionEnabled ) {
 			toggleSelection( selectionEnabled );
 		},

packages/editor/src/components/copy-handler/index.js

@@ -4,7 +4,7 @@
 import { Component } from '@wordpress/element';
 import { serialize } from '@wordpress/blocks';
 import { documentHasSelection } from '@wordpress/dom';
-import { withSelect, withDispatch } from '@wordpress/data';
+import { withDispatch } from '@wordpress/data';
 import { compose } from '@wordpress/compose';
 
 class CopyHandler extends Component {
@@ -26,33 +26,13 @@ class CopyHandler extends Component {
 	}
 
 	onCopy( event ) {
-		const { hasMultiSelection, selectedBlockClientIds, getBlocksByClientId } = this.props;
-
-		if ( selectedBlockClientIds.length === 0 ) {
-			return;
-		}
-
-		// Let native copy behaviour take over in input fields.
-		if ( ! hasMultiSelection && documentHasSelection() ) {
-			return;
-		}
-
-		const serialized = serialize( getBlocksByClientId( selectedBlockClientIds ) );
-
-		event.clipboardData.setData( 'text/plain', serialized );
-		event.clipboardData.setData( 'text/html', serialized );
-
+		this.props.onCopy( event.clipboardData );
 		event.preventDefault();
 	}
 
 	onCut( event ) {
-		const { hasMultiSelection, selectedBlockClientIds } = this.props;
-
-		this.onCopy( event );
-
-		if ( hasMultiSelection ) {
-			this.props.onRemove( selectedBlockClientIds );
-		}
+		this.props.onCut( event.clipboardData );
+		event.preventDefault();
 	}
 
 	render() {
@@ -61,27 +41,41 @@ class CopyHandler extends Component {
 }
 
 export default compose( [
-	withSelect( ( select ) => {
+	withDispatch( ( dispatch, ownProps, { select } ) => {
 		const {
+			getBlocksByClientId,
 			getMultiSelectedBlockClientIds,
 			getSelectedBlockClientId,
-			getBlocksByClientId,
 			hasMultiSelection,
 		} = select( 'core/editor' );
+		const { removeBlocks } = dispatch( 'core/editor' );
 
 		const selectedBlockClientId = getSelectedBlockClientId();
 		const selectedBlockClientIds = selectedBlockClientId ? [ selectedBlockClientId ] : getMultiSelectedBlockClientIds();
 
 		return {
-			hasMultiSelection: hasMultiSelection(),
-			selectedBlockClientIds,
-
-			// We only care about this value when the copy is performed
-			// We call it dynamically in the event handler to avoid unnecessary re-renders.
-			getBlocksByClientId,
+			onCopy( dataTransfer ) {
+				if ( selectedBlockClientIds.length === 0 ) {
+					return;
+				}
+
+				// Let native copy behaviour take over in input fields.
+				if ( ! hasMultiSelection() && documentHasSelection() ) {
+					return;
+				}
+
+				const serialized = serialize( getBlocksByClientId( selectedBlockClientIds ) );
+
+				dataTransfer.setData( 'text/plain', serialized );
+				dataTransfer.setData( 'text/html', serialized );
+			},
+			onCut( dataTransfer ) {
+				this.onCopy( dataTransfer );
+
+				if ( hasMultiSelection() ) {
+					removeBlocks( selectedBlockClientIds );
+				}
+			},
 		};
 	} ),
-	withDispatch( ( dispatch ) => ( {
-		onRemove: dispatch( 'core/editor' ).removeBlocks,
-	} ) ),
 ] )( CopyHandler );