From 99f8a2b94854f5649fe304c7c2f897bcf29abddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Gomes?= Date: Mon, 26 Nov 2018 11:44:32 +0000 Subject: [PATCH 1/2] Improve typing performance by splitting attributes in state tree. Attributes have been moved to `attributesByClientId` in order to reduce the impact of typing throughout the state tree. See #11782. Review fixes pass #1 Simplify block flattening code in reducers. Remove alignment toolbar optimization; should be a different PR Fix minor bug in test Fix failing getBlockDependantsCacheBust test Remove new `*WithoutAttributes` selectors. We'll go with a different approach: use the existing selectors, but keep the dependencies as they are. The attributes may get stale, but it doesn't matter if they're not being used. Change attributesById structure to not have an inner attributes Simplifying some selector dependencies Simplify withSaveReusableBlock a bit further and clarify its existence Reuse constant instead of running omit again Remove no longer needed mapClientIds Simplifying reducers after review comments. Further changes to selectors after review Fix types in JSDoc Renaming attributesByClientId to attributes Further selector fixes after review --- packages/editor/src/store/reducer.js | 213 ++++++--- packages/editor/src/store/selectors.js | 30 +- packages/editor/src/store/test/reducer.js | 68 ++- packages/editor/src/store/test/selectors.js | 485 +++++++++++++++----- 4 files changed, 601 insertions(+), 195 deletions(-) diff --git a/packages/editor/src/store/reducer.js b/packages/editor/src/store/reducer.js index 385e99e8be5366..dd198c47201819 100644 --- a/packages/editor/src/store/reducer.js +++ b/packages/editor/src/store/reducer.js @@ -13,6 +13,7 @@ import { omitBy, keys, isEqual, + isEmpty, overSome, get, } from 'lodash'; @@ -78,29 +79,52 @@ function mapBlockOrder( blocks, rootClientId = '' ) { } /** - * Given an array of blocks, returns an object containing all blocks, recursing - * into inner blocks. Keys correspond to the block client ID, the value of - * which is the block object. + * Helper method to iterate through all blocks, recursing into inner blocks, + * applying a transformation function to each one. + * Returns a flattened object with the transformed blocks. * * @param {Array} blocks Blocks to flatten. + * @param {Function} transform Transforming function to be applied to each block. * - * @return {Object} Flattened blocks object. + * @return {Object} Flattened object. */ -function getFlattenedBlocks( blocks ) { - const flattenedBlocks = {}; +function flattenBlocks( blocks, transform ) { + const result = {}; const stack = [ ...blocks ]; while ( stack.length ) { - // `innerBlocks` is redundant data which can fall out of sync, since - // this is reflected in `blocks.order`, so exclude from appended block. const { innerBlocks, ...block } = stack.shift(); - stack.push( ...innerBlocks ); - - flattenedBlocks[ block.clientId ] = block; + result[ block.clientId ] = transform( block ); } - return flattenedBlocks; + return result; +} + +/** + * Given an array of blocks, returns an object containing all blocks, without + * attributes, recursing into inner blocks. Keys correspond to the block client + * ID, the value of which is the attributes object. + * + * @param {Array} blocks Blocks to flatten. + * + * @return {Object} Flattened block attributes object. + */ +function getFlattenedBlocksWithoutAttributes( blocks ) { + return flattenBlocks( blocks, ( block ) => omit( block, 'attributes' ) ); +} + +/** + * Given an array of blocks, returns an object containing all block attributes, + * recursing into inner blocks. Keys correspond to the block client ID, the + * value of which is the attributes object. + * + * @param {Array} blocks Blocks to flatten. + * + * @return {Object} Flattened block attributes object. + */ +function getFlattenedBlockAttributes( blocks ) { + return flattenBlocks( blocks, ( block ) => block.attributes ); } /** @@ -252,7 +276,11 @@ const withBlockReset = ( reducer ) => ( state, action ) => { ...state, byClientId: { ...omit( state.byClientId, visibleClientIds ), - ...getFlattenedBlocks( action.blocks ), + ...getFlattenedBlocksWithoutAttributes( action.blocks ), + }, + attributes: { + ...omit( state.attributes, visibleClientIds ), + ...getFlattenedBlockAttributes( action.blocks ), }, order: { ...omit( state.order, visibleClientIds ), @@ -264,6 +292,43 @@ const withBlockReset = ( reducer ) => ( state, action ) => { return reducer( state, action ); }; +/** + * Higher-order reducer which targets the combined blocks reducer and handles + * the `SAVE_REUSABLE_BLOCK_SUCCESS` action. This action can't be handled by + * regular reducers and needs a higher-order reducer since it needs access to + * both `byClientId` and `attributes` simultaneously. + * + * @param {Function} reducer Original reducer function. + * + * @return {Function} Enhanced reducer function. + */ +const withSaveReusableBlock = ( reducer ) => ( state, action ) => { + if ( state && action.type === 'SAVE_REUSABLE_BLOCK_SUCCESS' ) { + const { id, updatedId } = action; + + // If a temporary reusable block is saved, we swap the temporary id with the final one + if ( id === updatedId ) { + return state; + } + + state = { ...state }; + + state.attributes = mapValues( state.attributes, ( attributes, clientId ) => { + const { name } = state.byClientId[ clientId ]; + if ( name === 'core/block' && attributes.ref === id ) { + return { + ...attributes, + ref: updatedId, + }; + } + + return attributes; + } ); + } + + return reducer( state, action ); +}; + /** * Undoable reducer returning the editor post state, including blocks parsed * from current HTML markup. @@ -342,6 +407,8 @@ export const editor = flow( [ withBlockReset, + withSaveReusableBlock, + // Track whether changes exist, resetting at each post save. Relies on // editor initialization firing post reset as an effect. withChangeDetection( { @@ -352,48 +419,71 @@ export const editor = flow( [ byClientId( state = {}, action ) { switch ( action.type ) { case 'SETUP_EDITOR_STATE': - return getFlattenedBlocks( action.blocks ); + return getFlattenedBlocksWithoutAttributes( action.blocks ); case 'RECEIVE_BLOCKS': return { ...state, - ...getFlattenedBlocks( action.blocks ), + ...getFlattenedBlocksWithoutAttributes( action.blocks ), }; - case 'UPDATE_BLOCK_ATTRIBUTES': + case 'UPDATE_BLOCK': // Ignore updates if block isn't known if ( ! state[ action.clientId ] ) { return state; } - // Consider as updates only changed values - const nextAttributes = reduce( action.attributes, ( result, value, key ) => { - if ( value !== result[ key ] ) { - result = getMutateSafeObject( state[ action.clientId ].attributes, result ); - result[ key ] = value; - } - - return result; - }, state[ action.clientId ].attributes ); - - // Skip update if nothing has been changed. The reference will - // match the original block if `reduce` had no changed values. - if ( nextAttributes === state[ action.clientId ].attributes ) { + // Do nothing if only attributes change. + const changes = omit( action.updates, 'attributes' ); + if ( isEmpty( changes ) ) { return state; } - // Otherwise merge attributes into state return { ...state, [ action.clientId ]: { ...state[ action.clientId ], - attributes: nextAttributes, + ...changes, }, }; + case 'INSERT_BLOCKS': + return { + ...state, + ...getFlattenedBlocksWithoutAttributes( action.blocks ), + }; + + case 'REPLACE_BLOCKS': + if ( ! action.blocks ) { + return state; + } + + return { + ...omit( state, action.clientIds ), + ...getFlattenedBlocksWithoutAttributes( action.blocks ), + }; + + case 'REMOVE_BLOCKS': + return omit( state, action.clientIds ); + } + + return state; + }, + + attributes( state = {}, action ) { + switch ( action.type ) { + case 'SETUP_EDITOR_STATE': + return getFlattenedBlockAttributes( action.blocks ); + + case 'RECEIVE_BLOCKS': + return { + ...state, + ...getFlattenedBlockAttributes( action.blocks ), + }; + case 'UPDATE_BLOCK': - // Ignore updates if block isn't known - if ( ! state[ action.clientId ] ) { + // Ignore updates if block isn't known or there are no attribute changes. + if ( ! state[ action.clientId ] || ! action.updates.attributes ) { return state; } @@ -401,14 +491,42 @@ export const editor = flow( [ ...state, [ action.clientId ]: { ...state[ action.clientId ], - ...action.updates, + ...action.updates.attributes, }, }; + case 'UPDATE_BLOCK_ATTRIBUTES': + // Ignore updates if block isn't known + if ( ! state[ action.clientId ] ) { + return state; + } + + // Consider as updates only changed values + const nextAttributes = reduce( action.attributes, ( result, value, key ) => { + if ( value !== result[ key ] ) { + result = getMutateSafeObject( state[ action.clientId ], result ); + result[ key ] = value; + } + + return result; + }, state[ action.clientId ] ); + + // Skip update if nothing has been changed. The reference will + // match the original block if `reduce` had no changed values. + if ( nextAttributes === state[ action.clientId ] ) { + return state; + } + + // Otherwise replace attributes in state + return { + ...state, + [ action.clientId ]: nextAttributes, + }; + case 'INSERT_BLOCKS': return { ...state, - ...getFlattenedBlocks( action.blocks ), + ...getFlattenedBlockAttributes( action.blocks ), }; case 'REPLACE_BLOCKS': @@ -418,34 +536,11 @@ export const editor = flow( [ return { ...omit( state, action.clientIds ), - ...getFlattenedBlocks( action.blocks ), + ...getFlattenedBlockAttributes( action.blocks ), }; case 'REMOVE_BLOCKS': return omit( state, action.clientIds ); - - case 'SAVE_REUSABLE_BLOCK_SUCCESS': { - const { id, updatedId } = action; - - // If a temporary reusable block is saved, we swap the temporary id with the final one - if ( id === updatedId ) { - return state; - } - - return mapValues( state, ( block ) => { - if ( block.name === 'core/block' && block.attributes.ref === id ) { - return { - ...block, - attributes: { - ...block.attributes, - ref: updatedId, - }, - }; - } - - return block; - } ); - } } return state; diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index d8b019930d1a07..4298f7b7d4b3e9 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -642,7 +642,7 @@ export const getBlockAttributes = createSelector( return null; } - let { attributes } = block; + let attributes = state.editor.present.blocks.attributes[ clientId ]; // Inject custom source attribute values. // @@ -667,6 +667,7 @@ export const getBlockAttributes = createSelector( }, ( state, clientId ) => [ state.editor.present.blocks.byClientId[ clientId ], + state.editor.present.blocks.attributes[ clientId ], state.editor.present.edits.meta, state.initialEdits.meta, state.currentPost.meta, @@ -698,9 +699,8 @@ export const getBlock = createSelector( }; }, ( state, clientId ) => [ - state.editor.present.blocks.byClientId[ clientId ], - getBlockDependantsCacheBust( state, clientId ), ...getBlockAttributes.getDependants( state, clientId ), + getBlockDependantsCacheBust( state, clientId ), ] ); @@ -729,9 +729,14 @@ export const getBlocks = createSelector( ( clientId ) => getBlock( state, clientId ) ); }, - ( state ) => [ - state.editor.present.blocks, - ] + ( state, rootClientId ) => { + if ( ! rootClientId ) { + return [ + state.editor.present.blocks, + ]; + } + return getBlock.getDependants( state, rootClientId ); + } ); /** @@ -1125,10 +1130,8 @@ export const getMultiSelectedBlocks = createSelector( return multiSelectedBlockClientIds.map( ( clientId ) => getBlock( state, clientId ) ); }, ( state ) => [ - state.editor.present.blocks.order, - state.blockSelection.start, - state.blockSelection.end, - state.editor.present.blocks.byClientId, + ...getMultiSelectedBlockClientIds.getDependants( state ), + state.editor.present.blocks, state.editor.present.edits.meta, state.initialEdits.meta, state.currentPost.meta, @@ -1178,7 +1181,7 @@ const isAncestorOf = createSelector( return possibleAncestorId === idToCheck; }, ( state ) => [ - state.editor.present.blocks, + state.editor.present.blocks.order, ], ); @@ -1985,7 +1988,8 @@ export const getInserterItems = createSelector( }, ( state, rootClientId ) => [ state.blockListSettings[ rootClientId ], - state.editor.present.blocks, + state.editor.present.blocks.byClientId, + state.editor.present.blocks.order, state.preferences.insertUsage, state.settings.allowedBlockTypes, state.settings.templateLock, @@ -2019,7 +2023,7 @@ export const hasInserterItems = createSelector( }, ( state, rootClientId ) => [ state.blockListSettings[ rootClientId ], - state.editor.present.blocks, + state.editor.present.blocks.byClientId, state.settings.allowedBlockTypes, state.settings.templateLock, state.reusableBlocks.data, diff --git a/packages/editor/src/store/test/reducer.js b/packages/editor/src/store/test/reducer.js index 884314fe553098..e9c1db43e53e8f 100644 --- a/packages/editor/src/store/test/reducer.js +++ b/packages/editor/src/store/test/reducer.js @@ -489,9 +489,12 @@ describe( 'state', () => { expect( state.present.blocks.byClientId.chicken ).toEqual( { clientId: 'chicken', name: 'core/test-block', - attributes: { content: 'ribs' }, isValid: true, } ); + + expect( state.present.blocks.attributes.chicken ).toEqual( { + content: 'ribs', + } ); } ); it( 'should update the reusable block reference if the temporary id is swapped', () => { @@ -517,11 +520,12 @@ describe( 'state', () => { expect( state.present.blocks.byClientId.chicken ).toEqual( { clientId: 'chicken', name: 'core/block', - attributes: { - ref: 3, - }, isValid: false, } ); + + expect( state.present.blocks.attributes.chicken ).toEqual( { + ref: 3, + } ); } ); it( 'should move the block up', () => { @@ -790,9 +794,11 @@ describe( 'state', () => { ribs: { clientId: 'ribs', name: 'core/test-block', - attributes: {}, }, } ); + expect( state.present.blocks.attributes ).toEqual( { + ribs: {}, + } ); } ); it( 'should remove multiple blocks', () => { @@ -827,9 +833,11 @@ describe( 'state', () => { ribs: { clientId: 'ribs', name: 'core/test-block', - attributes: {}, }, } ); + expect( state.present.blocks.attributes ).toEqual( { + ribs: {}, + } ); } ); it( 'should cascade remove to include inner blocks', () => { @@ -1211,6 +1219,46 @@ describe( 'state', () => { } ); describe( 'byClientId', () => { + it( 'should ignore updates to non-existent block', () => { + const original = deepFreeze( editor( undefined, { + type: 'RESET_BLOCKS', + blocks: [], + } ) ); + const state = editor( original, { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: 'kumquat', + attributes: { + updated: true, + }, + } ); + + expect( state.present.blocks.byClientId ).toBe( original.present.blocks.byClientId ); + } ); + + it( 'should return with same reference if no changes in updates', () => { + const original = deepFreeze( editor( undefined, { + type: 'RESET_BLOCKS', + blocks: [ { + clientId: 'kumquat', + attributes: { + updated: true, + }, + innerBlocks: [], + } ], + } ) ); + const state = editor( original, { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: 'kumquat', + attributes: { + updated: true, + }, + } ); + + expect( state.present.blocks.byClientId ).toBe( state.present.blocks.byClientId ); + } ); + } ); + + describe( 'attributes', () => { it( 'should return with attribute block updates', () => { const original = deepFreeze( editor( undefined, { type: 'RESET_BLOCKS', @@ -1228,7 +1276,7 @@ describe( 'state', () => { }, } ); - expect( state.present.blocks.byClientId.kumquat.attributes.updated ).toBe( true ); + expect( state.present.blocks.attributes.kumquat.updated ).toBe( true ); } ); it( 'should accumulate attribute block updates', () => { @@ -1250,7 +1298,7 @@ describe( 'state', () => { }, } ); - expect( state.present.blocks.byClientId.kumquat.attributes ).toEqual( { + expect( state.present.blocks.attributes.kumquat ).toEqual( { updated: true, moreUpdated: true, } ); @@ -1269,7 +1317,7 @@ describe( 'state', () => { }, } ); - expect( state.present.blocks.byClientId ).toBe( original.present.blocks.byClientId ); + expect( state.present.blocks.attributes ).toBe( original.present.blocks.attributes ); } ); it( 'should return with same reference if no changes in updates', () => { @@ -1291,7 +1339,7 @@ describe( 'state', () => { }, } ); - expect( state.present.blocks.byClientId ).toBe( state.present.blocks.byClientId ); + expect( state.present.blocks.attributes ).toBe( state.present.blocks.attributes ); } ); } ); } ); diff --git a/packages/editor/src/store/test/selectors.js b/packages/editor/src/store/test/selectors.js index 6303dde765a920..cfe66ed4c43c25 100644 --- a/packages/editor/src/store/test/selectors.js +++ b/packages/editor/src/store/test/selectors.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { filter, without } from 'lodash'; +import { filter, without, omit } from 'lodash'; /** * WordPress dependencies @@ -1150,6 +1150,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -1169,6 +1170,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -1192,6 +1194,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -1213,6 +1216,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -1238,9 +1242,11 @@ describe( 'selectors', () => { clientId: 123, name: 'core/test-block-a', isValid: true, - attributes: { - text: '', - }, + }, + }, + attributes: { + 123: { + text: '', }, }, order: { @@ -1267,9 +1273,11 @@ describe( 'selectors', () => { 123: { clientId: 123, name: 'core/test-freeform', - attributes: { - content: '', - }, + }, + }, + attributes: { + 123: { + content: '', }, }, order: { @@ -1297,9 +1305,11 @@ describe( 'selectors', () => { clientId: 123, name: 'core/test-freeform', isValid: true, - attributes: { - content: '', - }, + }, + }, + attributes: { + 123: { + content: '', }, }, order: { @@ -1327,6 +1337,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -1353,6 +1364,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -1498,6 +1510,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -1520,9 +1533,11 @@ describe( 'selectors', () => { clientId: 123, name: 'core/test-block-a', isValid: true, - attributes: { - text: '', - }, + }, + }, + attributes: { + 123: { + text: '', }, }, order: { @@ -1549,9 +1564,11 @@ describe( 'selectors', () => { clientId: 123, name: 'core/test-block-a', isValid: true, - attributes: { - text: '', - }, + }, + }, + attributes: { + 123: { + text: '', }, }, order: { @@ -1578,6 +1595,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -1598,6 +1616,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: { @@ -1622,9 +1641,11 @@ describe( 'selectors', () => { clientId: 123, name: 'core/test-freeform', isValid: true, - attributes: { - content: '', - }, + }, + }, + attributes: { + 123: { + content: '', }, }, order: { @@ -1651,9 +1672,11 @@ describe( 'selectors', () => { clientId: 123, name: 'core/test-freeform', isValid: true, - attributes: { - content: '', - }, + }, + }, + attributes: { + 123: { + content: '', }, }, order: { @@ -1682,9 +1705,11 @@ describe( 'selectors', () => { clientId: 123, name: 'core/test-freeform', isValid: true, - attributes: { - content: 'Test Data', - }, + }, + }, + attributes: { + 123: { + content: 'Test Data', }, }, order: { @@ -1713,17 +1738,19 @@ describe( 'selectors', () => { clientId: 123, name: 'core/test-freeform', isValid: true, - attributes: { - content: '', - }, }, 456: { clientId: 456, name: 'core/test-freeform', isValid: true, - attributes: { - content: '', - }, + }, + }, + attributes: { + 123: { + content: '', + }, + 456: { + content: '', }, }, order: { @@ -1866,7 +1893,8 @@ describe( 'selectors', () => { } ); describe( 'getBlockDependantsCacheBust', () => { - const rootBlock = { clientId: 123, name: 'core/paragraph', attributes: {} }; + const rootBlock = { clientId: 123, name: 'core/paragraph' }; + const rootBlockAttributes = {}; const rootOrder = [ 123 ]; it( 'returns an unchanging reference', () => { @@ -1880,6 +1908,9 @@ describe( 'selectors', () => { byClientId: { 123: rootBlock, }, + attributes: { + 123: rootBlockAttributes, + }, order: { '': rootOrder, 123: rootBlockOrder, @@ -1899,6 +1930,9 @@ describe( 'selectors', () => { byClientId: { 123: rootBlock, }, + attributes: { + 123: rootBlockAttributes, + }, order: { '': rootOrder, 123: rootBlockOrder, @@ -1924,6 +1958,9 @@ describe( 'selectors', () => { byClientId: { 123: rootBlock, }, + attributes: { + 123: rootBlockAttributes, + }, order: { '': rootOrder, 123: [], @@ -1942,7 +1979,11 @@ describe( 'selectors', () => { blocks: { byClientId: { 123: rootBlock, - 456: { clientId: 456, name: 'core/paragraph', attributes: {} }, + 456: { clientId: 456, name: 'core/paragraph' }, + }, + attributes: { + 123: rootBlockAttributes, + 456: {}, }, order: { '': rootOrder, @@ -1963,7 +2004,8 @@ describe( 'selectors', () => { it( 'returns an unchanging reference on unchanging inner block', () => { const rootBlockOrder = [ 456 ]; - const childBlock = { clientId: 456, name: 'core/paragraph', attributes: {} }; + const childBlock = { clientId: 456, name: 'core/paragraph' }; + const childBlockAttributes = {}; const childBlockOrder = []; const state = { @@ -1975,6 +2017,10 @@ describe( 'selectors', () => { 123: rootBlock, 456: childBlock, }, + attributes: { + 123: rootBlockAttributes, + 456: childBlockAttributes, + }, order: { '': rootOrder, 123: rootBlockOrder, @@ -1996,6 +2042,10 @@ describe( 'selectors', () => { 123: rootBlock, 456: childBlock, }, + attributes: { + 123: rootBlockAttributes, + 456: childBlockAttributes, + }, order: { '': rootOrder, 123: rootBlockOrder, @@ -2024,7 +2074,11 @@ describe( 'selectors', () => { blocks: { byClientId: { 123: rootBlock, - 456: { clientId: 456, name: 'core/paragraph', attributes: {} }, + 456: { clientId: 456, name: 'core/paragraph' }, + }, + attributes: { + 123: rootBlockAttributes, + 456: {}, }, order: { '': rootOrder, @@ -2045,7 +2099,11 @@ describe( 'selectors', () => { blocks: { byClientId: { 123: rootBlock, - 456: { clientId: 456, name: 'core/paragraph', attributes: { content: [ 'foo' ] } }, + 456: { clientId: 456, name: 'core/paragraph' }, + }, + attributes: { + 123: rootBlockAttributes, + 456: { content: [ 'foo' ] }, }, order: { '': rootOrder, @@ -2066,7 +2124,8 @@ describe( 'selectors', () => { it( 'returns a new reference on updated grandchild inner block', () => { const rootBlockOrder = [ 456 ]; - const childBlock = { clientId: 456, name: 'core/paragraph', attributes: {} }; + const childBlock = { clientId: 456, name: 'core/paragraph' }; + const childBlockAttributes = {}; const childBlockOrder = [ 789 ]; const grandChildBlockOrder = []; @@ -2078,7 +2137,12 @@ describe( 'selectors', () => { byClientId: { 123: rootBlock, 456: childBlock, - 789: { clientId: 789, name: 'core/paragraph', attributes: {} }, + 789: { clientId: 789, name: 'core/paragraph' }, + }, + attributes: { + 123: rootBlockAttributes, + 456: childBlockAttributes, + 789: {}, }, order: { '': rootOrder, @@ -2101,7 +2165,12 @@ describe( 'selectors', () => { byClientId: { 123: rootBlock, 456: childBlock, - 789: { clientId: 789, name: 'core/paragraph', attributes: { content: [ 'foo' ] } }, + 789: { clientId: 789, name: 'core/paragraph' }, + }, + attributes: { + 123: rootBlockAttributes, + 456: childBlockAttributes, + 789: { content: [ 'foo' ] }, }, order: { '': rootOrder, @@ -2130,6 +2199,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -2153,9 +2223,11 @@ describe( 'selectors', () => { 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1': { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1', name: 'core/paragraph', - attributes: {}, }, }, + attributes: { + 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1': {}, + }, order: { '': [ 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' ], 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1': [], @@ -2181,7 +2253,10 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 123: { clientId: 123, name: 'core/paragraph', attributes: {} }, + 123: { clientId: 123, name: 'core/paragraph' }, + }, + attributes: { + 123: {}, }, order: { '': [ 123 ], @@ -2209,6 +2284,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -2227,8 +2303,12 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 123: { clientId: 123, name: 'core/paragraph', attributes: {} }, - 456: { clientId: 456, name: 'core/paragraph', attributes: {} }, + 123: { clientId: 123, name: 'core/paragraph' }, + 456: { clientId: 456, name: 'core/paragraph' }, + }, + attributes: { + 123: {}, + 456: {}, }, order: { '': [ 123 ], @@ -2279,7 +2359,10 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 123: { clientId: 123, name: 'core/meta-block', attributes: {} }, + 123: { clientId: 123, name: 'core/meta-block' }, + }, + attributes: { + 123: {}, }, order: { '': [ 123 ], @@ -2313,8 +2396,12 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 23: { clientId: 23, name: 'core/heading', attributes: {} }, - 123: { clientId: 123, name: 'core/paragraph', attributes: {} }, + 23: { clientId: 23, name: 'core/heading' }, + 123: { clientId: 123, name: 'core/paragraph' }, + }, + attributes: { + 23: {}, + 123: {}, }, order: { '': [ 123, 23 ], @@ -2341,21 +2428,38 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 'uuid-2': { clientId: 'uuid-2', name: 'core/image', attributes: {} }, - 'uuid-4': { clientId: 'uuid-4', name: 'core/paragraph', attributes: {} }, - 'uuid-6': { clientId: 'uuid-6', name: 'core/paragraph', attributes: {} }, - 'uuid-8': { clientId: 'uuid-8', name: 'core/block', attributes: {} }, - 'uuid-10': { clientId: 'uuid-10', name: 'core/columns', attributes: {} }, - 'uuid-12': { clientId: 'uuid-12', name: 'core/column', attributes: {} }, - 'uuid-14': { clientId: 'uuid-14', name: 'core/column', attributes: {} }, - 'uuid-16': { clientId: 'uuid-16', name: 'core/quote', attributes: {} }, - 'uuid-18': { clientId: 'uuid-18', name: 'core/block', attributes: {} }, - 'uuid-20': { clientId: 'uuid-20', name: 'core/gallery', attributes: {} }, - 'uuid-22': { clientId: 'uuid-22', name: 'core/block', attributes: {} }, - 'uuid-24': { clientId: 'uuid-24', name: 'core/columns', attributes: {} }, - 'uuid-26': { clientId: 'uuid-26', name: 'core/column', attributes: {} }, - 'uuid-28': { clientId: 'uuid-28', name: 'core/column', attributes: {} }, - 'uuid-30': { clientId: 'uuid-30', name: 'core/paragraph', attributes: {} }, + 'uuid-2': { clientId: 'uuid-2', name: 'core/image' }, + 'uuid-4': { clientId: 'uuid-4', name: 'core/paragraph' }, + 'uuid-6': { clientId: 'uuid-6', name: 'core/paragraph' }, + 'uuid-8': { clientId: 'uuid-8', name: 'core/block' }, + 'uuid-10': { clientId: 'uuid-10', name: 'core/columns' }, + 'uuid-12': { clientId: 'uuid-12', name: 'core/column' }, + 'uuid-14': { clientId: 'uuid-14', name: 'core/column' }, + 'uuid-16': { clientId: 'uuid-16', name: 'core/quote' }, + 'uuid-18': { clientId: 'uuid-18', name: 'core/block' }, + 'uuid-20': { clientId: 'uuid-20', name: 'core/gallery' }, + 'uuid-22': { clientId: 'uuid-22', name: 'core/block' }, + 'uuid-24': { clientId: 'uuid-24', name: 'core/columns' }, + 'uuid-26': { clientId: 'uuid-26', name: 'core/column' }, + 'uuid-28': { clientId: 'uuid-28', name: 'core/column' }, + 'uuid-30': { clientId: 'uuid-30', name: 'core/paragraph' }, + }, + attributes: { + 'uuid-2': {}, + 'uuid-4': {}, + 'uuid-6': {}, + 'uuid-8': {}, + 'uuid-10': {}, + 'uuid-12': {}, + 'uuid-14': {}, + 'uuid-16': {}, + 'uuid-18': {}, + 'uuid-20': {}, + 'uuid-22': {}, + 'uuid-24': {}, + 'uuid-26': {}, + 'uuid-28': {}, + 'uuid-30': {}, }, order: { '': [ 'uuid-6', 'uuid-8', 'uuid-10', 'uuid-22' ], @@ -2401,21 +2505,38 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 'uuid-2': { clientId: 'uuid-2', name: 'core/image', attributes: {} }, - 'uuid-4': { clientId: 'uuid-4', name: 'core/paragraph', attributes: {} }, - 'uuid-6': { clientId: 'uuid-6', name: 'core/paragraph', attributes: {} }, - 'uuid-8': { clientId: 'uuid-8', name: 'core/block', attributes: {} }, - 'uuid-10': { clientId: 'uuid-10', name: 'core/columns', attributes: {} }, - 'uuid-12': { clientId: 'uuid-12', name: 'core/column', attributes: {} }, - 'uuid-14': { clientId: 'uuid-14', name: 'core/column', attributes: {} }, - 'uuid-16': { clientId: 'uuid-16', name: 'core/quote', attributes: {} }, - 'uuid-18': { clientId: 'uuid-18', name: 'core/block', attributes: {} }, - 'uuid-20': { clientId: 'uuid-20', name: 'core/gallery', attributes: {} }, - 'uuid-22': { clientId: 'uuid-22', name: 'core/block', attributes: {} }, - 'uuid-24': { clientId: 'uuid-24', name: 'core/columns', attributes: {} }, - 'uuid-26': { clientId: 'uuid-26', name: 'core/column', attributes: {} }, - 'uuid-28': { clientId: 'uuid-28', name: 'core/column', attributes: {} }, - 'uuid-30': { clientId: 'uuid-30', name: 'core/paragraph', attributes: {} }, + 'uuid-2': { clientId: 'uuid-2', name: 'core/image' }, + 'uuid-4': { clientId: 'uuid-4', name: 'core/paragraph' }, + 'uuid-6': { clientId: 'uuid-6', name: 'core/paragraph' }, + 'uuid-8': { clientId: 'uuid-8', name: 'core/block' }, + 'uuid-10': { clientId: 'uuid-10', name: 'core/columns' }, + 'uuid-12': { clientId: 'uuid-12', name: 'core/column' }, + 'uuid-14': { clientId: 'uuid-14', name: 'core/column' }, + 'uuid-16': { clientId: 'uuid-16', name: 'core/quote' }, + 'uuid-18': { clientId: 'uuid-18', name: 'core/block' }, + 'uuid-20': { clientId: 'uuid-20', name: 'core/gallery' }, + 'uuid-22': { clientId: 'uuid-22', name: 'core/block' }, + 'uuid-24': { clientId: 'uuid-24', name: 'core/columns' }, + 'uuid-26': { clientId: 'uuid-26', name: 'core/column' }, + 'uuid-28': { clientId: 'uuid-28', name: 'core/column' }, + 'uuid-30': { clientId: 'uuid-30', name: 'core/paragraph' }, + }, + attributes: { + 'uuid-2': {}, + 'uuid-4': {}, + 'uuid-6': {}, + 'uuid-8': {}, + 'uuid-10': {}, + 'uuid-12': {}, + 'uuid-14': {}, + 'uuid-16': {}, + 'uuid-18': {}, + 'uuid-20': {}, + 'uuid-22': {}, + 'uuid-24': {}, + 'uuid-26': {}, + 'uuid-28': {}, + 'uuid-30': {}, }, order: { '': [ 'uuid-6', 'uuid-8', 'uuid-10', 'uuid-22' ], @@ -2464,8 +2585,12 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 23: { clientId: 23, name: 'core/heading', attributes: {} }, - 123: { clientId: 123, name: 'core/paragraph', attributes: {} }, + 23: { clientId: 23, name: 'core/heading' }, + 123: { clientId: 123, name: 'core/paragraph' }, + }, + attributes: { + 23: {}, + 123: {}, }, order: { '': [ 123, 23 ], @@ -2484,9 +2609,14 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 123: { clientId: 123, name: 'core/columns', attributes: {} }, - 456: { clientId: 456, name: 'core/paragraph', attributes: {} }, - 789: { clientId: 789, name: 'core/paragraph', attributes: {} }, + 123: { clientId: 123, name: 'core/columns' }, + 456: { clientId: 456, name: 'core/paragraph' }, + 789: { clientId: 789, name: 'core/paragraph' }, + }, + attributes: { + 123: {}, + 456: {}, + 789: {}, }, order: { '': [ 123 ], @@ -2542,9 +2672,14 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 123: { clientId: 123, name: 'core/heading', attributes: {} }, - 456: { clientId: 456, name: 'core/paragraph', attributes: {} }, - 789: { clientId: 789, name: 'core/paragraph', attributes: {} }, + 123: { clientId: 123, name: 'core/heading' }, + 456: { clientId: 456, name: 'core/paragraph' }, + 789: { clientId: 789, name: 'core/paragraph' }, + }, + attributes: { + 123: {}, + 456: {}, + 789: {}, }, order: { '': [ 123, 456 ], @@ -2568,6 +2703,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, }, @@ -2613,8 +2749,12 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 23: { clientId: 23, name: 'core/heading', attributes: {} }, - 123: { clientId: 123, name: 'core/paragraph', attributes: {} }, + 23: { clientId: 23, name: 'core/heading' }, + 123: { clientId: 123, name: 'core/paragraph' }, + }, + attributes: { + 23: {}, + 123: {}, }, order: { '': [ 23, 123 ], @@ -2639,8 +2779,12 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 23: { clientId: 23, name: 'core/heading', attributes: {} }, - 123: { clientId: 123, name: 'core/paragraph', attributes: {} }, + 23: { clientId: 23, name: 'core/heading' }, + 123: { clientId: 123, name: 'core/paragraph' }, + }, + attributes: { + 23: {}, + 123: {}, }, order: { '': [ 23, 123 ], @@ -2665,8 +2809,12 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 23: { clientId: 23, name: 'core/heading', attributes: {} }, - 123: { clientId: 123, name: 'core/paragraph', attributes: {} }, + 23: { clientId: 23, name: 'core/heading' }, + 123: { clientId: 123, name: 'core/paragraph' }, + }, + attributes: { + 23: {}, + 123: {}, }, order: { '': [ 23, 123 ], @@ -2835,6 +2983,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -3433,6 +3582,10 @@ describe( 'selectors', () => { clientId1: { clientId: 'clientId1' }, clientId2: { clientId: 'clientId2' }, }, + attributes: { + clientId1: {}, + clientId2: {}, + }, order: { '': [ 'clientId1' ], clientId1: [ 'clientId2' ], @@ -3469,6 +3622,9 @@ describe( 'selectors', () => { byClientId: { clientId1: { clientId: 'clientId1' }, }, + attributes: { + clientId1: {}, + }, order: { '': [ 'clientId1' ], clientId1: [], @@ -3502,6 +3658,10 @@ describe( 'selectors', () => { clientId1: { clientId: 'clientId1' }, clientId2: { clientId: 'clientId2' }, }, + attributes: { + clientId1: {}, + clientId2: {}, + }, order: { '': [ 'clientId1' ], clientId1: [ 'clientId2' ], @@ -3536,6 +3696,10 @@ describe( 'selectors', () => { clientId1: { clientId: 'clientId1' }, clientId2: { clientId: 'clientId2' }, }, + attributes: { + clientId1: {}, + clientId2: {}, + }, order: { '': [ 'clientId1', 'clientId2' ], clientId1: [], @@ -3570,6 +3734,10 @@ describe( 'selectors', () => { clientId1: { clientId: 'clientId1' }, clientId2: { clientId: 'clientId2' }, }, + attributes: { + clientId1: {}, + clientId2: {}, + }, order: { '': [ 'clientId1', 'clientId2' ], clientId1: [], @@ -3684,6 +3852,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -3702,8 +3871,12 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 123: { clientId: 123, name: 'core/image', attributes: {} }, - 456: { clientId: 456, name: 'core/quote', attributes: {} }, + 123: { clientId: 123, name: 'core/image' }, + 456: { clientId: 456, name: 'core/quote' }, + }, + attributes: { + 123: {}, + 456: {}, }, order: { '': [ 123, 456 ], @@ -3725,7 +3898,10 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 123: { clientId: 123, name: 'core/image', attributes: {} }, + 123: { clientId: 123, name: 'core/image' }, + }, + attributes: { + 123: {}, }, order: { '': [ 123 ], @@ -3747,7 +3923,10 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 456: { clientId: 456, name: 'core/quote', attributes: {} }, + 456: { clientId: 456, name: 'core/quote' }, + }, + attributes: { + 456: {}, }, order: { '': [ 456 ], @@ -3769,7 +3948,10 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 567: { clientId: 567, name: 'core-embed/youtube', attributes: {} }, + 567: { clientId: 567, name: 'core-embed/youtube' }, + }, + attributes: { + 567: {}, }, order: { '': [ 567 ], @@ -3791,8 +3973,12 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - 456: { clientId: 456, name: 'core/quote', attributes: {} }, - 789: { clientId: 789, name: 'core/paragraph', attributes: {} }, + 456: { clientId: 456, name: 'core/quote' }, + 789: { clientId: 789, name: 'core/paragraph' }, + }, + attributes: { + 456: {}, + 789: {}, }, order: { '': [ 456, 789 ], @@ -3844,7 +4030,10 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - [ block.clientId ]: block, + [ block.clientId ]: omit( block, 'attributes' ), + }, + attributes: { + [ block.clientId ]: block.attributes, }, order: { '': [ block.clientId ], @@ -3872,7 +4061,10 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - [ block.clientId ]: block, + [ block.clientId ]: omit( block, 'attributes' ), + }, + attributes: { + [ block.clientId ]: block.attributes, }, order: { '': [ block.clientId ], @@ -3899,7 +4091,10 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - [ unknownBlock.clientId ]: unknownBlock, + [ unknownBlock.clientId ]: omit( unknownBlock, 'attributes' ), + }, + attributes: { + [ unknownBlock.clientId ]: unknownBlock.attributes, }, order: { '': [ unknownBlock.clientId ], @@ -3929,8 +4124,12 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - [ firstUnknown.clientId ]: firstUnknown, - [ secondUnknown.clientId ]: secondUnknown, + [ firstUnknown.clientId ]: omit( firstUnknown, 'attributes' ), + [ secondUnknown.clientId ]: omit( secondUnknown, 'attributes' ), + }, + attributes: { + [ firstUnknown.clientId ]: firstUnknown.attributes, + [ secondUnknown.clientId ]: secondUnknown.attributes, }, order: { '': [ firstUnknown.clientId, secondUnknown.clientId ], @@ -3955,7 +4154,10 @@ describe( 'selectors', () => { present: { blocks: { byClientId: { - [ defaultBlock.clientId ]: defaultBlock, + [ defaultBlock.clientId ]: omit( defaultBlock, 'attributes' ), + }, + attributes: { + [ defaultBlock.clientId ]: defaultBlock.attributes, }, order: { '': [ defaultBlock.clientId ], @@ -3981,11 +4183,13 @@ describe( 'selectors', () => { blocks: { byClientId: { [ defaultBlock.clientId ]: { - ...defaultBlock, - attributes: { - ...defaultBlock.attributes, - modified: true, - }, + ...omit( defaultBlock, 'attributes' ), + }, + }, + attributes: { + [ defaultBlock.clientId ]: { + ...defaultBlock.attributes, + modified: true, }, }, order: { @@ -4012,6 +4216,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, }, }, }, @@ -4027,6 +4232,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, }, }, }, @@ -4044,6 +4250,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, }, }, }, @@ -4061,6 +4268,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, }, }, }, @@ -4078,6 +4286,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, }, }, }, @@ -4095,6 +4304,9 @@ describe( 'selectors', () => { byClientId: { block1: { name: 'core/test-block-a' }, }, + attributes: { + block1: {}, + }, }, }, }, @@ -4112,6 +4324,9 @@ describe( 'selectors', () => { byClientId: { block1: { name: 'core/test-block-b' }, }, + attributes: { + block1: {}, + }, }, }, }, @@ -4129,6 +4344,9 @@ describe( 'selectors', () => { byClientId: { block1: { name: 'core/test-block-a' }, }, + attributes: { + block1: {}, + }, }, }, }, @@ -4150,6 +4368,9 @@ describe( 'selectors', () => { byClientId: { block1: { name: 'core/test-block-a' }, }, + attributes: { + block1: {}, + }, }, }, }, @@ -4171,6 +4392,9 @@ describe( 'selectors', () => { byClientId: { block1: { name: 'core/test-block-b' }, }, + attributes: { + block1: {}, + }, }, }, }, @@ -4194,6 +4418,9 @@ describe( 'selectors', () => { byClientId: { block1: { name: 'core/test-block-a' }, }, + attributes: { + block1: {}, + }, order: {}, }, edits: {}, @@ -4255,12 +4482,18 @@ describe( 'selectors', () => { block1ref: { name: 'core/block', clientId: 'block1ref', + }, + itselfBlock1: { name: 'core/test-block-a' }, + itselfBlock2: { name: 'core/test-block-b' }, + }, + attributes: { + block1ref: { attributes: { ref: 1, }, }, - itselfBlock1: { name: 'core/test-block-a' }, - itselfBlock2: { name: 'core/test-block-b' }, + itselfBlock1: {}, + itselfBlock2: {}, }, order: { '': [ 'block1ref' ], @@ -4311,15 +4544,23 @@ describe( 'selectors', () => { block2ref: { name: 'core/block', clientId: 'block1ref', - attributes: { - ref: 2, - }, }, referredBlock1: { name: 'core/test-block-a' }, referredBlock2: { name: 'core/test-block-b' }, childReferredBlock2: { name: 'core/test-block-a' }, grandchildReferredBlock2: { name: 'core/test-block-b' }, }, + attributes: { + block2ref: { + attributes: { + ref: 2, + }, + }, + referredBlock1: {}, + referredBlock2: {}, + childReferredBlock2: {}, + grandchildReferredBlock2: {}, + }, order: { '': [ 'block2ref' ], referredBlock2: [ 'childReferredBlock2' ], @@ -4370,6 +4611,10 @@ describe( 'selectors', () => { block1: { name: 'core/test-block-a' }, block2: { name: 'core/test-block-a' }, }, + attributes: { + block1: {}, + block2: {}, + }, order: {}, }, edits: {}, @@ -4413,6 +4658,12 @@ describe( 'selectors', () => { block3: { name: 'core/test-block-a' }, block4: { name: 'core/test-block-a' }, }, + attributes: { + block1: {}, + block2: {}, + block3: {}, + block4: {}, + }, order: { '': [ 'block3', 'block4' ], }, @@ -4477,6 +4728,9 @@ describe( 'selectors', () => { byClientId: { block1: { clientId: 'block1', name: 'core/test-block-b' }, }, + attributes: { + block1: { attribute: {} }, + }, order: { '': [ 'block1' ], }, @@ -4506,6 +4760,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -4533,6 +4788,7 @@ describe( 'selectors', () => { present: { blocks: { byClientId: {}, + attributes: {}, order: {}, }, edits: {}, @@ -4565,6 +4821,9 @@ describe( 'selectors', () => { byClientId: { block1: { name: 'core/test-block-b' }, }, + attributes: { + block1: { attribute: {} }, + }, order: { '': [ 'block1' ], }, From cdd96ca183fca1dd15b946812c34746e079b6c7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Gomes?= Date: Tue, 4 Dec 2018 18:33:16 +0000 Subject: [PATCH 2/2] Revert changes to getBlocks dependants --- packages/editor/src/store/selectors.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index 4298f7b7d4b3e9..23455621d24027 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -729,14 +729,7 @@ export const getBlocks = createSelector( ( clientId ) => getBlock( state, clientId ) ); }, - ( state, rootClientId ) => { - if ( ! rootClientId ) { - return [ - state.editor.present.blocks, - ]; - } - return getBlock.getDependants( state, rootClientId ); - } + ( state ) => [ state.editor.present.blocks ] ); /**