Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shared Blocks: Refactor fetching/saving/updating to avoid cycle dependency #7080

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions core-blocks/block/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Placeholder, Spinner, Disabled } from '@wordpress/components';
import { withSelect, withDispatch } from '@wordpress/data';
import { __ } from '@wordpress/i18n';
import { BlockEdit } from '@wordpress/editor';
import { serialize } from '@wordpress/blocks';

/**
* Internal dependencies
Expand Down Expand Up @@ -72,14 +73,11 @@ class SharedBlockEdit extends Component {
}

save() {
const { sharedBlock, onUpdateTitle, updateAttributes, block, onSave } = this.props;
const { onChange, block, onSave } = this.props;
const { title, changedAttributes } = this.state;

if ( title !== sharedBlock.title ) {
onUpdateTitle( title );
}

updateAttributes( block.uid, changedAttributes );
const content = serialize( { ...block, attributes: { ...block.attributes, ...changedAttributes } } );
onChange( { title, content } );
onSave();

this.stopEditing();
Expand Down Expand Up @@ -138,7 +136,7 @@ export default compose( [
getSharedBlock,
isFetchingSharedBlock,
isSavingSharedBlock,
getBlock,
getParsedSharedBlock,
} = select( 'core/editor' );
const { ref } = ownProps.attributes;
const sharedBlock = getSharedBlock( ref );
Expand All @@ -147,22 +145,20 @@ export default compose( [
sharedBlock,
isFetching: isFetchingSharedBlock( ref ),
isSaving: isSavingSharedBlock( ref ),
block: sharedBlock ? getBlock( sharedBlock.uid ) : null,
block: getParsedSharedBlock( ref ),
};
} ),
withDispatch( ( dispatch, ownProps ) => {
const {
fetchSharedBlocks,
updateBlockAttributes,
updateSharedBlockTitle,
updateSharedBlock,
saveSharedBlock,
} = dispatch( 'core/editor' );
const { ref } = ownProps.attributes;

return {
fetchSharedBlock: partial( fetchSharedBlocks, ref ),
updateAttributes: updateBlockAttributes,
onUpdateTitle: partial( updateSharedBlockTitle, ref ),
onChange: partial( updateSharedBlock, ref ),
onSave: partial( saveSharedBlock, ref ),
};
} ),
Expand Down
14 changes: 6 additions & 8 deletions editor/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,6 @@ export function fetchSharedBlocks( id ) {
/**
* Returns an action object used in signalling that shared blocks have been
* received. `results` is an array of objects containing:
* - `sharedBlock` - Details about how the shared block is persisted.
* - `parsedBlock` - The original block.
*
* @param {Object[]} results Shared blocks received.
*
Expand All @@ -591,7 +589,7 @@ export function fetchSharedBlocks( id ) {
export function receiveSharedBlocks( results ) {
return {
type: 'RECEIVE_SHARED_BLOCKS',
results,
results: castArray( results ),
};
}

Expand Down Expand Up @@ -628,16 +626,16 @@ export function deleteSharedBlock( id ) {
* Returns an action object used in signalling that a shared block's title is
* to be updated.
*
* @param {number} id The ID of the shared block to update.
* @param {string} title The new title.
* @param {number} id The ID of the shared block to update.
* @param {Object} changes The updated shared block properties.
*
* @return {Object} Action object.
*/
export function updateSharedBlockTitle( id, title ) {
export function updateSharedBlock( id, changes ) {
return {
type: 'UPDATE_SHARED_BLOCK_TITLE',
type: 'UPDATE_SHARED_BLOCK',
id,
title,
changes,
};
}

Expand Down
39 changes: 8 additions & 31 deletions editor/store/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import { BEGIN, COMMIT, REVERT } from 'redux-optimist';
import { get, includes, last, map, castArray, uniqueId } from 'lodash';
import { get, includes, last, uniqueId } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -29,7 +29,6 @@ import {
setupEditorState,
resetAutosave,
resetPost,
receiveBlocks,
receiveSharedBlocks,
replaceBlock,
replaceBlocks,
Expand Down Expand Up @@ -420,13 +419,7 @@ export default {

result.then(
( sharedBlockOrBlocks ) => {
dispatch( receiveSharedBlocks( map(
castArray( sharedBlockOrBlocks ),
( sharedBlock ) => ( {
sharedBlock,
parsedBlock: parse( sharedBlock.content )[ 0 ],
} )
) ) );
dispatch( receiveSharedBlocks( sharedBlockOrBlocks ) );

dispatch( {
type: 'FETCH_SHARED_BLOCKS_SUCCESS',
Expand All @@ -445,9 +438,6 @@ export default {
}
);
},
RECEIVE_SHARED_BLOCKS( action ) {
return receiveBlocks( map( action.results, 'parsedBlock' ) );
},
SAVE_SHARED_BLOCK( action, store ) {
// TODO: these are potentially undefined, this fix is in place
// until there is a filter to not use shared blocks if undefined
Expand All @@ -460,10 +450,7 @@ export default {
const { dispatch } = store;
const state = store.getState();

const { uid, title, isTemporary } = getSharedBlock( state, id );
const { name, attributes, innerBlocks } = getBlock( state, uid );
const content = serialize( createBlock( name, attributes, innerBlocks ) );

const { isTemporary, title, content } = getSharedBlock( state, id );
const data = isTemporary ? { title, content } : { id, title, content };
const path = isTemporary ? `/wp/v2/${ basePath }` : `/wp/v2/${ basePath }/${ id }`;
const method = isTemporary ? 'POST' : 'PUT';
Expand Down Expand Up @@ -519,10 +506,7 @@ export default {
} );

// Remove the parsed block.
dispatch( removeBlocks( [
...associatedBlockUids,
sharedBlock.uid,
] ) );
dispatch( removeBlocks( associatedBlockUids ) );

wp.apiRequest( { path: `/wp/v2/${ basePath }/${ id }`, method: 'DELETE' } ).then(
() => {
Expand Down Expand Up @@ -552,8 +536,8 @@ export default {
const state = store.getState();
const oldBlock = getBlock( state, action.uid );
const sharedBlock = getSharedBlock( state, oldBlock.attributes.ref );
const referencedBlock = getBlock( state, sharedBlock.uid );
const newBlock = createBlock( referencedBlock.name, referencedBlock.attributes );
const parsedBlock = parse( sharedBlock.content )[ 0 ];
const newBlock = createBlock( parsedBlock.name, parsedBlock.attributes );
store.dispatch( replaceBlock( oldBlock.uid, newBlock ) );
},
CONVERT_BLOCK_TO_SHARED( action, store ) {
Expand All @@ -562,15 +546,11 @@ export default {
const parsedBlock = getBlock( getState(), action.uid );
const sharedBlock = {
id: uniqueId( 'shared' ),
uid: parsedBlock.uid,
title: __( 'Untitled shared block' ),
content: serialize( parsedBlock ),
};

dispatch( receiveSharedBlocks( [ {
sharedBlock,
parsedBlock,
} ] ) );

dispatch( receiveSharedBlocks( sharedBlock ) );
dispatch( saveSharedBlock( sharedBlock.id ) );

dispatch( replaceBlock(
Expand All @@ -580,9 +560,6 @@ export default {
layout: parsedBlock.attributes.layout,
} )
) );

// Re-add the original block to the store, since replaceBlock() will have removed it
dispatch( receiveBlocks( [ parsedBlock ] ) );
},
CREATE_NOTICE( { notice: { content, spokenMessage } } ) {
const message = spokenMessage || content;
Expand Down
24 changes: 9 additions & 15 deletions editor/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -921,35 +921,26 @@ export const sharedBlocks = combineReducers( {
switch ( action.type ) {
case 'RECEIVE_SHARED_BLOCKS': {
return reduce( action.results, ( nextState, result ) => {
const { id, title } = result.sharedBlock;
const { uid } = result.parsedBlock;

const value = { uid, title };

if ( ! isEqual( nextState[ id ], value ) ) {
if ( ! isEqual( nextState[ result.id ], result ) ) {
if ( nextState === state ) {
nextState = { ...nextState };
}

nextState[ id ] = value;
nextState[ result.id ] = result;
}

return nextState;
}, state );
}

case 'UPDATE_SHARED_BLOCK_TITLE': {
const { id, title } = action;

if ( ! state[ id ] || state[ id ].title === title ) {
return state;
}
case 'UPDATE_SHARED_BLOCK': {
const { id, changes } = action;

return {
...state,
[ id ]: {
...state[ id ],
title,
...changes,
},
};
}
Expand All @@ -965,7 +956,10 @@ export const sharedBlocks = combineReducers( {
const value = state[ id ];
return {
...omit( state, id ),
[ updatedId ]: value,
[ updatedId ]: {
id: updatedId,
...value,
},
};
}

Expand Down
28 changes: 25 additions & 3 deletions editor/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import createSelector from 'rememo';
/**
* WordPress dependencies
*/
import { serialize, getBlockType, getBlockTypes, hasBlockSupport, hasChildBlocks } from '@wordpress/blocks';
import { parse, serialize, getBlockType, getBlockTypes, hasBlockSupport, hasChildBlocks } from '@wordpress/blocks';
import { __ } from '@wordpress/i18n';
import { moment } from '@wordpress/date';
import deprecated from '@wordpress/deprecated';
Expand Down Expand Up @@ -1487,7 +1487,7 @@ export const getInserterItems = createSelector(
return false;
}

const referencedBlock = getBlock( state, sharedBlock.uid );
const referencedBlock = getParsedSharedBlock( state, sharedBlock.id );
if ( ! referencedBlock ) {
return false;
}
Expand All @@ -1507,7 +1507,7 @@ export const getInserterItems = createSelector(
const buildSharedBlockInserterItem = ( sharedBlock ) => {
const id = `core/block/${ sharedBlock.id }`;

const referencedBlock = getBlock( state, sharedBlock.uid );
const referencedBlock = getParsedSharedBlock( state, sharedBlock.id );
const referencedBlockType = getBlockType( referencedBlock.name );

const { time, count = 0 } = getInsertUsage( state, id ) || {};
Expand Down Expand Up @@ -1608,6 +1608,28 @@ export const getSharedBlock = createSelector(
],
);

/**
* Returns the parsed block saved as shared block with the given ID.
*
* @param {Object} state Global application state.
* @param {number|string} ref The shared block's ID.
*
* @return {Object} The parsed block.
*/
export const getParsedSharedBlock = createSelector(
( state, ref ) => {
const sharedBlock = getSharedBlock( state, ref );
if ( ! sharedBlock ) {
return null;
}

return parse( sharedBlock.content )[ 0 ];
},
( state, ref ) => [
get( state.sharedBlocks.data, [ ref, 'content' ] ),
],
);

/**
* Returns whether or not the shared block with the given ID is being saved.
*
Expand Down
Loading