From 9044dc5f4ef57584e319c75728a4dad86c8805e7 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 26 Mar 2019 12:30:00 +0100 Subject: [PATCH 1/7] Refactor the core/data store to be independent from the registry --- lib/packages-dependencies.php | 1 + packages/data/package.json | 1 + .../index.js} | 107 +++++++++--------- .../metadata}/actions.js | 30 ++--- .../metadata}/reducer.js | 19 +--- .../metadata}/selectors.js | 35 +++--- .../metadata}/test/reducer.js | 0 .../metadata}/test/selectors.js | 0 .../metadata}/test/utils.js | 0 .../metadata}/utils.js | 0 packages/data/src/plugins/controls/index.js | 35 +----- .../data/src/plugins/persistence/index.js | 7 +- packages/data/src/registry.js | 18 +-- packages/data/src/store/index.js | 56 +++++++-- 14 files changed, 152 insertions(+), 157 deletions(-) rename packages/data/src/{namespace-store.js => namespace-store/index.js} (70%) rename packages/data/src/{store => namespace-store/metadata}/actions.js (65%) rename packages/data/src/{store => namespace-store/metadata}/reducer.js (77%) rename packages/data/src/{store => namespace-store/metadata}/selectors.js (54%) rename packages/data/src/{store => namespace-store/metadata}/test/reducer.js (100%) rename packages/data/src/{store => namespace-store/metadata}/test/selectors.js (100%) rename packages/data/src/{store => namespace-store/metadata}/test/utils.js (100%) rename packages/data/src/{store => namespace-store/metadata}/utils.js (100%) diff --git a/lib/packages-dependencies.php b/lib/packages-dependencies.php index fe839ee056704c..3bfbecc382c490 100644 --- a/lib/packages-dependencies.php +++ b/lib/packages-dependencies.php @@ -111,6 +111,7 @@ 'wp-data' => array( 'lodash', 'wp-compose', + 'wp-deprecated', 'wp-element', 'wp-is-shallow-equal', 'wp-priority-queue', diff --git a/packages/data/package.json b/packages/data/package.json index d8fde6251944fb..2f05a8cbb499dc 100644 --- a/packages/data/package.json +++ b/packages/data/package.json @@ -24,6 +24,7 @@ "dependencies": { "@babel/runtime": "^7.3.1", "@wordpress/compose": "file:../compose", + "@wordpress/deprecated": "file:../deprecated", "@wordpress/element": "file:../element", "@wordpress/is-shallow-equal": "file:../is-shallow-equal", "@wordpress/priority-queue": "file:../priority-queue", diff --git a/packages/data/src/namespace-store.js b/packages/data/src/namespace-store/index.js similarity index 70% rename from packages/data/src/namespace-store.js rename to packages/data/src/namespace-store/index.js index 97f1e2eae1b388..2b0d885437e2c5 100644 --- a/packages/data/src/namespace-store.js +++ b/packages/data/src/namespace-store/index.js @@ -7,12 +7,21 @@ import { get, mapValues, } from 'lodash'; +import combineReducers from 'turbo-combine-reducers'; + +/** + * WordPress dependencies + */ +import createReduxRoutineMiddleware from '@wordpress/redux-routine'; /** * Internal dependencies */ -import promise from './promise-middleware'; -import createResolversCacheMiddleware from './resolvers-cache-middleware'; +import promise from '../promise-middleware'; +import createResolversCacheMiddleware from '../resolvers-cache-middleware'; +import metadataReducer from './metadata/reducer'; +import * as metadataSelectors from './metadata/selectors'; +import * as metadataActions from './metadata/actions'; /** * Creates a namespace object with a store derived from the reducer given. @@ -27,16 +36,17 @@ export default function createNamespace( key, options, registry ) { const reducer = options.reducer; const store = createReduxStore( key, options, registry ); - let selectors, actions, resolvers; - if ( options.actions ) { - actions = mapActions( options.actions, store ); - } - if ( options.selectors ) { - selectors = mapSelectors( options.selectors, store, registry ); - } + let resolvers; + const actions = mapActions( { + ...metadataActions, + ...options.actions, + }, store ); + let selectors = mapSelectors( { + ...mapValues( metadataSelectors, ( selector ) => ( state, ...args ) => selector( state.metadata, ...args ) ), + ...mapValues( options.selectors, ( selector ) => ( state, ...args ) => selector( state.root, ...args ) ), + }, store, registry ); if ( options.resolvers ) { - const fulfillment = getCoreDataFulfillment( registry, key ); - const result = mapResolvers( options.resolvers, selectors, fulfillment, store ); + const result = mapResolvers( options.resolvers, selectors, store ); resolvers = result.resolvers; selectors = result.selectors; } @@ -84,15 +94,32 @@ export default function createNamespace( key, options, registry ) { * @return {Object} Newly created redux store. */ function createReduxStore( key, options, registry ) { + const middlewares = [ + createResolversCacheMiddleware( registry, key ), + promise, + ]; + + if ( options.controls ) { + const normalizedControls = mapValues( options.controls, ( control ) => { + return control.isRegistryControl ? control( registry ) : control; + } ); + middlewares.push( createReduxRoutineMiddleware( normalizedControls ) ); + } + const enhancers = [ - applyMiddleware( createResolversCacheMiddleware( registry, key ), promise ), + applyMiddleware( ...middlewares ), ]; if ( typeof window !== 'undefined' && window.__REDUX_DEVTOOLS_EXTENSION__ ) { enhancers.push( window.__REDUX_DEVTOOLS_EXTENSION__( { name: key, instanceId: key } ) ); } const { reducer, initialState } = options; - return createStore( reducer, initialState, flowRight( enhancers ) ); + const enhancedReducer = combineReducers( { + metadata: metadataReducer, + root: reducer, + } ); + + return createStore( enhancedReducer, initialState, flowRight( enhancers ) ); } /** @@ -151,11 +178,15 @@ function mapActions( actions, store ) { * * @param {Object} resolvers Resolvers to register. * @param {Object} selectors The current selectors to be modified. - * @param {Object} fulfillment Fulfillment implementation functions. * @param {Object} store The redux store to which the resolvers should be mapped. * @return {Object} An object containing updated selectors and resolvers. */ -function mapResolvers( resolvers, selectors, fulfillment, store ) { +function mapResolvers( resolvers, selectors, store ) { + const mappedResolvers = mapValues( resolvers, ( resolver ) => { + const { fulfill: resolverFulfill = resolver } = resolver; + return { ...resolver, fulfill: resolverFulfill }; + } ); + const mapSelector = ( selector, selectorName ) => { const resolver = resolvers[ selectorName ]; if ( ! resolver ) { @@ -169,13 +200,13 @@ function mapResolvers( resolvers, selectors, fulfillment, store ) { return; } - if ( fulfillment.hasStarted( selectorName, args ) ) { + if ( metadataSelectors.hasStartedResolution( store.getState().metadata, selectorName, args ) ) { return; } - fulfillment.start( selectorName, args ); - await fulfillment.fulfill( selectorName, ...args ); - fulfillment.finish( selectorName, args ); + store.dispatch( metadataActions.startResolution( selectorName, args ) ); + await fulfillResolver( store, mappedResolvers, selectorName, ...args ); + store.dispatch( metadataActions.finishResolution( selectorName, args ) ); } fulfillSelector( ...args ); @@ -183,54 +214,28 @@ function mapResolvers( resolvers, selectors, fulfillment, store ) { }; }; - const mappedResolvers = mapValues( resolvers, ( resolver ) => { - const { fulfill: resolverFulfill = resolver } = resolver; - return { ...resolver, fulfill: resolverFulfill }; - } ); - return { resolvers: mappedResolvers, selectors: mapValues( selectors, mapSelector ), }; } -/** - * Bundles up fulfillment functions for resolvers. - * @param {Object} registry Registry reference, for fulfilling via resolvers - * @param {string} key Part of the state shape to register the - * selectors for. - * @return {Object} An object providing fulfillment functions. - */ -function getCoreDataFulfillment( registry, key ) { - const { hasStartedResolution } = registry.select( 'core/data' ); - const { startResolution, finishResolution } = registry.dispatch( 'core/data' ); - - return { - hasStarted: ( ...args ) => hasStartedResolution( key, ...args ), - start: ( ...args ) => startResolution( key, ...args ), - finish: ( ...args ) => finishResolution( key, ...args ), - fulfill: ( ...args ) => fulfillWithRegistry( registry, key, ...args ), - }; -} - /** * Calls a resolver given arguments * - * @param {Object} registry Registry reference, for fulfilling via resolvers - * @param {string} key Part of the state shape to register the - * selectors for. + * @param {Object} store Store reference, for fulfilling via resolvers + * @param {Object} resolvers Store Resolvers * @param {string} selectorName Selector name to fulfill. - * @param {Array} args Selector Arguments. + * @param {Array} args Selector Arguments. */ -async function fulfillWithRegistry( registry, key, selectorName, ...args ) { - const namespace = registry.stores[ key ]; - const resolver = get( namespace, [ 'resolvers', selectorName ] ); +async function fulfillResolver( store, resolvers, selectorName, ...args ) { + const resolver = get( resolvers, [ selectorName ] ); if ( ! resolver ) { return; } const action = resolver.fulfill( ...args ); if ( action ) { - await namespace.store.dispatch( action ); + await store.dispatch( action ); } } diff --git a/packages/data/src/store/actions.js b/packages/data/src/namespace-store/metadata/actions.js similarity index 65% rename from packages/data/src/store/actions.js rename to packages/data/src/namespace-store/metadata/actions.js index b7bd9aa805738f..528cce18cd39f1 100644 --- a/packages/data/src/store/actions.js +++ b/packages/data/src/namespace-store/metadata/actions.js @@ -2,16 +2,14 @@ * Returns an action object used in signalling that selector resolution has * started. * - * @param {string} reducerKey Registered store reducer key. * @param {string} selectorName Name of selector for which resolver triggered. * @param {...*} args Arguments to associate for uniqueness. * * @return {Object} Action object. */ -export function startResolution( reducerKey, selectorName, args ) { +export function startResolution( selectorName, args ) { return { type: 'START_RESOLUTION', - reducerKey, selectorName, args, }; @@ -21,16 +19,14 @@ export function startResolution( reducerKey, selectorName, args ) { * Returns an action object used in signalling that selector resolution has * completed. * - * @param {string} reducerKey Registered store reducer key. * @param {string} selectorName Name of selector for which resolver triggered. * @param {...*} args Arguments to associate for uniqueness. * * @return {Object} Action object. */ -export function finishResolution( reducerKey, selectorName, args ) { +export function finishResolution( selectorName, args ) { return { type: 'FINISH_RESOLUTION', - reducerKey, selectorName, args, }; @@ -39,53 +35,43 @@ export function finishResolution( reducerKey, selectorName, args ) { /** * Returns an action object used in signalling that we should invalidate the resolution cache. * - * @param {string} reducerKey Registered store reducer key. * @param {string} selectorName Name of selector for which resolver should be invalidated. * @param {Array} args Arguments to associate for uniqueness. * * @return {Object} Action object. */ -export function invalidateResolution( reducerKey, selectorName, args ) { +export function invalidateResolution( selectorName, args ) { return { type: 'INVALIDATE_RESOLUTION', - reducerKey, selectorName, args, }; } /** - * Returns an action object used in signalling that the resolution cache for a - * given reducerKey should be invalidated. - * - * @param {string} reducerKey Registered store reducer key. + * Returns an action object used in signalling that the resolution + * should be invalidated. * * @return {Object} Action object. */ -export function invalidateResolutionForStore( reducerKey ) { +export function invalidateResolutionForStore() { return { type: 'INVALIDATE_RESOLUTION_FOR_STORE', - reducerKey, }; } /** * Returns an action object used in signalling that the resolution cache for a - * given reducerKey and selectorName should be invalidated. + * given selectorName should be invalidated. * - * @param {string} reducerKey Registered store reducer key. * @param {string} selectorName Name of selector for which all resolvers should * be invalidated. * * @return {Object} Action object. */ -export function invalidateResolutionForStoreSelector( - reducerKey, - selectorName -) { +export function invalidateResolutionForStoreSelector( selectorName ) { return { type: 'INVALIDATE_RESOLUTION_FOR_STORE_SELECTOR', - reducerKey, selectorName, }; } diff --git a/packages/data/src/store/reducer.js b/packages/data/src/namespace-store/metadata/reducer.js similarity index 77% rename from packages/data/src/store/reducer.js rename to packages/data/src/namespace-store/metadata/reducer.js index cd043f753f833b..46ddc183563ae0 100644 --- a/packages/data/src/store/reducer.js +++ b/packages/data/src/namespace-store/metadata/reducer.js @@ -13,7 +13,7 @@ import { onSubKey } from './utils'; * Reducer function returning next state for selector resolution of * subkeys, object form: * - * reducerKey -> selectorName -> EquivalentKeyMap + * selectorName -> EquivalentKeyMap * * @param {Object} state Current state. * @param {Object} action Dispatched action. @@ -21,7 +21,6 @@ import { onSubKey } from './utils'; * @returns {Object} Next state. */ const subKeysIsResolved = flowRight( [ - onSubKey( 'reducerKey' ), onSubKey( 'selectorName' ), ] )( ( state = new EquivalentKeyMap(), action ) => { switch ( action.type ) { @@ -44,7 +43,7 @@ const subKeysIsResolved = flowRight( [ /** * Reducer function returning next state for selector resolution, object form: * - * reducerKey -> selectorName -> EquivalentKeyMap + * selectorName -> EquivalentKeyMap * * @param {Object} state Current state. * @param {Object} action Dispatched action. @@ -54,18 +53,10 @@ const subKeysIsResolved = flowRight( [ const isResolved = ( state = {}, action ) => { switch ( action.type ) { case 'INVALIDATE_RESOLUTION_FOR_STORE': - return has( state, action.reducerKey ) ? - omit( state, [ action.reducerKey ] ) : - state; + return {}; case 'INVALIDATE_RESOLUTION_FOR_STORE_SELECTOR': - return has( state, [ action.reducerKey, action.selectorName ] ) ? - { - ...state, - [ action.reducerKey ]: omit( - state[ action.reducerKey ], - [ action.selectorName ] - ), - } : + return has( state, [ action.selectorName ] ) ? + omit( state, [ action.selectorName ] ) : state; case 'START_RESOLUTION': case 'FINISH_RESOLUTION': diff --git a/packages/data/src/store/selectors.js b/packages/data/src/namespace-store/metadata/selectors.js similarity index 54% rename from packages/data/src/store/selectors.js rename to packages/data/src/namespace-store/metadata/selectors.js index b17594e6705a5d..34217edcedd10f 100644 --- a/packages/data/src/store/selectors.js +++ b/packages/data/src/namespace-store/metadata/selectors.js @@ -4,20 +4,19 @@ import { get } from 'lodash'; /** - * Returns the raw `isResolving` value for a given reducer key, selector name, + * Returns the raw `isResolving` value for a given selector name, * and arguments set. May be undefined if the selector has never been resolved * or not resolved for the given set of arguments, otherwise true or false for * resolution started and completed respectively. * * @param {Object} state Data state. - * @param {string} reducerKey Registered store reducer key. * @param {string} selectorName Selector name. * @param {Array} args Arguments passed to selector. * * @return {?boolean} isResolving value. */ -export function getIsResolving( state, reducerKey, selectorName, args ) { - const map = get( state, [ reducerKey, selectorName ] ); +export function getIsResolving( state, selectorName, args ) { + const map = get( state, [ selectorName ] ); if ( ! map ) { return; } @@ -26,58 +25,54 @@ export function getIsResolving( state, reducerKey, selectorName, args ) { } /** - * Returns true if resolution has already been triggered for a given reducer - * key, selector name, and arguments set. + * Returns true if resolution has already been triggered for a given + * selector name, and arguments set. * * @param {Object} state Data state. - * @param {string} reducerKey Registered store reducer key. * @param {string} selectorName Selector name. * @param {?Array} args Arguments passed to selector (default `[]`). * * @return {boolean} Whether resolution has been triggered. */ -export function hasStartedResolution( state, reducerKey, selectorName, args = [] ) { - return getIsResolving( state, reducerKey, selectorName, args ) !== undefined; +export function hasStartedResolution( state, selectorName, args = [] ) { + return getIsResolving( state, selectorName, args ) !== undefined; } /** - * Returns true if resolution has completed for a given reducer key, selector + * Returns true if resolution has completed for a given selector * name, and arguments set. * * @param {Object} state Data state. - * @param {string} reducerKey Registered store reducer key. * @param {string} selectorName Selector name. * @param {?Array} args Arguments passed to selector. * * @return {boolean} Whether resolution has completed. */ -export function hasFinishedResolution( state, reducerKey, selectorName, args = [] ) { - return getIsResolving( state, reducerKey, selectorName, args ) === false; +export function hasFinishedResolution( state, selectorName, args = [] ) { + return getIsResolving( state, selectorName, args ) === false; } /** * Returns true if resolution has been triggered but has not yet completed for - * a given reducer key, selector name, and arguments set. + * a given selector name, and arguments set. * * @param {Object} state Data state. - * @param {string} reducerKey Registered store reducer key. * @param {string} selectorName Selector name. * @param {?Array} args Arguments passed to selector. * * @return {boolean} Whether resolution is in progress. */ -export function isResolving( state, reducerKey, selectorName, args = [] ) { - return getIsResolving( state, reducerKey, selectorName, args ) === true; +export function isResolving( state, selectorName, args = [] ) { + return getIsResolving( state, selectorName, args ) === true; } /** * Returns the list of the cached resolvers. * * @param {Object} state Data state. - * @param {string} reducerKey Registered store reducer key. * * @return {Object} Resolvers mapped by args and selectorName. */ -export function getCachedResolvers( state, reducerKey ) { - return state.hasOwnProperty( reducerKey ) ? state[ reducerKey ] : {}; +export function getCachedResolvers( state ) { + return state; } diff --git a/packages/data/src/store/test/reducer.js b/packages/data/src/namespace-store/metadata/test/reducer.js similarity index 100% rename from packages/data/src/store/test/reducer.js rename to packages/data/src/namespace-store/metadata/test/reducer.js diff --git a/packages/data/src/store/test/selectors.js b/packages/data/src/namespace-store/metadata/test/selectors.js similarity index 100% rename from packages/data/src/store/test/selectors.js rename to packages/data/src/namespace-store/metadata/test/selectors.js diff --git a/packages/data/src/store/test/utils.js b/packages/data/src/namespace-store/metadata/test/utils.js similarity index 100% rename from packages/data/src/store/test/utils.js rename to packages/data/src/namespace-store/metadata/test/utils.js diff --git a/packages/data/src/store/utils.js b/packages/data/src/namespace-store/metadata/utils.js similarity index 100% rename from packages/data/src/store/utils.js rename to packages/data/src/namespace-store/metadata/utils.js diff --git a/packages/data/src/plugins/controls/index.js b/packages/data/src/plugins/controls/index.js index a023dd6916b107..f5dd906dde60e7 100644 --- a/packages/data/src/plugins/controls/index.js +++ b/packages/data/src/plugins/controls/index.js @@ -1,36 +1,11 @@ -/** - * External dependencies - */ -import { applyMiddleware } from 'redux'; -import { mapValues } from 'lodash'; - /** * WordPress dependencies */ -import createMiddleware from '@wordpress/redux-routine'; +import deprecated from '@wordpress/deprecated'; export default function( registry ) { - return { - registerStore( reducerKey, options ) { - const store = registry.registerStore( reducerKey, options ); - - if ( options.controls ) { - const normalizedControls = mapValues( options.controls, ( control ) => { - return control.isRegistryControl ? control( registry ) : control; - } ); - const middleware = createMiddleware( normalizedControls ); - const enhancer = applyMiddleware( middleware ); - const createStore = () => store; - - Object.assign( - store, - enhancer( createStore )( options.reducer ) - ); - - registry.namespaces[ reducerKey ].supportControls = true; - } - - return store; - }, - }; + deprecated( 'wp.data.plugins.controls', { + hint: 'The controls plugins is now baked-in.', + } ); + return registry; } diff --git a/packages/data/src/plugins/persistence/index.js b/packages/data/src/plugins/persistence/index.js index 5ce7f41a37c2fc..09b218ef189c97 100644 --- a/packages/data/src/plugins/persistence/index.js +++ b/packages/data/src/plugins/persistence/index.js @@ -184,7 +184,12 @@ const persistencePlugin = function( registry, pluginOptions ) { initialState = persistedState; } - options = { ...options, initialState }; + options = { + ...options, + initialState: { + root: initialState, + }, + }; } const store = registry.registerStore( reducerKey, options ); diff --git a/packages/data/src/registry.js b/packages/data/src/registry.js index acca73b93129a3..9838afc4cfcc85 100644 --- a/packages/data/src/registry.js +++ b/packages/data/src/registry.js @@ -9,8 +9,8 @@ import { /** * Internal dependencies */ -import createNamespace from './namespace-store.js'; -import dataStore from './store'; +import createNamespace from './namespace-store'; +import createCoreDataStore from './store'; /** * An isolated orchestrator of store registrations. @@ -150,7 +150,10 @@ export function createRegistry( storeConfigs = {} ) { const namespace = createNamespace( reducerKey, options, registry ); registerGenericStore( reducerKey, namespace ); - return namespace.store; + return { + ...namespace.store, + getState: () => namespace.store.getState().root, + }; }; // @@ -166,10 +169,11 @@ export function createRegistry( storeConfigs = {} ) { return registry; } - Object.entries( { - 'core/data': dataStore, - ...storeConfigs, - } ).map( ( [ name, config ] ) => registry.registerStore( name, config ) ); + registerGenericStore( 'core/data', createCoreDataStore( registry ) ); + + Object.entries( storeConfigs ).map( + ( [ name, config ] ) => registry.registerStore( name, config ) + ); return withPlugins( registry ); } diff --git a/packages/data/src/store/index.js b/packages/data/src/store/index.js index 67ff3b67220856..5f5ee93007efd9 100644 --- a/packages/data/src/store/index.js +++ b/packages/data/src/store/index.js @@ -1,12 +1,44 @@ -/** - * Internal dependencies - */ -import reducer from './reducer'; -import * as selectors from './selectors'; -import * as actions from './actions'; - -export default { - reducer, - actions, - selectors, -}; + +function createCoreDataStore( registry ) { + const getCoreDataSelector = ( selectorName ) => ( reducerKey, ...args ) => { + return registry.select( reducerKey )[ selectorName ]( ...args ); + }; + + const getCoreDataAction = ( actionName ) => ( reducerKey, ...args ) => { + return registry.dispatch( reducerKey )[ actionName ]( ...args ); + }; + + return { + getSelectors() { + return [ + 'getIsResolving', + 'hasStartedResolution', + 'hasFinishedResolution', + 'isResolving', + 'getCachedResolvers', + ].reduce( ( memo, selectorName ) => ( { + ...memo, + [ selectorName ]: getCoreDataSelector( selectorName ), + } ), {} ); + }, + + getActions() { + return [ + 'startResolution', + 'finishResolution', + 'invalidateResolution', + 'invalidateResolutionForStore', + 'invalidateResolutionForStoreSelector', + ].reduce( ( memo, actionName ) => ( { + ...memo, + [ actionName ]: getCoreDataAction( actionName ), + } ), {} ); + }, + + subscribe() { + return () => {}; + }, + }; +} + +export default createCoreDataStore; From a215c0ef71da9b1e2f7bbe465108fac48fa7b672 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 26 Mar 2019 14:06:23 +0100 Subject: [PATCH 2/7] Fix unit tests --- package-lock.json | 1 + packages/data/src/namespace-store/index.js | 22 +++++++-- .../namespace-store/metadata/test/reducer.js | 46 +++++------------- .../metadata/test/selectors.js | 48 +++++++------------ .../test/index.js | 6 +-- .../data/src/plugins/persistence/index.js | 23 ++++----- test/unit/__mocks__/@wordpress/data.js | 8 ---- 7 files changed, 58 insertions(+), 96 deletions(-) rename packages/data/src/{plugins/controls => namespace-store}/test/index.js (83%) delete mode 100644 test/unit/__mocks__/@wordpress/data.js diff --git a/package-lock.json b/package-lock.json index 8fafc8cf6c8501..5ba3653de181ca 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2767,6 +2767,7 @@ "requires": { "@babel/runtime": "^7.3.1", "@wordpress/compose": "file:packages/compose", + "@wordpress/deprecated": "file:packages/deprecated", "@wordpress/element": "file:packages/element", "@wordpress/is-shallow-equal": "file:packages/is-shallow-equal", "@wordpress/priority-queue": "file:packages/priority-queue", diff --git a/packages/data/src/namespace-store/index.js b/packages/data/src/namespace-store/index.js index 2b0d885437e2c5..e9b09e1574d8df 100644 --- a/packages/data/src/namespace-store/index.js +++ b/packages/data/src/namespace-store/index.js @@ -43,7 +43,17 @@ export default function createNamespace( key, options, registry ) { }, store ); let selectors = mapSelectors( { ...mapValues( metadataSelectors, ( selector ) => ( state, ...args ) => selector( state.metadata, ...args ) ), - ...mapValues( options.selectors, ( selector ) => ( state, ...args ) => selector( state.root, ...args ) ), + ...mapValues( options.selectors, ( selector ) => { + if ( selector.isRegistrySelector ) { + const mappedSelector = ( reg ) => ( state, ...args ) => { + return selector( reg )( state.root, ...args ); + }; + mappedSelector.isRegistrySelector = selector.isRegistrySelector; + return mappedSelector; + } + + return ( state, ...args ) => selector( state.root, ...args ); + } ), }, store, registry ); if ( options.resolvers ) { const result = mapResolvers( options.resolvers, selectors, store ); @@ -119,7 +129,11 @@ function createReduxStore( key, options, registry ) { root: reducer, } ); - return createStore( enhancedReducer, initialState, flowRight( enhancers ) ); + return createStore( + enhancedReducer, + { root: initialState }, + flowRight( enhancers ) + ); } /** @@ -196,11 +210,11 @@ function mapResolvers( resolvers, selectors, store ) { return ( ...args ) => { async function fulfillSelector() { const state = store.getState(); - if ( typeof resolver.isFulfilled === 'function' && resolver.isFulfilled( state, ...args ) ) { + if ( typeof resolver.isFulfilled === 'function' && resolver.isFulfilled( state.root, ...args ) ) { return; } - if ( metadataSelectors.hasStartedResolution( store.getState().metadata, selectorName, args ) ) { + if ( metadataSelectors.hasStartedResolution( state.metadata, selectorName, args ) ) { return; } diff --git a/packages/data/src/namespace-store/metadata/test/reducer.js b/packages/data/src/namespace-store/metadata/test/reducer.js index eab156e89a71e4..d1af3f7804ea9c 100644 --- a/packages/data/src/namespace-store/metadata/test/reducer.js +++ b/packages/data/src/namespace-store/metadata/test/reducer.js @@ -18,128 +18,106 @@ describe( 'reducer', () => { it( 'should return with started resolution', () => { const state = reducer( undefined, { type: 'START_RESOLUTION', - reducerKey: 'test', selectorName: 'getFoo', args: [], } ); // { test: { getFoo: EquivalentKeyMap( [] => true ) } } - expect( state.test.getFoo.get( [] ) ).toBe( true ); + expect( state.getFoo.get( [] ) ).toBe( true ); } ); it( 'should return with finished resolution', () => { const original = reducer( undefined, { type: 'START_RESOLUTION', - reducerKey: 'test', selectorName: 'getFoo', args: [], } ); const state = reducer( deepFreeze( original ), { type: 'FINISH_RESOLUTION', - reducerKey: 'test', selectorName: 'getFoo', args: [], } ); // { test: { getFoo: EquivalentKeyMap( [] => false ) } } - expect( state.test.getFoo.get( [] ) ).toBe( false ); + expect( state.getFoo.get( [] ) ).toBe( false ); } ); it( 'should remove invalidations', () => { let state = reducer( undefined, { type: 'START_RESOLUTION', - reducerKey: 'test', selectorName: 'getFoo', args: [], } ); state = reducer( deepFreeze( state ), { type: 'FINISH_RESOLUTION', - reducerKey: 'test', selectorName: 'getFoo', args: [], } ); state = reducer( deepFreeze( state ), { type: 'INVALIDATE_RESOLUTION', - reducerKey: 'test', selectorName: 'getFoo', args: [], } ); - // { test: { getFoo: EquivalentKeyMap( [] => undefined ) } } - expect( state.test.getFoo.get( [] ) ).toBe( undefined ); + // { getFoo: EquivalentKeyMap( [] => undefined ) } + expect( state.getFoo.get( [] ) ).toBe( undefined ); } ); it( 'different arguments should not conflict', () => { const original = reducer( undefined, { type: 'START_RESOLUTION', - reducerKey: 'test', selectorName: 'getFoo', args: [ 'post' ], } ); let state = reducer( deepFreeze( original ), { type: 'FINISH_RESOLUTION', - reducerKey: 'test', selectorName: 'getFoo', args: [ 'post' ], } ); state = reducer( deepFreeze( state ), { type: 'START_RESOLUTION', - reducerKey: 'test', selectorName: 'getFoo', args: [ 'block' ], } ); - // { test: { getFoo: EquivalentKeyMap( [] => false ) } } - expect( state.test.getFoo.get( [ 'post' ] ) ).toBe( false ); - expect( state.test.getFoo.get( [ 'block' ] ) ).toBe( true ); + // { getFoo: EquivalentKeyMap( [] => false ) } + expect( state.getFoo.get( [ 'post' ] ) ).toBe( false ); + expect( state.getFoo.get( [ 'block' ] ) ).toBe( true ); } ); it( 'should remove invalidation for store level and leave others ' + 'intact', () => { const original = reducer( undefined, { type: 'FINISH_RESOLUTION', - reducerKey: 'testA', selectorName: 'getFoo', args: [ 'post' ], } ); - let state = reducer( deepFreeze( original ), { - type: 'FINISH_RESOLUTION', - reducerKey: 'testB', - selectorName: 'getBar', - args: [ 'postBar' ], - } ); - state = reducer( deepFreeze( state ), { + const state = reducer( deepFreeze( original ), { type: 'INVALIDATE_RESOLUTION_FOR_STORE', - reducerKey: 'testA', } ); - expect( state.testA ).toBeUndefined(); - // { testB: { getBar: EquivalentKeyMap( [] => false ) } } - expect( state.testB.getBar.get( [ 'postBar' ] ) ).toBe( false ); + expect( state ).toEqual( {} ); } ); it( 'should remove invalidation for store and selector name level and ' + 'leave other selectors at store level intact', () => { const original = reducer( undefined, { type: 'FINISH_RESOLUTION', - reducerKey: 'test', selectorName: 'getFoo', args: [ 'post' ], } ); let state = reducer( deepFreeze( original ), { type: 'FINISH_RESOLUTION', - reducerKey: 'test', selectorName: 'getBar', args: [ 'postBar' ], } ); state = reducer( deepFreeze( state ), { type: 'INVALIDATE_RESOLUTION_FOR_STORE_SELECTOR', - reducerKey: 'test', selectorName: 'getBar', } ); - expect( state.test.getBar ).toBeUndefined(); - // { test: { getFoo: EquivalentKeyMap( [] => false ) } } - expect( state.test.getFoo.get( [ 'post' ] ) ).toBe( false ); + expect( state.getBar ).toBeUndefined(); + // { getFoo: EquivalentKeyMap( [] => false ) } + expect( state.getFoo.get( [ 'post' ] ) ).toBe( false ); } ); } ); diff --git a/packages/data/src/namespace-store/metadata/test/selectors.js b/packages/data/src/namespace-store/metadata/test/selectors.js index 1af4bf9b8adf19..916d3beb436312 100644 --- a/packages/data/src/namespace-store/metadata/test/selectors.js +++ b/packages/data/src/namespace-store/metadata/test/selectors.js @@ -16,29 +16,25 @@ import { describe( 'getIsResolving', () => { it( 'should return undefined if no state by reducerKey, selectorName', () => { const state = {}; - const result = getIsResolving( state, 'test', 'getFoo', [] ); + const result = getIsResolving( state, 'getFoo', [] ); expect( result ).toBe( undefined ); } ); it( 'should return undefined if state by reducerKey, selectorName, but not args', () => { const state = { - test: { - getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), - }, + getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), }; - const result = getIsResolving( state, 'test', 'getFoo', [ 'bar' ] ); + const result = getIsResolving( state, 'getFoo', [ 'bar' ] ); expect( result ).toBe( undefined ); } ); it( 'should return value by reducerKey, selectorName', () => { const state = { - test: { - getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), - }, + getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), }; - const result = getIsResolving( state, 'test', 'getFoo', [] ); + const result = getIsResolving( state, 'getFoo', [] ); expect( result ).toBe( true ); } ); @@ -47,18 +43,16 @@ describe( 'getIsResolving', () => { describe( 'hasStartedResolution', () => { it( 'returns false if not has started', () => { const state = {}; - const result = hasStartedResolution( state, 'test', 'getFoo', [] ); + const result = hasStartedResolution( state, 'getFoo', [] ); expect( result ).toBe( false ); } ); it( 'returns true if has started', () => { const state = { - test: { - getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), - }, + getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), }; - const result = hasStartedResolution( state, 'test', 'getFoo', [] ); + const result = hasStartedResolution( state, 'getFoo', [] ); expect( result ).toBe( true ); } ); @@ -67,22 +61,18 @@ describe( 'hasStartedResolution', () => { describe( 'hasFinishedResolution', () => { it( 'returns false if not has finished', () => { const state = { - test: { - getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), - }, + getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), }; - const result = hasFinishedResolution( state, 'test', 'getFoo', [] ); + const result = hasFinishedResolution( state, 'getFoo', [] ); expect( result ).toBe( false ); } ); it( 'returns true if has finished', () => { const state = { - test: { - getFoo: new EquivalentKeyMap( [ [ [], false ] ] ), - }, + getFoo: new EquivalentKeyMap( [ [ [], false ] ] ), }; - const result = hasFinishedResolution( state, 'test', 'getFoo', [] ); + const result = hasFinishedResolution( state, 'getFoo', [] ); expect( result ).toBe( true ); } ); @@ -91,29 +81,25 @@ describe( 'hasFinishedResolution', () => { describe( 'isResolving', () => { it( 'returns false if not has started', () => { const state = {}; - const result = isResolving( state, 'test', 'getFoo', [] ); + const result = isResolving( state, 'getFoo', [] ); expect( result ).toBe( false ); } ); it( 'returns false if has finished', () => { const state = { - test: { - getFoo: new EquivalentKeyMap( [ [ [], false ] ] ), - }, + getFoo: new EquivalentKeyMap( [ [ [], false ] ] ), }; - const result = isResolving( state, 'test', 'getFoo', [] ); + const result = isResolving( state, 'getFoo', [] ); expect( result ).toBe( false ); } ); it( 'returns true if has started but not finished', () => { const state = { - test: { - getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), - }, + getFoo: new EquivalentKeyMap( [ [ [], true ] ] ), }; - const result = isResolving( state, 'test', 'getFoo', [] ); + const result = isResolving( state, 'getFoo', [] ); expect( result ).toBe( true ); } ); diff --git a/packages/data/src/plugins/controls/test/index.js b/packages/data/src/namespace-store/test/index.js similarity index 83% rename from packages/data/src/plugins/controls/test/index.js rename to packages/data/src/namespace-store/test/index.js index d60cb9053211d9..eba9e7e6697fc5 100644 --- a/packages/data/src/plugins/controls/test/index.js +++ b/packages/data/src/namespace-store/test/index.js @@ -1,16 +1,14 @@ /** * Internal dependencies */ -import { createRegistry } from '../../../registry'; -import { createRegistryControl } from '../../../factory'; -import controlsPlugin from '../'; +import { createRegistry } from '../../registry'; +import { createRegistryControl } from '../../factory'; describe( 'controls', () => { let registry; beforeEach( () => { registry = createRegistry(); - registry.use( controlsPlugin ); } ); describe( 'should call registry-aware controls', () => { diff --git a/packages/data/src/plugins/persistence/index.js b/packages/data/src/plugins/persistence/index.js index 09b218ef189c97..4c9f5c7c0f2b97 100644 --- a/packages/data/src/plugins/persistence/index.js +++ b/packages/data/src/plugins/persistence/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { flow, merge, isPlainObject, omit } from 'lodash'; +import { merge, isPlainObject, omit } from 'lodash'; /** * Internal dependencies @@ -147,14 +147,12 @@ const persistencePlugin = function( registry, pluginOptions ) { let lastState = getPersistedState( undefined, { nextState: getState() } ); - return ( result ) => { + return () => { const state = getPersistedState( lastState, { nextState: getState() } ); if ( state !== lastState ) { persistence.set( reducerKey, state ); lastState = state; } - - return result; }; } @@ -186,22 +184,17 @@ const persistencePlugin = function( registry, pluginOptions ) { options = { ...options, - initialState: { - root: initialState, - }, + initialState, }; } const store = registry.registerStore( reducerKey, options ); - store.dispatch = flow( [ - store.dispatch, - createPersistOnChange( - store.getState, - reducerKey, - options.persist - ), - ] ); + store.subscribe( createPersistOnChange( + store.getState, + reducerKey, + options.persist + ) ); return store; }, diff --git a/test/unit/__mocks__/@wordpress/data.js b/test/unit/__mocks__/@wordpress/data.js deleted file mode 100644 index 729e53648b211e..00000000000000 --- a/test/unit/__mocks__/@wordpress/data.js +++ /dev/null @@ -1,8 +0,0 @@ -/** - * Internal dependencies - */ -import { use, plugins } from '../../../../packages/data/src'; - -use( plugins.controls ); - -export * from '../../../../packages/data/src'; From b440ce83ca256792ffbfc9e5b6abf7a6a2da2cba Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 26 Mar 2019 14:43:48 +0100 Subject: [PATCH 3/7] Override core inline scripts --- lib/client-assets.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/client-assets.php b/lib/client-assets.php index 2a97319dd9f1c3..f3ad7c7ee92dfa 100644 --- a/lib/client-assets.php +++ b/lib/client-assets.php @@ -245,7 +245,9 @@ function gutenberg_register_scripts_and_styles() { ); // TEMPORARY: Core does not (yet) provide persistence migration from the - // introduction of the block editor. + // introduction of the block editor and still calls the data plugins. + // We unset the existing inline scripts first. + $wp_scripts->registered['wp-data']->extra['after'] = array(); wp_add_inline_script( 'wp-data', implode( @@ -254,8 +256,10 @@ function gutenberg_register_scripts_and_styles() { '( function() {', ' var userId = ' . get_current_user_ID() . ';', ' var storageKey = "WP_DATA_USER_" + userId;', + ' wp.data', + ' .use( wp.data.plugins.persistence, { storageKey: storageKey } );', ' wp.data.plugins.persistence.__unstableMigrate( { storageKey: storageKey } );', - '} )()', + '} )();', ) ) ); From 43b95fbd844ee6667bc6958b76efb9cacdf033bf Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 26 Mar 2019 15:37:04 +0100 Subject: [PATCH 4/7] Fix monkey-patching the stores --- packages/data/src/namespace-store/index.js | 17 ++++++++++++----- packages/data/src/registry.js | 5 +---- packages/data/src/resolvers-cache-middleware.js | 2 +- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/data/src/namespace-store/index.js b/packages/data/src/namespace-store/index.js index e9b09e1574d8df..6d1c8897f4ca91 100644 --- a/packages/data/src/namespace-store/index.js +++ b/packages/data/src/namespace-store/index.js @@ -64,12 +64,18 @@ export default function createNamespace( key, options, registry ) { const getSelectors = () => selectors; const getActions = () => actions; + // We have some modules monkey-patching the store object + // It's wrong to do so but until we refactor all of our effects to controls + // We need to keep the same "store" instance here. + store.__unstableOriginalGetState = store.getState; + store.getState = () => store.__unstableOriginalGetState().root; + // Customize subscribe behavior to call listeners only on effective change, // not on every dispatch. const subscribe = store && function( listener ) { - let lastState = store.getState(); + let lastState = store.__unstableOriginalGetState(); store.subscribe( () => { - const state = store.getState(); + const state = store.__unstableOriginalGetState(); const hasChanged = state !== lastState; lastState = state; @@ -161,7 +167,7 @@ function mapSelectors( selectors, store, registry ) { // direct assignment. const argsLength = arguments.length; const args = new Array( argsLength + 1 ); - args[ 0 ] = store.getState(); + args[ 0 ] = store.__unstableOriginalGetState(); for ( let i = 0; i < argsLength; i++ ) { args[ i + 1 ] = arguments[ i ]; } @@ -210,11 +216,12 @@ function mapResolvers( resolvers, selectors, store ) { return ( ...args ) => { async function fulfillSelector() { const state = store.getState(); - if ( typeof resolver.isFulfilled === 'function' && resolver.isFulfilled( state.root, ...args ) ) { + if ( typeof resolver.isFulfilled === 'function' && resolver.isFulfilled( state, ...args ) ) { return; } - if ( metadataSelectors.hasStartedResolution( state.metadata, selectorName, args ) ) { + const metadata = store.__unstableOriginalGetState().metadata; + if ( metadataSelectors.hasStartedResolution( metadata, selectorName, args ) ) { return; } diff --git a/packages/data/src/registry.js b/packages/data/src/registry.js index 9838afc4cfcc85..13937e20429de1 100644 --- a/packages/data/src/registry.js +++ b/packages/data/src/registry.js @@ -150,10 +150,7 @@ export function createRegistry( storeConfigs = {} ) { const namespace = createNamespace( reducerKey, options, registry ); registerGenericStore( reducerKey, namespace ); - return { - ...namespace.store, - getState: () => namespace.store.getState().root, - }; + return namespace.store; }; // diff --git a/packages/data/src/resolvers-cache-middleware.js b/packages/data/src/resolvers-cache-middleware.js index 0bc57390795669..7477b36d805b59 100644 --- a/packages/data/src/resolvers-cache-middleware.js +++ b/packages/data/src/resolvers-cache-middleware.js @@ -14,7 +14,7 @@ import { get } from 'lodash'; const createResolversCacheMiddleware = ( registry, reducerKey ) => () => ( next ) => ( action ) => { const resolvers = registry.select( 'core/data' ).getCachedResolvers( reducerKey ); Object.entries( resolvers ).forEach( ( [ selectorName, resolversByArgs ] ) => { - const resolver = get( registry.namespaces, [ reducerKey, 'resolvers', selectorName ] ); + const resolver = get( registry.stores, [ reducerKey, 'resolvers', selectorName ] ); if ( ! resolver || ! resolver.shouldInvalidate ) { return; } From 59d43d23ab22bf7f024a0fa080f29e2462a464fa Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 27 Mar 2019 08:53:27 +0100 Subject: [PATCH 5/7] Document the empty subscribe function --- packages/data/src/store/index.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/data/src/store/index.js b/packages/data/src/store/index.js index 5f5ee93007efd9..9a14aac264508e 100644 --- a/packages/data/src/store/index.js +++ b/packages/data/src/store/index.js @@ -36,6 +36,10 @@ function createCoreDataStore( registry ) { }, subscribe() { + // There's no reasons to trigger any listener when we subscribe to this store + // because there's no state stored in this store that need to retrigger selectors + // if a change happens, the corresponding store where the tracking stated live + // would have already triggered a "subscribe" call. return () => {}; }, }; From 7400e0102322445f2e649d484dc96af0c8c204df Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 27 Mar 2019 08:54:22 +0100 Subject: [PATCH 6/7] Destructure metadata --- packages/data/src/namespace-store/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/data/src/namespace-store/index.js b/packages/data/src/namespace-store/index.js index 6d1c8897f4ca91..8a53d28b300c3c 100644 --- a/packages/data/src/namespace-store/index.js +++ b/packages/data/src/namespace-store/index.js @@ -220,7 +220,7 @@ function mapResolvers( resolvers, selectors, store ) { return; } - const metadata = store.__unstableOriginalGetState().metadata; + const { metadata } = store.__unstableOriginalGetState(); if ( metadataSelectors.hasStartedResolution( metadata, selectorName, args ) ) { return; } From 7e6e3af6bb1035f58ec62502682cbc28c99ff9c0 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Wed, 27 Mar 2019 08:55:35 +0100 Subject: [PATCH 7/7] forEach instead of map --- packages/data/src/registry.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/data/src/registry.js b/packages/data/src/registry.js index 13937e20429de1..e1c324f6ef8ac0 100644 --- a/packages/data/src/registry.js +++ b/packages/data/src/registry.js @@ -168,7 +168,7 @@ export function createRegistry( storeConfigs = {} ) { registerGenericStore( 'core/data', createCoreDataStore( registry ) ); - Object.entries( storeConfigs ).map( + Object.entries( storeConfigs ).forEach( ( [ name, config ] ) => registry.registerStore( name, config ) );