From c16611addd91f544f362877a4470a030d0277195 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Thu, 16 Mar 2023 12:43:01 +0100 Subject: [PATCH 1/4] Data: refuse to register an already registered store --- packages/data/src/registry.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/data/src/registry.js b/packages/data/src/registry.js index 8fe418e5d8bfa..899870b8d71be 100644 --- a/packages/data/src/registry.js +++ b/packages/data/src/registry.js @@ -215,6 +215,10 @@ export function createRegistry( storeConfigs = {}, parent = null ) { * @param {Object} store Store instance object (getSelectors, getActions, subscribe). */ function registerStoreInstance( name, store ) { + if ( stores[ name ] ) { + throw new Error( 'duplicate store oh no' ); + } + if ( typeof store.getSelectors !== 'function' ) { throw new TypeError( 'store.getSelectors must be a function' ); } From 9dd4139ed62ccd992c88f23a3c8d36423a6087ce Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Fri, 17 Mar 2023 09:07:44 +0100 Subject: [PATCH 2/4] Create store lazily, console error on duplicate store --- packages/data/src/registry.js | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/data/src/registry.js b/packages/data/src/registry.js index 899870b8d71be..7a913da488f46 100644 --- a/packages/data/src/registry.js +++ b/packages/data/src/registry.js @@ -211,14 +211,18 @@ export function createRegistry( storeConfigs = {}, parent = null ) { /** * Registers a store instance. * - * @param {string} name Store registry name. - * @param {Object} store Store instance object (getSelectors, getActions, subscribe). + * @param {string} name Store registry name. + * @param {Function} createStore Function that creates a store object (getSelectors, getActions, subscribe). */ - function registerStoreInstance( name, store ) { + function registerStoreInstance( name, createStore ) { if ( stores[ name ] ) { - throw new Error( 'duplicate store oh no' ); + // eslint-disable-next-line no-console + console.error( 'Store "' + name + '" is already registered.' ); + return stores[ name ]; } + const store = createStore(); + if ( typeof store.getSelectors !== 'function' ) { throw new TypeError( 'store.getSelectors must be a function' ); } @@ -266,6 +270,8 @@ export function createRegistry( storeConfigs = {}, parent = null ) { // ignore it. } } + + return store; } /** @@ -274,7 +280,9 @@ export function createRegistry( storeConfigs = {}, parent = null ) { * @param {StoreDescriptor} store Store descriptor. */ function register( store ) { - registerStoreInstance( store.name, store.instantiate( registry ) ); + registerStoreInstance( store.name, () => + store.instantiate( registry ) + ); } function registerGenericStore( name, store ) { @@ -282,7 +290,7 @@ export function createRegistry( storeConfigs = {}, parent = null ) { since: '5.9', alternative: 'wp.data.register( storeDescriptor )', } ); - registerStoreInstance( name, store ); + registerStoreInstance( name, () => store ); } /** @@ -298,10 +306,10 @@ export function createRegistry( storeConfigs = {}, parent = null ) { throw new TypeError( 'Must specify store reducer' ); } - const store = createReduxStore( storeName, options ).instantiate( - registry + const store = registerStoreInstance( storeName, () => + createReduxStore( storeName, options ).instantiate( registry ) ); - registerStoreInstance( storeName, store ); + return store.store; } From f039c3a9be6dabb35c362e702227b5a70c23fbc6 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Fri, 17 Mar 2023 10:56:07 +0100 Subject: [PATCH 3/4] Add changelog entry --- packages/data/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/data/CHANGELOG.md b/packages/data/CHANGELOG.md index 60592bd2ee53f..f422a9824cd74 100644 --- a/packages/data/CHANGELOG.md +++ b/packages/data/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Breaking Changes + +- The `registry.register` function will no longer register a store if another instance is registered with the same name. + ## 8.6.0 (2023-03-15) ## 8.5.0 (2023-03-01) From b97cb2f52d52fac6df182b06a786efeedf9dd779 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Mon, 20 Mar 2023 10:52:29 +0100 Subject: [PATCH 4/4] Add unit test to check double registration behavior --- packages/data/src/test/registry.js | 53 +++++++++++++++++------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/packages/data/src/test/registry.js b/packages/data/src/test/registry.js index 1422ef1082556..b9288eae821d8 100644 --- a/packages/data/src/test/registry.js +++ b/packages/data/src/test/registry.js @@ -481,41 +481,50 @@ describe( 'createRegistry', () => { } ); describe( 'register', () => { + const store = createReduxStore( 'demo', { + reducer( state = 'OK', action ) { + if ( action.type === 'UPDATE' ) { + return 'UPDATED'; + } + return state; + }, + actions: { + update: () => ( { type: 'UPDATE' } ), + }, + selectors: { + getValue: ( state ) => state, + }, + } ); + it( 'should work with the store descriptor as param for select', () => { - const store = createReduxStore( 'demo', { - reducer: ( state = 'OK' ) => state, - selectors: { - getValue: ( state ) => state, - }, - } ); registry.register( store ); expect( registry.select( store ).getValue() ).toBe( 'OK' ); } ); it( 'should work with the store descriptor as param for dispatch', async () => { - const store = createReduxStore( 'demo', { - reducer( state = 'OK', action ) { - if ( action.type === 'UPDATE' ) { - return 'UPDATED'; - } - return state; - }, - actions: { - update() { - return { type: 'UPDATE' }; - }, - }, - selectors: { - getValue: ( state ) => state, - }, - } ); registry.register( store ); expect( registry.select( store ).getValue() ).toBe( 'OK' ); await registry.dispatch( store ).update(); expect( registry.select( store ).getValue() ).toBe( 'UPDATED' ); } ); + + it( 'should keep the existing store instance on duplicate registration', async () => { + registry.register( store ); + + await registry.dispatch( store ).update(); + expect( registry.select( store ).getValue() ).toBe( 'UPDATED' ); + + registry.register( store ); + + // check that the state hasn't been reset back to `OK`, as a re-registration would do + expect( registry.select( store ).getValue() ).toBe( 'UPDATED' ); + + expect( console ).toHaveErroredWith( + 'Store "demo" is already registered.' + ); + } ); } ); describe( 'select', () => {