From 6eb84dfaa8244600cc726afb6d78f7a95a56a3c5 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Tue, 22 Mar 2022 14:26:46 +0800 Subject: [PATCH] Migrate block editor insert usage to preferences store Update tests Make updateInsertUsage a proper action that can be unit tested Fix reusable block tests Update test Try fixing private actions with new store registration Update tests Add special handling for the insertUsage migration as it was performed later than others Remove unused function Add mark next change as expensive action to preferences store Update debounce function to handle a longer debounce for expensive changes Mark the insertUsage preference change as expensive Make expensive calls use a trailing edge debounce Fix duplicate keys in tests Improve trailing edge test Fix tests, and ensure options object is optional Make updateInsertUsage a private API Make markNextChangeAsExpensive a private API Update docs Update package-lock Opt-in preferences as a core module using private apis Do not unlock what is already unlocked Remove time property from INSERT_BLOCKS and REPLACE_BLOCKS action objects Rename `match` to `variation` Rename file to create-async-debouncer Add an extra param for defining the debounce of expensive requests Add a default value of `false` for the `isExpensive` option Make `__unstableGetInsertUsage` a private selector called `getInsertUsage`. Remove `__unstableGetInsertUsageForBlock` Only run the migration when needed Keep the `insertUsage` data at a maximum of 100 records Fix linting issues Update docs --- packages/block-editor/src/store/actions.js | 5 +- packages/block-editor/src/store/defaults.js | 4 - .../block-editor/src/store/defaults.native.js | 7 +- packages/block-editor/src/store/index.js | 20 +-- .../block-editor/src/store/private-actions.js | 70 ++++++++ .../src/store/private-selectors.js | 19 ++ packages/block-editor/src/store/reducer.js | 59 +------ packages/block-editor/src/store/selectors.js | 28 +-- .../block-editor/src/store/test/actions.js | 87 +++++++++- .../src/store/test/private-actions.js | 162 ++++++++++++++++++ .../block-editor/src/store/test/reducer.js | 142 --------------- .../block-editor/src/store/test/selectors.js | 37 ++-- packages/preferences-persistence/README.md | 1 + ...nce-async.js => create-async-debouncer.js} | 18 +- .../src/create/index.js | 72 ++++---- ...nce-async.js => create-async-debouncer.js} | 86 +++++++--- packages/preferences-persistence/src/index.js | 32 +++- .../legacy-local-storage-data/index.js | 27 +-- .../legacy-local-storage-data/test/index.js | 162 ++++++++++++++---- packages/preferences/src/store/index.js | 3 + .../preferences/src/store/private-actions.js | 12 ++ packages/preferences/src/store/reducer.js | 10 +- .../preferences/src/store/test/reducer.js | 41 ++++- packages/private-apis/src/implementation.js | 1 + .../reusable-blocks/src/store/test/actions.js | 2 + 25 files changed, 723 insertions(+), 384 deletions(-) rename packages/preferences-persistence/src/create/{debounce-async.js => create-async-debouncer.js} (80%) rename packages/preferences-persistence/src/create/test/{debounce-async.js => create-async-debouncer.js} (56%) create mode 100644 packages/preferences/src/store/private-actions.js diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 170c5192f3bee..3d302a2b33d71 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -373,11 +373,11 @@ export const replaceBlocks = type: 'REPLACE_BLOCKS', clientIds, blocks, - time: Date.now(), indexToSelect, initialPosition, meta, } ); + dispatch.updateInsertUsage( blocks ); // To avoid a focus loss when removing the last block, assure there is // always a default block if the last of the blocks have been removed. dispatch.ensureDefaultBlock(); @@ -572,12 +572,12 @@ export const insertBlocks = } } if ( allowedBlocks.length ) { + dispatch.updateInsertUsage( blocks ); dispatch( { type: 'INSERT_BLOCKS', blocks: allowedBlocks, index, rootClientId, - time: Date.now(), updateSelection, initialPosition: updateSelection ? initialPosition : null, meta, @@ -1394,7 +1394,6 @@ export function replaceInnerBlocks( blocks, updateSelection, initialPosition: updateSelection ? initialPosition : null, - time: Date.now(), }; } diff --git a/packages/block-editor/src/store/defaults.js b/packages/block-editor/src/store/defaults.js index 54877eafc3690..78fb35b112ef0 100644 --- a/packages/block-editor/src/store/defaults.js +++ b/packages/block-editor/src/store/defaults.js @@ -3,10 +3,6 @@ */ import { __, _x } from '@wordpress/i18n'; -export const PREFERENCES_DEFAULTS = { - insertUsage: {}, -}; - /** * The default editor settings * diff --git a/packages/block-editor/src/store/defaults.native.js b/packages/block-editor/src/store/defaults.native.js index e951c39b3d1a4..44ee33c36e8e9 100644 --- a/packages/block-editor/src/store/defaults.native.js +++ b/packages/block-editor/src/store/defaults.native.js @@ -1,10 +1,7 @@ /** * Internal dependencies */ -import { - PREFERENCES_DEFAULTS, - SETTINGS_DEFAULTS as SETTINGS, -} from './defaults.js'; +import { SETTINGS_DEFAULTS as SETTINGS } from './defaults.js'; const SETTINGS_DEFAULTS = { ...SETTINGS, @@ -20,4 +17,4 @@ const SETTINGS_DEFAULTS = { }, }; -export { PREFERENCES_DEFAULTS, SETTINGS_DEFAULTS }; +export { SETTINGS_DEFAULTS }; diff --git a/packages/block-editor/src/store/index.js b/packages/block-editor/src/store/index.js index 0bcc00cb5f6ae..181139aad69e6 100644 --- a/packages/block-editor/src/store/index.js +++ b/packages/block-editor/src/store/index.js @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { createReduxStore, registerStore } from '@wordpress/data'; +import { createReduxStore, register } from '@wordpress/data'; /** * Internal dependencies @@ -32,24 +32,8 @@ export const storeConfig = { */ export const store = createReduxStore( STORE_NAME, { ...storeConfig, - persist: [ 'preferences' ], } ); -// We will be able to use the `register` function once we switch -// the "preferences" persistence to use the new preferences package. -const registeredStore = registerStore( STORE_NAME, { - ...storeConfig, - persist: [ 'preferences' ], -} ); -unlock( registeredStore ).registerPrivateActions( privateActions ); -unlock( registeredStore ).registerPrivateSelectors( privateSelectors ); - -// TODO: Remove once we switch to the `register` function (see above). -// -// Until then, private functions also need to be attached to the original -// `store` descriptor in order to avoid unit tests failing, which could happen -// when tests create new registries in which they register stores. -// -// @see https://github.com/WordPress/gutenberg/pull/51145#discussion_r1239999590 unlock( store ).registerPrivateActions( privateActions ); unlock( store ).registerPrivateSelectors( privateSelectors ); +register( store ); diff --git a/packages/block-editor/src/store/private-actions.js b/packages/block-editor/src/store/private-actions.js index 74aec3c49d1e8..1dee5999e9df5 100644 --- a/packages/block-editor/src/store/private-actions.js +++ b/packages/block-editor/src/store/private-actions.js @@ -1,7 +1,9 @@ /** * WordPress dependencies */ +import { store as blocksStore } from '@wordpress/blocks'; import { Platform } from '@wordpress/element'; +import { store as preferencesStore } from '@wordpress/preferences'; /** * Internal dependencies @@ -382,3 +384,71 @@ export const modifyContentLockBlock = focusModeToRevert ); }; + +/** + * Updates the inserter usage statistics in the preferences store. + * + * Note: this function is an internal and not intended to ever be made + * non-private. + * + * @param {Array} blocks The array of blocks that were inserted. + */ +export const updateInsertUsage = + ( blocks ) => + ( { registry } ) => { + const previousInsertUsage = + registry.select( preferencesStore ).get( 'core', 'insertUsage' ) ?? + {}; + + const time = Date.now(); + + let updatedInsertUsage = blocks.reduce( ( previousState, block ) => { + const { attributes, name: blockName } = block; + let id = blockName; + const variation = registry + .select( blocksStore ) + .getActiveBlockVariation( blockName, attributes ); + + if ( variation?.name ) { + id += '/' + variation.name; + } + + if ( blockName === 'core/block' ) { + id += '/' + attributes.ref; + } + + const previousCount = previousState?.[ id ]?.count ?? 0; + + return { + ...previousState, + [ id ]: { + time, + count: previousCount + 1, + }, + }; + }, previousInsertUsage ); + + // Ensure the list of blocks doesn't grow above `limit` items. + // This is to ensure the preferences store data doesn't grow too big + // given it's persisted in the database. + const limit = 100; + const entries = Object.entries( updatedInsertUsage ); + if ( entries.length > limit ) { + // Most recently inserted blocks first. + entries.sort( ( entryLeft, entryRight ) => { + const [ , { time: timeLeft } ] = entryLeft; + const [ , { time: timeRight } ] = entryRight; + return timeRight - timeLeft; + } ); + + // Slice an array of items that are the newest and convert them + // back into object form. + const entriesToKeep = entries.slice( 0, limit ); + updatedInsertUsage = Object.fromEntries( entriesToKeep ); + } + + registry.dispatch( preferencesStore ); + registry + .dispatch( preferencesStore ) + .set( 'core', 'insertUsage', updatedInsertUsage ); + }; diff --git a/packages/block-editor/src/store/private-selectors.js b/packages/block-editor/src/store/private-selectors.js index c534c65b8defe..0f876ca02e854 100644 --- a/packages/block-editor/src/store/private-selectors.js +++ b/packages/block-editor/src/store/private-selectors.js @@ -2,6 +2,7 @@ * WordPress dependencies */ import { createSelector, createRegistrySelector } from '@wordpress/data'; +import { store as preferencesStore } from '@wordpress/preferences'; /** * Internal dependencies @@ -31,6 +32,8 @@ import { export { getBlockSettings } from './get-block-settings'; +const EMPTY_OBJECT = {}; + /** * Returns true if the block interface is hidden, or false otherwise. * @@ -511,3 +514,19 @@ export function getTemporarilyEditingAsBlocks( state ) { export function getTemporarilyEditingFocusModeToRevert( state ) { return state.temporarilyEditingFocusModeRevert; } + +/** + * Return all insert usage stats. + * + * This is only exported since registry selectors need to be exported. It's marked + * as unstable so that it's not considered part of the public API. + * + * @return {Object} An object with an `id` key representing the type + * of block and an object value that contains + * block insertion statistics. + */ +export const getInsertUsage = createRegistrySelector( + ( registrySelect ) => () => + registrySelect( preferencesStore ).get( 'core', 'insertUsage' ) ?? + EMPTY_OBJECT +); diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 7c83887876919..1e8658e287d59 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -7,12 +7,12 @@ import fastDeepEqual from 'fast-deep-equal/es6'; * WordPress dependencies */ import { pipe } from '@wordpress/compose'; -import { combineReducers, select } from '@wordpress/data'; -import { store as blocksStore } from '@wordpress/blocks'; +import { combineReducers } from '@wordpress/data'; + /** * Internal dependencies */ -import { PREFERENCES_DEFAULTS, SETTINGS_DEFAULTS } from './defaults'; +import { SETTINGS_DEFAULTS } from './defaults'; import { insertAt, moveTo } from './array'; const identity = ( x ) => x; @@ -1676,58 +1676,6 @@ export function settings( state = SETTINGS_DEFAULTS, action ) { return state; } -/** - * Reducer returning the user preferences. - * - * @param {Object} state Current state. - * @param {Object} action Dispatched action. - * - * @return {string} Updated state. - */ -export function preferences( state = PREFERENCES_DEFAULTS, action ) { - switch ( action.type ) { - case 'INSERT_BLOCKS': - case 'REPLACE_BLOCKS': { - const nextInsertUsage = action.blocks.reduce( - ( prevUsage, block ) => { - const { attributes, name: blockName } = block; - let id = blockName; - // If a block variation match is found change the name to be the same with the - // one that is used for block variations in the Inserter (`getItemFromVariation`). - const match = select( blocksStore ).getActiveBlockVariation( - blockName, - attributes - ); - if ( match?.name ) { - id += '/' + match.name; - } - if ( blockName === 'core/block' ) { - id += '/' + attributes.ref; - } - - return { - ...prevUsage, - [ id ]: { - time: action.time, - count: prevUsage[ id ] - ? prevUsage[ id ].count + 1 - : 1, - }, - }; - }, - state.insertUsage - ); - - return { - ...state, - insertUsage: nextInsertUsage, - }; - } - } - - return state; -} - /** * Reducer returning an object where each key is a block client ID, its value * representing the settings for its nested blocks. @@ -2083,7 +2031,6 @@ const combinedReducers = combineReducers( { insertionPoint, template, settings, - preferences, lastBlockAttributesChange, lastFocus, editorMode, diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index d78d75e4210c8..b0b4ba0d17271 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -34,6 +34,7 @@ import { unlock } from '../lock-unlock'; import { getContentLockingParent, + getInsertUsage, getTemporarilyEditingAsBlocks, getTemporarilyEditingFocusModeToRevert, } from './private-selectors'; @@ -1819,20 +1820,6 @@ export function canLockBlockType( state, nameOrType ) { return !! state.settings?.canLockBlocks; } -/** - * Returns information about how recently and frequently a block has been inserted. - * - * @param {Object} state Global application state. - * @param {string} id A string which identifies the insert, e.g. 'core/block/12' - * - * @return {?{ time: number, count: number }} An object containing `time` which is when the last - * insert occurred as a UNIX epoch, and `count` which is - * the number of inserts that have occurred. - */ -function getInsertUsage( state, id ) { - return state.preferences.insertUsage?.[ id ] ?? null; -} - /** * Returns whether we can show a block type in the inserter * @@ -1859,7 +1846,8 @@ const canIncludeBlockTypeInInserter = ( state, blockType, rootClientId ) => { */ const getItemFromVariation = ( state, item ) => ( variation ) => { const variationId = `${ item.id }/${ variation.name }`; - const { time, count = 0 } = getInsertUsage( state, variationId ) || {}; + const insertUsage = getInsertUsage(); + const { time, count = 0 } = insertUsage?.[ variationId ] ?? {}; return { ...item, id: variationId, @@ -1934,7 +1922,8 @@ const buildBlockTypeItem = ).some( ( { name } ) => name === blockType.name ); } - const { time, count = 0 } = getInsertUsage( state, id ) || {}; + const insertUsage = getInsertUsage(); + const { time, count = 0 } = insertUsage?.[ id ] ?? {}; const blockItemBase = { id, name: blockType.name, @@ -2003,7 +1992,8 @@ export const getInserterItems = createRegistrySelector( ( select ) => } : symbol; const id = `core/block/${ reusableBlock.id }`; - const { time, count = 0 } = getInsertUsage( state, id ) || {}; + const insertUsage = getInsertUsage(); + const { time, count = 0 } = insertUsage?.[ id ] ?? {}; const frecency = calculateFrecency( time, count ); return { @@ -2147,7 +2137,7 @@ export const getInserterItems = createRegistrySelector( ( select ) => getBlockTypes(), unlock( select( STORE_NAME ) ).getReusableBlocks(), state.blocks.order, - state.preferences.insertUsage, + getInsertUsage(), ...getInsertBlockTypeDependants( state, rootClientId ), ] ) @@ -2214,7 +2204,7 @@ export const getBlockTransformItems = createSelector( }, ( state, blocks, rootClientId ) => [ getBlockTypes(), - state.preferences.insertUsage, + getInsertUsage(), ...getInsertBlockTypeDependants( state, rootClientId ), ] ); diff --git a/packages/block-editor/src/store/test/actions.js b/packages/block-editor/src/store/test/actions.js index f960363cdb0dd..68d6644f5107b 100644 --- a/packages/block-editor/src/store/test/actions.js +++ b/packages/block-editor/src/store/test/actions.js @@ -219,6 +219,7 @@ describe( 'actions', () => { }; const dispatch = jest.fn(); dispatch.ensureDefaultBlock = jest.fn(); + dispatch.updateInsertUsage = jest.fn(); const registry = createRegistry(); replaceBlock( 'chicken', block )( { select, dispatch, registry } ); @@ -260,6 +261,8 @@ describe( 'actions', () => { }, }; const dispatch = jest.fn(); + dispatch.ensureDefaultBlock = jest.fn(); + dispatch.updateInsertUsage = jest.fn(); replaceBlocks( [ 'chicken' ], blocks )( { select, dispatch } ); @@ -286,6 +289,7 @@ describe( 'actions', () => { }; const dispatch = jest.fn(); dispatch.ensureDefaultBlock = jest.fn(); + dispatch.updateInsertUsage = jest.fn(); const registry = createRegistry(); replaceBlocks( @@ -297,8 +301,9 @@ describe( 'actions', () => { type: 'REPLACE_BLOCKS', clientIds: [ 'chicken' ], blocks, - time: expect.any( Number ), initialPosition: 0, + indexToSelect: undefined, + meta: undefined, } ); } ); @@ -324,6 +329,7 @@ describe( 'actions', () => { }; const dispatch = jest.fn(); dispatch.ensureDefaultBlock = jest.fn(); + dispatch.updateInsertUsage = jest.fn(); const registry = createRegistry(); replaceBlocks( @@ -338,12 +344,44 @@ describe( 'actions', () => { type: 'REPLACE_BLOCKS', clientIds: [ 'chicken' ], blocks, - time: expect.any( Number ), indexToSelect: null, initialPosition: null, meta: { patternName: 'core/chicken-ribs-pattern' }, } ); } ); + + it( 'should set insertUsage in the preferences store', () => { + const blocks = [ + { + clientId: 'ribs', + name: 'core/test-ribs', + }, + { + clientId: 'chicken', + name: 'core/test-chicken', + }, + ]; + + const select = { + getSettings: () => null, + getBlockRootClientId: () => null, + canInsertBlockType: () => true, + getBlockCount: () => 1, + }; + + const dispatch = jest.fn(); + dispatch.ensureDefaultBlock = jest.fn(); + dispatch.updateInsertUsage = jest.fn(); + + replaceBlocks( + [ 'pineapple' ], + blocks, + null, + null + )( { select, dispatch } ); + + expect( dispatch.updateInsertUsage ).toHaveBeenCalledWith( blocks ); + } ); } ); describe( 'insertBlock', () => { @@ -358,7 +396,9 @@ describe( 'actions', () => { getSettings: () => null, canInsertBlockType: () => true, }; - const dispatch = jest.fn(); + const dispatch = Object.assign( jest.fn(), { + updateInsertUsage: () => {}, + } ); insertBlock( block, @@ -372,9 +412,9 @@ describe( 'actions', () => { blocks: [ block ], index, rootClientId: 'testclientid', - time: expect.any( Number ), updateSelection: true, initialPosition: 0, + meta: undefined, } ); } ); } ); @@ -411,6 +451,8 @@ describe( 'actions', () => { }, }; const dispatch = jest.fn(); + dispatch.ensureDefaultBlock = jest.fn(); + dispatch.updateInsertUsage = jest.fn(); insertBlocks( blocks, @@ -424,9 +466,9 @@ describe( 'actions', () => { blocks: [ ribsBlock, chickenRibsBlock ], index: 5, rootClientId: 'testrootid', - time: expect.any( Number ), updateSelection: false, initialPosition: null, + meta: undefined, } ); } ); @@ -446,6 +488,7 @@ describe( 'actions', () => { canInsertBlockType: () => false, }; const dispatch = jest.fn(); + dispatch.updateInsertUsage = jest.fn(); insertBlocks( blocks, @@ -489,6 +532,7 @@ describe( 'actions', () => { }, }; const dispatch = jest.fn(); + dispatch.updateInsertUsage = jest.fn(); insertBlocks( blocks, @@ -504,12 +548,41 @@ describe( 'actions', () => { blocks: [ ribsBlock, chickenRibsBlock ], index: 5, rootClientId: 'testrootid', - time: expect.any( Number ), updateSelection: false, initialPosition: null, meta: { patternName: 'core/chicken-ribs-pattern' }, } ); } ); + + it( 'should set insertUsage in the preferences store', () => { + const blocks = [ + { + clientId: 'ribs', + name: 'core/test-ribs', + }, + { + clientId: 'chicken', + name: 'core/test-chicken', + }, + ]; + + const select = { + getSettings: () => null, + canInsertBlockType: () => true, + }; + const dispatch = jest.fn(); + dispatch.updateInsertUsage = jest.fn(); + + insertBlocks( + blocks, + 5, + 'testrootid', + false, + 0 + )( { select, dispatch } ); + + expect( dispatch.updateInsertUsage ).toHaveBeenCalledWith( blocks ); + } ); } ); describe( 'showInsertionPoint', () => { @@ -783,7 +856,6 @@ describe( 'actions', () => { type: 'REPLACE_INNER_BLOCKS', blocks: [ block ], rootClientId: 'root', - time: expect.any( Number ), updateSelection: false, initialPosition: null, } ); @@ -794,7 +866,6 @@ describe( 'actions', () => { type: 'REPLACE_INNER_BLOCKS', blocks: [ block ], rootClientId: 'root', - time: expect.any( Number ), updateSelection: true, initialPosition: 0, } ); diff --git a/packages/block-editor/src/store/test/private-actions.js b/packages/block-editor/src/store/test/private-actions.js index 7576b95866306..856d840f3cbe5 100644 --- a/packages/block-editor/src/store/test/private-actions.js +++ b/packages/block-editor/src/store/test/private-actions.js @@ -5,6 +5,7 @@ import { hideBlockInterface, showBlockInterface, expandBlock, + updateInsertUsage, __experimentalUpdateSettings, setOpenedBlockSettingsMenu, startDragging, @@ -28,6 +29,167 @@ describe( 'private actions', () => { } ); } ); + describe( 'updateInsertUsage', () => { + it( 'should record recently used blocks', () => { + const setPreference = jest.fn(); + const registry = { + dispatch: () => ( { + set: setPreference, + markNextChangeAsExpensive: () => {}, + } ), + select: () => ( { + get: () => {}, + getActiveBlockVariation: () => {}, + } ), + }; + + updateInsertUsage( [ + { + clientId: 'bacon', + name: 'core/embed', + }, + ] )( { registry } ); + + expect( setPreference ).toHaveBeenCalledWith( + 'core', + 'insertUsage', + { + 'core/embed': { + time: expect.any( Number ), + count: 1, + }, + } + ); + } ); + + it( 'merges insert usage if more blocks are added of the same type', () => { + const setPreference = jest.fn(); + const registry = { + dispatch: () => ( { + set: setPreference, + markNextChangeAsExpensive: () => {}, + } ), + select: () => ( { + // simulate an existing embed block. + get: () => ( { + 'core/embed': { + time: 123456, + count: 1, + }, + } ), + getActiveBlockVariation: () => {}, + } ), + }; + + updateInsertUsage( [ + { + clientId: 'eggs', + name: 'core/embed', + }, + { + clientId: 'bacon', + name: 'core/block', + attributes: { ref: 123 }, + }, + ] )( { registry } ); + + expect( setPreference ).toHaveBeenCalledWith( + 'core', + 'insertUsage', + { + // The reusable block has a special case where each ref is + // stored as though an individual block, and the ref is + // also recorded in the `insert` object. + 'core/block/123': { + time: expect.any( Number ), + count: 1, + }, + 'core/embed': { + time: expect.any( Number ), + count: 2, + }, + } + ); + } ); + + describe( 'block variations handling', () => { + const blockWithVariations = 'core/test-block-with-variations'; + + it( 'should return proper results with both found or not found block variation matches', () => { + const setPreference = jest.fn(); + const registry = { + dispatch: () => ( { + set: setPreference, + markNextChangeAsExpensive: () => {}, + } ), + select: () => ( { + get: () => {}, + // simulate an active block variation: + // - 'apple' when the fruit attribute is 'apple'. + // - 'orange' when the fruit attribute is 'orange'. + getActiveBlockVariation: ( + blockName, + { fruit } = {} + ) => { + if ( blockName === blockWithVariations ) { + if ( fruit === 'orange' ) { + return { name: 'orange' }; + } + if ( fruit === 'apple' ) { + return { name: 'apple' }; + } + } + }, + } ), + }; + + updateInsertUsage( [ + { + clientId: 'no match', + name: blockWithVariations, + }, + { + clientId: 'not a variation match', + name: blockWithVariations, + attributes: { fruit: 'not in a variation' }, + }, + { + clientId: 'orange', + name: blockWithVariations, + attributes: { fruit: 'orange' }, + }, + { + clientId: 'apple', + name: blockWithVariations, + attributes: { fruit: 'apple' }, + }, + ] )( { registry } ); + + const orangeVariationName = `${ blockWithVariations }/orange`; + const appleVariationName = `${ blockWithVariations }/apple`; + + expect( setPreference ).toHaveBeenCalledWith( + 'core', + 'insertUsage', + { + [ orangeVariationName ]: { + time: expect.any( Number ), + count: 1, + }, + [ appleVariationName ]: { + time: expect.any( Number ), + count: 1, + }, + [ blockWithVariations ]: { + time: expect.any( Number ), + count: 2, + }, + } + ); + } ); + } ); + } ); + describe( '__experimentalUpdateSettings', () => { const experimentalSettings = { inserterMediaCategories: 'foo', diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index cd472fa59ac72..67408622357e7 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -26,7 +26,6 @@ import { selection, initialPosition, isMultiSelecting, - preferences, blocksMode, insertionPoint, template, @@ -2885,147 +2884,6 @@ describe( 'state', () => { } ); } ); - describe( 'preferences()', () => { - it( 'should apply all defaults', () => { - const state = preferences( undefined, {} ); - - expect( state ).toEqual( { - insertUsage: {}, - } ); - } ); - it( 'should record recently used blocks', () => { - const state = preferences( deepFreeze( { insertUsage: {} } ), { - type: 'INSERT_BLOCKS', - blocks: [ - { - clientId: 'bacon', - name: 'core/embed', - }, - ], - time: 123456, - } ); - - expect( state ).toEqual( { - insertUsage: { - 'core/embed': { - time: 123456, - count: 1, - }, - }, - } ); - - const twoRecentBlocks = preferences( - deepFreeze( { - insertUsage: { - 'core/embed': { - time: 123456, - count: 1, - }, - }, - } ), - { - type: 'INSERT_BLOCKS', - blocks: [ - { - clientId: 'eggs', - name: 'core/embed', - }, - { - clientId: 'bacon', - name: 'core/block', - attributes: { ref: 123 }, - }, - ], - time: 123457, - } - ); - - expect( twoRecentBlocks ).toEqual( { - insertUsage: { - 'core/embed': { - time: 123457, - count: 2, - }, - 'core/block/123': { - time: 123457, - count: 1, - }, - }, - } ); - } ); - describe( 'block variations handling', () => { - const blockWithVariations = 'core/test-block-with-variations'; - beforeAll( () => { - const variations = [ - { - name: 'apple', - attributes: { fruit: 'apple' }, - }, - { name: 'orange', attributes: { fruit: 'orange' } }, - ].map( ( variation ) => ( { - ...variation, - isActive: ( blockAttributes, variationAttributes ) => - blockAttributes?.fruit === variationAttributes.fruit, - } ) ); - registerBlockType( blockWithVariations, { - save: noop, - edit: noop, - title: 'Fruit with variations', - variations, - } ); - } ); - afterAll( () => { - unregisterBlockType( blockWithVariations ); - } ); - it( 'should return proper results with both found or not found block variation matches', () => { - const state = preferences( deepFreeze( { insertUsage: {} } ), { - type: 'INSERT_BLOCKS', - blocks: [ - { - clientId: 'no match', - name: blockWithVariations, - }, - { - clientId: 'not a variation match', - name: blockWithVariations, - attributes: { fruit: 'not in a variation' }, - }, - { - clientId: 'orange', - name: blockWithVariations, - attributes: { fruit: 'orange' }, - }, - { - clientId: 'apple', - name: blockWithVariations, - attributes: { fruit: 'apple' }, - }, - ], - time: 123456, - } ); - - const orangeVariationName = `${ blockWithVariations }/orange`; - const appleVariationName = `${ blockWithVariations }/apple`; - expect( state ).toEqual( { - insertUsage: expect.objectContaining( { - [ orangeVariationName ]: { - time: 123456, - count: 1, - }, - [ appleVariationName ]: { - time: 123456, - count: 1, - }, - [ blockWithVariations ]: { - time: 123456, - count: 2, - }, - } ), - } ); - } ); - } ); - } ); - describe( 'blocksMode', () => { it( 'should set mode to html if not set', () => { const action = { diff --git a/packages/block-editor/src/store/test/selectors.js b/packages/block-editor/src/store/test/selectors.js index 85006621c4701..e06d56f56dc69 100644 --- a/packages/block-editor/src/store/test/selectors.js +++ b/packages/block-editor/src/store/test/selectors.js @@ -15,6 +15,7 @@ import { select, dispatch } from '@wordpress/data'; */ import * as selectors from '../selectors'; import { store } from '../'; +import { getInsertUsage } from '../private-selectors'; const { getBlockName, @@ -73,6 +74,11 @@ const { getBlockEditingMode, } = selectors; +jest.mock( '../private-selectors', () => ( { + ...jest.requireActual( '../private-selectors' ), + getInsertUsage: jest.fn(), +} ) ); + describe( 'selectors', () => { let cachedSelectors; @@ -3287,14 +3293,9 @@ describe( 'selectors', () => { } ); describe( 'getInserterItems', () => { - afterAll( async () => { - await dispatch( store ).updateSettings( { - __experimentalReusableBlocks: [], - } ); - await dispatch( store ).resetBlocks( [] ); - } ); - it( 'should properly list block type and reusable block items', async () => { + getInsertUsage.mockImplementation( () => ( {} ) ); + await dispatch( store ).updateSettings( { __experimentalReusableBlocks: [ { @@ -3348,6 +3349,10 @@ describe( 'selectors', () => { } ); it( 'should correctly cache the return values', async () => { + // Define the empty object here to simulate that the preferences + // store won't return a new object every time. + const EMPTY_OBJECT = {}; + getInsertUsage.mockImplementation( () => EMPTY_OBJECT ); await dispatch( store ).updateSettings( { __experimentalReusableBlocks: [ { @@ -3424,6 +3429,7 @@ describe( 'selectors', () => { } ); it( 'should set isDisabled when a block with `multiple: false` has been used', async () => { + getInsertUsage.mockImplementation( () => ( {} ) ); await dispatch( store ).resetBlocks( [ { clientId: 'block1', @@ -3439,16 +3445,9 @@ describe( 'selectors', () => { } ); it( 'should set a frecency', async () => { - for ( let i = 0; i < 10; i++ ) { - await dispatch( store ).insertBlocks( [ - { - clientId: 'block1', - name: 'core/test-block-b', - innerBlocks: [], - }, - ] ); - } - + getInsertUsage.mockImplementation( () => ( { + 'core/test-block-b': { count: 10, time: 1000 }, + } ) ); const items = select( store ).getInserterItems(); const reusableBlock2Item = items.find( ( item ) => item.id === 'core/test-block-b' @@ -3686,6 +3685,10 @@ describe( 'selectors', () => { ); } ); it( 'should set frecency', () => { + getInsertUsage.mockImplementation( () => ( { + 'core/with-tranforms-a': { count: 10, time: 1000 }, + } ) ); + const state = { blocks: { byClientId: new Map(), diff --git a/packages/preferences-persistence/README.md b/packages/preferences-persistence/README.md index af8f6cc4b1438..7137eb0083b1c 100644 --- a/packages/preferences-persistence/README.md +++ b/packages/preferences-persistence/README.md @@ -42,6 +42,7 @@ _Parameters_ - _options.preloadedData_ `?Object`: Any persisted preferences data that should be preloaded. When set, the persistence layer will avoid fetching data from the REST API. - _options.localStorageRestoreKey_ `?string`: The key to use for restoring the localStorage backup, used when the persistence layer calls `localStorage.getItem` or `localStorage.setItem`. - _options.requestDebounceMS_ `?number`: Debounce requests to the API so that they only occur at minimum every `requestDebounceMS` milliseconds, and don't swamp the server. Defaults to 2500ms. +- _options.expensiveRequestDebounceMS_ `?number`: A longer debounce that can be defined for updates that have `isExpensive=true` defined. defaults to 60000ms. _Returns_ diff --git a/packages/preferences-persistence/src/create/debounce-async.js b/packages/preferences-persistence/src/create/create-async-debouncer.js similarity index 80% rename from packages/preferences-persistence/src/create/debounce-async.js rename to packages/preferences-persistence/src/create/create-async-debouncer.js index 89fb4c9b30705..4788c6eec9378 100644 --- a/packages/preferences-persistence/src/create/debounce-async.js +++ b/packages/preferences-persistence/src/create/create-async-debouncer.js @@ -9,23 +9,21 @@ * This is distinct from `{ debounce } from @wordpress/compose` in that it * waits for promise resolution. * - * @param {Function} func A function that returns a promise. - * @param {number} delayMS A delay in milliseconds. - * - * @return {Function} A function that debounce whatever function is passed - * to it. + * @return {Function} A function that debounces the async function passed to it + * in the first parameter by the time passed in the second + * options parameter. */ -export default function debounceAsync( func, delayMS ) { +export default function createAsyncDebouncer() { let timeoutId; let activePromise; - return async function debounced( ...args ) { + return async function debounced( func, { delayMS, isTrailing = false } ) { // This is a leading edge debounce. If there's no promise or timeout // in progress, call the debounced function immediately. - if ( ! activePromise && ! timeoutId ) { + if ( ! isTrailing && ! activePromise && ! timeoutId ) { return new Promise( ( resolve, reject ) => { // Keep a reference to the promise. - activePromise = func( ...args ) + activePromise = func() .then( ( ...thenArgs ) => { resolve( ...thenArgs ); } ) @@ -56,7 +54,7 @@ export default function debounceAsync( func, delayMS ) { return new Promise( ( resolve, reject ) => { // Schedule the next request but with a delay. timeoutId = setTimeout( () => { - activePromise = func( ...args ) + activePromise = func() .then( ( ...thenArgs ) => { resolve( ...thenArgs ); } ) diff --git a/packages/preferences-persistence/src/create/index.js b/packages/preferences-persistence/src/create/index.js index c550e0a7ac2c7..ed847e63a3cf0 100644 --- a/packages/preferences-persistence/src/create/index.js +++ b/packages/preferences-persistence/src/create/index.js @@ -6,7 +6,7 @@ import apiFetch from '@wordpress/api-fetch'; /** * Internal dependencies */ -import debounceAsync from './debounce-async'; +import createAsyncDebouncer from './create-async-debouncer'; const EMPTY_OBJECT = {}; const localStorage = window.localStorage; @@ -16,15 +16,18 @@ const localStorage = window.localStorage; * REST API. * * @param {Object} options - * @param {?Object} options.preloadedData Any persisted preferences data that should be preloaded. - * When set, the persistence layer will avoid fetching data - * from the REST API. - * @param {?string} options.localStorageRestoreKey The key to use for restoring the localStorage backup, used - * when the persistence layer calls `localStorage.getItem` or - * `localStorage.setItem`. - * @param {?number} options.requestDebounceMS Debounce requests to the API so that they only occur at - * minimum every `requestDebounceMS` milliseconds, and don't - * swamp the server. Defaults to 2500ms. + * @param {?Object} options.preloadedData Any persisted preferences data that should be preloaded. + * When set, the persistence layer will avoid fetching data + * from the REST API. + * @param {?string} options.localStorageRestoreKey The key to use for restoring the localStorage backup, used + * when the persistence layer calls `localStorage.getItem` or + * `localStorage.setItem`. + * @param {?number} options.requestDebounceMS Debounce requests to the API so that they only occur at + * minimum every `requestDebounceMS` milliseconds, and don't + * swamp the server. Defaults to 2500ms. + * + * @param {?number} options.expensiveRequestDebounceMS A longer debounce that can be defined for updates that have + * `isExpensive=true` defined. defaults to 60000ms. * * @return {Object} A persistence layer for WordPress user meta. */ @@ -32,9 +35,10 @@ export default function create( { preloadedData, localStorageRestoreKey = 'WP_PREFERENCES_RESTORE_DATA', requestDebounceMS = 2500, + expensiveRequestDebounceMS = 60000, } = {} ) { let cache = preloadedData; - const debouncedApiFetch = debounceAsync( apiFetch, requestDebounceMS ); + const debounce = createAsyncDebouncer(); async function get() { if ( cache ) { @@ -68,7 +72,7 @@ export default function create( { return cache; } - function set( newData ) { + function set( newData, { isExpensive = false } = {} ) { const dataWithTimestamp = { ...newData, _modified: new Date().toISOString(), @@ -83,26 +87,30 @@ export default function create( { JSON.stringify( dataWithTimestamp ) ); - // The user meta endpoint seems susceptible to errors when consecutive - // requests are made in quick succession. Ensure there's a gap between - // any consecutive requests. - // - // Catch and do nothing with errors from the REST API. - debouncedApiFetch( { - path: '/wp/v2/users/me', - method: 'PUT', - // `keepalive` will still send the request in the background, - // even when a browser unload event might interrupt it. - // This should hopefully make things more resilient. - // This does have a size limit of 64kb, but the data is usually - // much less. - keepalive: true, - data: { - meta: { - persisted_preferences: dataWithTimestamp, - }, - }, - } ).catch( () => {} ); + debounce( + () => + apiFetch( { + path: '/wp/v2/users/me', + method: 'PUT', + // `keepalive` will still send the request in the background, + // even when a browser unload event might interrupt it. + // This should hopefully make things more resilient. + // This does have a size limit of 64kb, but the data is usually + // much less. + keepalive: true, + data: { + meta: { + persisted_preferences: dataWithTimestamp, + }, + }, + } ).catch( () => {} ), + { + delayMS: isExpensive + ? expensiveRequestDebounceMS + : requestDebounceMS, + isTrailing: isExpensive, + } + ); } return { diff --git a/packages/preferences-persistence/src/create/test/debounce-async.js b/packages/preferences-persistence/src/create/test/create-async-debouncer.js similarity index 56% rename from packages/preferences-persistence/src/create/test/debounce-async.js rename to packages/preferences-persistence/src/create/test/create-async-debouncer.js index 44a867c998fc0..a8e1cc10b4ebe 100644 --- a/packages/preferences-persistence/src/create/test/debounce-async.js +++ b/packages/preferences-persistence/src/create/test/create-async-debouncer.js @@ -1,7 +1,7 @@ /** * Internal dependencies */ -import debounceAsync from '../debounce-async'; +import createAsyncDebouncer from '../create-async-debouncer'; // See https://stackoverflow.com/questions/52177631/jest-timer-and-promise-dont-work-well-settimeout-and-async-function. // Jest fake timers and async functions don't mix too well, since queued up @@ -17,25 +17,25 @@ function timeout( milliseconds ) { } describe( 'debounceAsync', () => { - it( 'uses a leading debounce, the first call happens immediately', () => { + it( 'uses a leading debounce by default, the first call happens immediately', () => { const fn = jest.fn( async () => {} ); - const debounced = debounceAsync( fn, 20 ); - debounced(); + const debounce = createAsyncDebouncer(); + debounce( () => fn(), { delayMS: 20 } ); expect( fn ).toHaveBeenCalledTimes( 1 ); } ); - it( 'calls the function on the leading edge and then once on the trailing edge when there are multiple calls', async () => { + it( 'calls the function on the leading edge and then once on the trailing edge when there are multiple leading edge calls', async () => { jest.useFakeTimers(); const fn = jest.fn( async () => {} ); - const debounced = debounceAsync( fn, 20 ); + const debounce = createAsyncDebouncer(); - debounced( 'A' ); + debounce( () => fn( 'A' ), { delayMS: 20 } ); expect( fn ).toHaveBeenCalledTimes( 1 ); - debounced( 'B' ); - debounced( 'C' ); - debounced( 'D' ); + debounce( () => fn( 'B' ), { delayMS: 20 } ); + debounce( () => fn( 'C' ), { delayMS: 20 } ); + debounce( () => fn( 'D' ), { delayMS: 20 } ); await flushPromises(); jest.runAllTimers(); @@ -48,16 +48,50 @@ describe( 'debounceAsync', () => { jest.useRealTimers(); } ); + it( 'can be configured to use a trailing edge debounce, the first call happens after the delay', () => { + jest.useFakeTimers(); + const fn = jest.fn( async () => {} ); + const debounce = createAsyncDebouncer(); + debounce( () => fn(), { delayMS: 20, isTrailing: true } ); + + // The function isn't called immediately. + expect( fn ).toHaveBeenCalledTimes( 0 ); + + // After the delay, the function is called. + jest.advanceTimersByTime( 20 ); + expect( fn ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'calls the function on the trailing edge only once when there are multiple trailing edge calls', async () => { + jest.useFakeTimers(); + const fn = jest.fn( async () => {} ); + const debounce = createAsyncDebouncer(); + + debounce( () => fn( 'A' ), { delayMS: 20, isTrailing: true } ); + debounce( () => fn( 'B' ), { delayMS: 20, isTrailing: true } ); + debounce( () => fn( 'C' ), { delayMS: 20, isTrailing: true } ); + debounce( () => fn( 'D' ), { delayMS: 20, isTrailing: true } ); + + await flushPromises(); + jest.runAllTimers(); + + expect( fn ).toHaveBeenCalledTimes( 1 ); + expect( fn ).toHaveBeenCalledWith( 'D' ); + + jest.runOnlyPendingTimers(); + jest.useRealTimers(); + } ); + it( 'ensures the delay has elapsed between calls', async () => { jest.useFakeTimers(); const fn = jest.fn( async () => timeout( 10 ) ); - const debounced = debounceAsync( fn, 20 ); + const debounce = createAsyncDebouncer(); // The first call has been triggered, but will take 10ms to resolve. - debounced(); - debounced(); - debounced(); - debounced(); + debounce( () => fn(), { delayMS: 20 } ); + debounce( () => fn(), { delayMS: 20 } ); + debounce( () => fn(), { delayMS: 20 } ); + debounce( () => fn(), { delayMS: 20 } ); expect( fn ).toHaveBeenCalledTimes( 1 ); // The first call has resolved. The delay period has started but has yet to finish. @@ -87,16 +121,18 @@ describe( 'debounceAsync', () => { it( 'is thenable, returning any data from promise resolution of the debounced function', async () => { expect.assertions( 2 ); const fn = async () => 'test'; - const debounced = debounceAsync( fn, 20 ); + const debounce = createAsyncDebouncer(); // Test the return value via awaiting. - const returnValue = await debounced(); + const returnValue = await debounce( () => fn(), { + delayMS: 20, + } ); expect( returnValue ).toBe( 'test' ); // Test then-ing. - await debounced().then( ( thenValue ) => - expect( thenValue ).toBe( 'test' ) - ); + await debounce( () => fn(), { + delayMS: 20, + } ).then( ( thenValue ) => expect( thenValue ).toBe( 'test' ) ); } ); it( 'is catchable', async () => { @@ -106,11 +142,13 @@ describe( 'debounceAsync', () => { throw expectedError; }; - const debounced = debounceAsync( fn, 20 ); + const debounce = createAsyncDebouncer(); // Test traditional try/catch. try { - await debounced(); + await debounce( () => fn(), { + delayMS: 20, + } ); } catch ( error ) { // Disable reason - the test uses `expect.assertions` to ensure // conditional assertions are called. @@ -119,7 +157,9 @@ describe( 'debounceAsync', () => { } // Test chained .catch(). - await debounced().catch( ( error ) => { + await await debounce( () => fn(), { + delayMS: 20, + } ).catch( ( error ) => { // Disable reason - the test uses `expect.assertions` to ensure // conditional assertions are called. // eslint-disable-next-line jest/no-conditional-expect diff --git a/packages/preferences-persistence/src/index.js b/packages/preferences-persistence/src/index.js index 2b1bcdd10fb03..68ffe4e41dcde 100644 --- a/packages/preferences-persistence/src/index.js +++ b/packages/preferences-persistence/src/index.js @@ -2,7 +2,11 @@ * Internal dependencies */ import create from './create'; -import convertLegacyLocalStorageData from './migrations/legacy-local-storage-data'; +import { + getLegacyData, + convertLegacyData, + convertLegacyInsertUsageData, +} from './migrations/legacy-local-storage-data'; import convertPreferencesPackageData from './migrations/preferences-package-data'; export { create }; @@ -38,9 +42,31 @@ export function __unstableCreatePersistenceLayer( serverData, userId ) { preloadedData = convertPreferencesPackageData( serverData ); } else if ( localData ) { preloadedData = convertPreferencesPackageData( localData ); - } else { + } + + // Handle legacy data migrations. + if ( ! preloadedData ) { // Check if there is data in the legacy format from the old persistence system. - preloadedData = convertLegacyLocalStorageData( userId ); + const legacyData = getLegacyData( userId ); + preloadedData = convertLegacyData( legacyData ); + } + + // The insertUsage preference was migrated later than others, the following code + // performs a separate migration just for this data. + if ( preloadedData && ! preloadedData?.core?.insertUsage ) { + const legacyData = getLegacyData( userId ); + + // Only run the migration if there's something to migrate. + if ( legacyData?.[ 'core/block-editor' ]?.preferences?.insertUsage ) { + // Check if there is data in the legacy format from the old persistence system. + preloadedData = { + ...preloadedData, + core: { + ...preloadedData.core, + insertUsage: convertLegacyInsertUsageData( legacyData ), + }, + }; + } } return create( { diff --git a/packages/preferences-persistence/src/migrations/legacy-local-storage-data/index.js b/packages/preferences-persistence/src/migrations/legacy-local-storage-data/index.js index 6f0bebd13a8a9..66022c554f123 100644 --- a/packages/preferences-persistence/src/migrations/legacy-local-storage-data/index.js +++ b/packages/preferences-persistence/src/migrations/legacy-local-storage-data/index.js @@ -14,7 +14,7 @@ import convertEditPostPanels from './convert-edit-post-panels'; * * @return {Object | null} The local storage data. */ -function getLegacyData( userId ) { +export function getLegacyData( userId ) { const key = `WP_DATA_USER_${ userId }`; const unparsedData = window.localStorage.getItem( key ); return JSON.parse( unparsedData ); @@ -81,22 +81,23 @@ export function convertLegacyData( data ) { { from: 'core/edit-site', to: 'core/edit-site' }, 'editorMode' ); + data = moveIndividualPreference( + data, + { from: 'core/block-editor', to: 'core' }, + 'insertUsage' + ); // The new system is only concerned with persisting // 'core/preferences' preferences reducer, so only return that. return data?.[ 'core/preferences' ]?.preferences; } -/** - * Gets the legacy local storage data for the given user and returns the - * data converted to the new format. - * - * @param {string | number} userId The user id. - * - * @return {Object | undefined} The converted data or undefined if no local - * storage data could be found. - */ -export default function convertLegacyLocalStorageData( userId ) { - const data = getLegacyData( userId ); - return convertLegacyData( data ); +export function convertLegacyInsertUsageData( data ) { + data = moveIndividualPreference( + data, + { from: 'core/block-editor', to: 'core' }, + 'insertUsage' + ); + + return data?.[ 'core/preferences' ]?.preferences?.core?.insertUsage; } diff --git a/packages/preferences-persistence/src/migrations/legacy-local-storage-data/test/index.js b/packages/preferences-persistence/src/migrations/legacy-local-storage-data/test/index.js index d4df39165c73d..87709791075e1 100644 --- a/packages/preferences-persistence/src/migrations/legacy-local-storage-data/test/index.js +++ b/packages/preferences-persistence/src/migrations/legacy-local-storage-data/test/index.js @@ -4,6 +4,40 @@ import { convertLegacyData } from '..'; const legacyData = { + 'core/block-editor': { + preferences: { + insertUsage: { + 'core/paragraph': { + time: 1649320988011, + count: 2, + insert: { + name: 'core/paragraph', + }, + }, + 'core/quote': { + time: 1649320934860, + count: 1, + insert: { + name: 'core/quote', + }, + }, + 'core/image': { + time: 1649321017053, + count: 1, + insert: { + name: 'core/image', + }, + }, + 'core/group': { + time: 1649321017077, + count: 1, + insert: { + name: 'core/group', + }, + }, + }, + }, + }, 'core/interface': { enableItems: { singleEnableItems: { @@ -65,42 +99,44 @@ const legacyData = { }; const alreadyConvertedData = { - 'core/block-editor': { + 'core/preferences': { preferences: { - insertUsage: { - 'core/paragraph': { - time: 1649320988011, - count: 2, - insert: { - name: 'core/paragraph', + core: { + insertUsage: { + 'core/paragraph': { + time: 1649320988011, + count: 2, + insert: { + name: 'core/paragraph', + }, }, - }, - 'core/quote': { - time: 1649320934860, - count: 1, - insert: { - name: 'core/quote', + 'core/quote': { + time: 1649320934860, + count: 1, + insert: { + name: 'core/quote', + }, }, - }, - 'core/image': { - time: 1649321017053, - count: 1, - insert: { - name: 'core/image', + 'core/image': { + time: 1649321017053, + count: 1, + insert: { + name: 'core/image', + }, }, - }, - 'core/group': { - time: 1649321017077, - count: 1, - insert: { - name: 'core/group', + 'core/group': { + time: 1649321017077, + count: 1, + insert: { + name: 'core/group', + }, }, }, }, - }, - }, - 'core/preferences': { - preferences: { + 'core/customize-widgets': { + welcomeGuide: false, + fixedToolbar: true, + }, 'core/edit-widgets': { welcomeGuide: false, fixedToolbar: true, @@ -134,6 +170,38 @@ describe( 'convertLegacyData', () => { it( 'converts to the expected format', () => { expect( convertLegacyData( legacyData ) ).toMatchInlineSnapshot( ` { + "core": { + "insertUsage": { + "core/group": { + "count": 1, + "insert": { + "name": "core/group", + }, + "time": 1649321017077, + }, + "core/image": { + "count": 1, + "insert": { + "name": "core/image", + }, + "time": 1649321017053, + }, + "core/paragraph": { + "count": 2, + "insert": { + "name": "core/paragraph", + }, + "time": 1649320988011, + }, + "core/quote": { + "count": 1, + "insert": { + "name": "core/quote", + }, + "time": 1649320934860, + }, + }, + }, "core/customize-widgets": { "fixedToolbar": true, "keepCaretInsideBlock": true, @@ -183,6 +251,42 @@ describe( 'convertLegacyData', () => { expect( convertLegacyData( alreadyConvertedData ) ) .toMatchInlineSnapshot( ` { + "core": { + "insertUsage": { + "core/group": { + "count": 1, + "insert": { + "name": "core/group", + }, + "time": 1649321017077, + }, + "core/image": { + "count": 1, + "insert": { + "name": "core/image", + }, + "time": 1649321017053, + }, + "core/paragraph": { + "count": 2, + "insert": { + "name": "core/paragraph", + }, + "time": 1649320988011, + }, + "core/quote": { + "count": 1, + "insert": { + "name": "core/quote", + }, + "time": 1649320934860, + }, + }, + }, + "core/customize-widgets": { + "fixedToolbar": true, + "welcomeGuide": false, + }, "core/edit-post": { "complementaryArea": "edit-post/block", "editorMode": "visual", diff --git a/packages/preferences/src/store/index.js b/packages/preferences/src/store/index.js index 7e57dac5e712c..215a8849fcb4b 100644 --- a/packages/preferences/src/store/index.js +++ b/packages/preferences/src/store/index.js @@ -8,8 +8,10 @@ import { createReduxStore, register } from '@wordpress/data'; */ import reducer from './reducer'; import * as actions from './actions'; +import * as privateActions from './private-actions'; import * as selectors from './selectors'; import { STORE_NAME } from './constants'; +import { unlock } from '../lock-unlock'; /** * Store definition for the preferences namespace. @@ -24,4 +26,5 @@ export const store = createReduxStore( STORE_NAME, { selectors, } ); +unlock( store ).registerPrivateActions( privateActions ); register( store ); diff --git a/packages/preferences/src/store/private-actions.js b/packages/preferences/src/store/private-actions.js new file mode 100644 index 0000000000000..86a72b3d75003 --- /dev/null +++ b/packages/preferences/src/store/private-actions.js @@ -0,0 +1,12 @@ +/** + * Marks the next value change as 'expensive'. + * + * This can be used for preferences that might be updated regularly. + * The persistence layer `set` function receives an `isExpensive: true` option when + * this is set and allows it to make optimizations around saving the value. + * + * @return {Object} Action object. + */ +export const markNextChangeAsExpensive = () => ( { + type: 'MARK_NEXT_CHANGE_AS_EXPENSIVE', +} ); diff --git a/packages/preferences/src/store/reducer.js b/packages/preferences/src/store/reducer.js index 0d2f463ab01de..3b1f12f989da0 100644 --- a/packages/preferences/src/store/reducer.js +++ b/packages/preferences/src/store/reducer.js @@ -41,6 +41,7 @@ export function defaults( state = {}, action ) { */ function withPersistenceLayer( reducer ) { let persistenceLayer; + let isNextChangeExpensive = false; return ( state, action ) => { // Setup the persistence layer, and return the persisted data @@ -51,9 +52,16 @@ function withPersistenceLayer( reducer ) { return persistedData; } + if ( action.type === 'MARK_NEXT_CHANGE_AS_EXPENSIVE' ) { + isNextChangeExpensive = true; + } + const nextState = reducer( state, action ); if ( action.type === 'SET_PREFERENCE_VALUE' ) { - persistenceLayer?.set( nextState ); + persistenceLayer?.set( nextState, { + isExpensive: isNextChangeExpensive, + } ); + isNextChangeExpensive = false; } return nextState; diff --git a/packages/preferences/src/store/test/reducer.js b/packages/preferences/src/store/test/reducer.js index 985959acb2f00..60919bd5ebc00 100644 --- a/packages/preferences/src/store/test/reducer.js +++ b/packages/preferences/src/store/test/reducer.js @@ -42,6 +42,45 @@ describe( 'withPersistenceLayer( preferences )', () => { const state = preferences( {}, setPreferenceValueAction ); - expect( set ).toHaveBeenCalledWith( state ); + expect( set ).toHaveBeenCalledWith( state, { isExpensive: false } ); + } ); + + it( 'passes the `isExpensive` flag to the persistence layer when the `MARK_NEXT_CHANGE_AS_EXPENSIVE` action is dispatched', () => { + const set = jest.fn(); + const persistenceLayer = { + set, + }; + + // Set the persistence layer. + const setPersistenceLayerAction = { + type: 'SET_PERSISTENCE_LAYER', + persistenceLayer, + persistedData: {}, + }; + preferences( {}, setPersistenceLayerAction ); + + // Mark the next change as expensive. + const markNextChangeAsExpensiveAction = { + type: 'MARK_NEXT_CHANGE_AS_EXPENSIVE', + }; + preferences( {}, markNextChangeAsExpensiveAction ); + + // Update a value, and expect the persistence layer `set` function to receive the `isExpensive: true` option. + const setPreferenceValueAction1 = { + type: 'SET_PREFERENCE_VALUE', + name: 'myPreference', + value: 'myValue1', + }; + const state1 = preferences( {}, setPreferenceValueAction1 ); + expect( set ).toHaveBeenCalledWith( state1, { isExpensive: true } ); + + // Update with another value, and expect that the `isExpensive` option is `false` again. + const setPreferenceValueAction2 = { + type: 'SET_PREFERENCE_VALUE', + name: 'myPreference', + value: 'myValue2', + }; + const state2 = preferences( {}, setPreferenceValueAction2 ); + expect( set ).toHaveBeenCalledWith( state2, { isExpensive: false } ); } ); } ); diff --git a/packages/private-apis/src/implementation.js b/packages/private-apis/src/implementation.js index c268e46f669cb..a95ec5d0b32e1 100644 --- a/packages/private-apis/src/implementation.js +++ b/packages/private-apis/src/implementation.js @@ -31,6 +31,7 @@ const CORE_MODULES_USING_PRIVATE_APIS = [ '@wordpress/reusable-blocks', '@wordpress/router', '@wordpress/dataviews', + '@wordpress/preferences', ]; /** diff --git a/packages/reusable-blocks/src/store/test/actions.js b/packages/reusable-blocks/src/store/test/actions.js index b2feae1195ee1..9db00c25f213c 100644 --- a/packages/reusable-blocks/src/store/test/actions.js +++ b/packages/reusable-blocks/src/store/test/actions.js @@ -11,6 +11,7 @@ import { } from '@wordpress/blocks'; import { store as coreStore } from '@wordpress/core-data'; +import { store as preferencesStore } from '@wordpress/preferences'; import apiFetch from '@wordpress/api-fetch'; /** @@ -31,6 +32,7 @@ function createRegistryWithStores() { registry.register( blockEditorStore ); registry.register( reusableBlocksStore ); registry.register( blocksStore ); + registry.register( preferencesStore ); // Register entity here instead of mocking API handlers for loadPostTypeEntities() registry.dispatch( coreStore ).addEntities( [