From 147e23d5127e4ec1ec2ed0e3f19dbeef9102bfe0 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Mon, 29 May 2023 14:11:52 +0200 Subject: [PATCH 1/6] Return stable references to private selectors and actions --- packages/data/src/redux-store/index.js | 41 +++++++++++++++++++------- packages/data/src/test/privateAPIs.js | 20 +++++++++++++ 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/packages/data/src/redux-store/index.js b/packages/data/src/redux-store/index.js index 2618eb92c67454..cc913151af3cc6 100644 --- a/packages/data/src/redux-store/index.js +++ b/packages/data/src/redux-store/index.js @@ -101,6 +101,21 @@ function createResolversCache() { }; } +function createBindingCache( bind ) { + const cache = new WeakMap(); + + return { + get( item ) { + let boundItem = cache.get( item ); + if ( ! boundItem ) { + boundItem = bind( item ); + cache.set( item, boundItem ); + } + return boundItem; + }, + }; +} + /** * Creates a data store descriptor for the provided Redux store configuration containing * properties describing reducer, actions, selectors, controls and resolvers. @@ -185,18 +200,21 @@ export default function createReduxStore( key, options ) { ...mapValues( options.actions, bindAction ), }; - lock( - actions, - new Proxy( privateActions, { + const boundPrivateActions = createBindingCache( bindAction ); + const allActions = new Proxy( + {}, + { get: ( target, prop ) => { const privateAction = privateActions[ prop ]; return privateAction - ? bindAction( privateAction ) + ? boundPrivateActions.get( privateAction ) : actions[ prop ]; }, - } ) + } ); + lock( actions, allActions ); + function bindSelector( selector ) { if ( selector.isRegistrySelector ) { selector.registry = registry; @@ -234,18 +252,21 @@ export default function createReduxStore( key, options ) { ); } - lock( - selectors, - new Proxy( privateSelectors, { + const boundPrivateSelectors = createBindingCache( bindSelector ); + const allSelectors = new Proxy( + {}, + { get: ( target, prop ) => { const privateSelector = privateSelectors[ prop ]; return privateSelector - ? bindSelector( privateSelector ) + ? boundPrivateSelectors.get( privateSelector ) : selectors[ prop ]; }, - } ) + } ); + lock( selectors, allSelectors ); + const resolveSelectors = mapResolveSelectors( selectors, store ); const suspendSelectors = mapSuspendSelectors( selectors, store ); diff --git a/packages/data/src/test/privateAPIs.js b/packages/data/src/test/privateAPIs.js index 1ccf6adb0c5115..ab6b8bc623220e 100644 --- a/packages/data/src/test/privateAPIs.js +++ b/packages/data/src/test/privateAPIs.js @@ -147,6 +147,16 @@ describe( 'Private data APIs', () => { expect( unlockedSelectors.getPublicPrice() ).toEqual( 1000 ); } ); + it( 'should return stable references to selectors', () => { + const groceryStore = createStore(); + unlock( groceryStore ).registerPrivateSelectors( { + getSecretDiscount, + } ); + const select = unlock( registry.select( groceryStore ) ); + expect( select.getPublicPrice ).toBe( select.getPublicPrice ); + expect( select.getSecretDiscount ).toBe( select.getSecretDiscount ); + } ); + it( 'should support registerStore', () => { const groceryStore = registry.registerStore( storeName, @@ -263,6 +273,16 @@ describe( 'Private data APIs', () => { ).toEqual( 400 ); } ); + it( 'should return stable references to actions', () => { + const groceryStore = createStore(); + unlock( groceryStore ).registerPrivateActions( { + setSecretDiscount, + } ); + const disp = unlock( registry.dispatch( groceryStore ) ); + expect( disp.setPublicPrice ).toBe( disp.setPublicPrice ); + expect( disp.setSecretDiscount ).toBe( disp.setSecretDiscount ); + } ); + it( 'should dispatch public actions on the unlocked store', () => { const groceryStore = createStore(); unlock( groceryStore ).registerPrivateActions( { From 35552b61ed59a39ce5f825614759624f3f38af43 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Mon, 29 May 2023 15:01:30 +0200 Subject: [PATCH 2/6] Expose unlocked private actions and selectors to thunks --- packages/data/src/redux-store/index.js | 59 ++++++++++++-------------- packages/data/src/test/privateAPIs.js | 39 ++++++++++++----- 2 files changed, 57 insertions(+), 41 deletions(-) diff --git a/packages/data/src/redux-store/index.js b/packages/data/src/redux-store/index.js index cc913151af3cc6..ac1ef1012ceb65 100644 --- a/packages/data/src/redux-store/index.js +++ b/packages/data/src/redux-store/index.js @@ -162,17 +162,10 @@ export default function createReduxStore( key, options ) { const thunkArgs = { registry, get dispatch() { - return Object.assign( - ( action ) => store.dispatch( action ), - getActions() - ); + return thunkActions; }, get select() { - return Object.assign( - ( selector ) => - selector( store.__unstableOriginalGetState() ), - getSelectors() - ); + return thunkSelectors; }, get resolveSelect() { return getResolveSelectors(); @@ -201,17 +194,19 @@ export default function createReduxStore( key, options ) { }; const boundPrivateActions = createBindingCache( bindAction ); - const allActions = new Proxy( - {}, - { - get: ( target, prop ) => { - const privateAction = privateActions[ prop ]; - return privateAction - ? boundPrivateActions.get( privateAction ) - : actions[ prop ]; - }, - } - ); + const allActions = new Proxy( () => {}, { + get: ( target, prop ) => { + const privateAction = privateActions[ prop ]; + return privateAction + ? boundPrivateActions.get( privateAction ) + : actions[ prop ]; + }, + } ); + + const thunkActions = new Proxy( allActions, { + apply: ( target, thisArg, [ action ] ) => + store.dispatch( action ), + } ); lock( actions, allActions ); @@ -253,17 +248,19 @@ export default function createReduxStore( key, options ) { } const boundPrivateSelectors = createBindingCache( bindSelector ); - const allSelectors = new Proxy( - {}, - { - get: ( target, prop ) => { - const privateSelector = privateSelectors[ prop ]; - return privateSelector - ? boundPrivateSelectors.get( privateSelector ) - : selectors[ prop ]; - }, - } - ); + const allSelectors = new Proxy( () => {}, { + get: ( target, prop ) => { + const privateSelector = privateSelectors[ prop ]; + return privateSelector + ? boundPrivateSelectors.get( privateSelector ) + : selectors[ prop ]; + }, + } ); + + const thunkSelectors = new Proxy( allSelectors, { + apply: ( target, thisArg, [ selector ] ) => + selector( store.__unstableOriginalGetState() ), + } ); lock( selectors, allSelectors ); diff --git a/packages/data/src/test/privateAPIs.js b/packages/data/src/test/privateAPIs.js index ab6b8bc623220e..08c2b55e3c39ef 100644 --- a/packages/data/src/test/privateAPIs.js +++ b/packages/data/src/test/privateAPIs.js @@ -32,23 +32,19 @@ describe( 'Private data APIs', () => { getState: ( state ) => state, }, actions: { setPublicPrice }, - reducer: ( state, action ) => { - if ( action?.type === 'SET_PRIVATE_PRICE' ) { + reducer: ( state = { price: 1000, secretDiscount: 800 }, action ) => { + if ( action.type === 'SET_PRIVATE_PRICE' ) { return { ...state, - secretDiscount: action?.price, + secretDiscount: action.price, }; - } else if ( action?.type === 'SET_PUBLIC_PRICE' ) { + } else if ( action.type === 'SET_PUBLIC_PRICE' ) { return { ...state, - price: action?.price, + price: action.price, }; } - return { - price: 1000, - secretDiscount: 800, - ...( state || {} ), - }; + return state; }, }; function createStore() { @@ -314,6 +310,29 @@ describe( 'Private data APIs', () => { ).toEqual( 100 ); } ); + it( 'should expose unlocked private selectors and actions to thunks', () => { + const groceryStore = createStore(); + unlock( groceryStore ).registerPrivateSelectors( { + getSecretDiscount, + } ); + unlock( groceryStore ).registerPrivateActions( { + setSecretDiscount, + doubleSecretDiscount() { + return ( { dispatch, select } ) => { + dispatch.setSecretDiscount( + select.getSecretDiscount() * 2 + ); + }; + }, + } ); + const privateActions = unlock( registry.dispatch( groceryStore ) ); + privateActions.setSecretDiscount( 100 ); + privateActions.doubleSecretDiscount(); + expect( + unlock( registry.select( groceryStore ) ).getSecretDiscount() + ).toEqual( 200 ); + } ); + it( 'should support registerStore', () => { const groceryStore = registry.registerStore( storeName, From 681433116080b9db8767f4ecf1fc6d30c3cf5e49 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Fri, 26 May 2023 11:32:05 +1000 Subject: [PATCH 3/6] Add tests for private registry selectors, including one failing test --- packages/data/src/test/privateAPIs.js | 37 +++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/data/src/test/privateAPIs.js b/packages/data/src/test/privateAPIs.js index 08c2b55e3c39ef..7e60ac9543f9d1 100644 --- a/packages/data/src/test/privateAPIs.js +++ b/packages/data/src/test/privateAPIs.js @@ -4,6 +4,7 @@ import { createRegistry } from '../registry'; import createReduxStore from '../redux-store'; import { unlock } from '../private-apis'; +import { createRegistrySelector } from '../factory'; describe( 'Private data APIs', () => { let registry; @@ -201,6 +202,42 @@ describe( 'Private data APIs', () => { ); expect( subPrivateSelectors.getSecretDiscount() ).toEqual( 800 ); } ); + + it( 'should support private registry selectors', () => { + const groceryStore = createStore(); + const otherStore = createReduxStore( 'other', { + reducer: ( state = {} ) => state, + } ); + registry.register( otherStore ); + unlock( otherStore ).registerPrivateSelectors( { + getPrice: createRegistrySelector( + ( select ) => () => select( groceryStore ).getPublicPrice() + ), + } ); + const privateSelectors = unlock( registry.select( otherStore ) ); + expect( privateSelectors.getPrice() ).toEqual( 1000 ); + } ); + + it( 'should support calling a private registry selector from a public selector', () => { + const groceryStore = createStore(); + const store = createReduxStore( 'a', { + reducer: ( state = {} ) => state, + selectors: { + getPriceWithShippingAndTax: ( state ) => + getPriceWithShipping( state ) * 1.1, + }, + } ); + registry.register( store ); + const getPriceWithShipping = createRegistrySelector( + ( select ) => () => select( groceryStore ).getPublicPrice() + 5 + ); + unlock( store ).registerPrivateSelectors( { + getPriceWithShipping, + } ); + expect( + registry.select( store ).getPriceWithShippingAndTax() + ).toEqual( 1105.5 ); + } ); } ); describe( 'private actions', () => { From 144fb057b7cefa9adfa2575e404affc07c2ea08d Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Tue, 30 May 2023 10:04:49 +0200 Subject: [PATCH 4/6] Fix comment typo in createRegistrySelector --- packages/data/src/factory.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/data/src/factory.js b/packages/data/src/factory.js index 046836dd6d0332..a4adc4dcf42517 100644 --- a/packages/data/src/factory.js +++ b/packages/data/src/factory.js @@ -47,7 +47,7 @@ export function createRegistrySelector( registrySelector ) { /** * Flag indicating that the selector is a registry selector that needs the correct registry - * reference to be assigned to `selecto.registry` to make it work correctly. + * reference to be assigned to `selector.registry` to make it work correctly. * be mapped as a registry selector. * * @type {boolean} From ba0246b3d368c3367e8fc55001bf11623f2484a0 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Tue, 30 May 2023 10:05:35 +0200 Subject: [PATCH 5/6] Private registry selectors test: register selectors before store instantiation --- packages/data/src/test/privateAPIs.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/data/src/test/privateAPIs.js b/packages/data/src/test/privateAPIs.js index 7e60ac9543f9d1..aa5a53719041d7 100644 --- a/packages/data/src/test/privateAPIs.js +++ b/packages/data/src/test/privateAPIs.js @@ -208,18 +208,21 @@ describe( 'Private data APIs', () => { const otherStore = createReduxStore( 'other', { reducer: ( state = {} ) => state, } ); - registry.register( otherStore ); unlock( otherStore ).registerPrivateSelectors( { getPrice: createRegistrySelector( ( select ) => () => select( groceryStore ).getPublicPrice() ), } ); + registry.register( otherStore ); const privateSelectors = unlock( registry.select( otherStore ) ); expect( privateSelectors.getPrice() ).toEqual( 1000 ); } ); it( 'should support calling a private registry selector from a public selector', () => { const groceryStore = createStore(); + const getPriceWithShipping = createRegistrySelector( + ( select ) => () => select( groceryStore ).getPublicPrice() + 5 + ); const store = createReduxStore( 'a', { reducer: ( state = {} ) => state, selectors: { @@ -227,13 +230,10 @@ describe( 'Private data APIs', () => { getPriceWithShipping( state ) * 1.1, }, } ); - registry.register( store ); - const getPriceWithShipping = createRegistrySelector( - ( select ) => () => select( groceryStore ).getPublicPrice() + 5 - ); unlock( store ).registerPrivateSelectors( { getPriceWithShipping, } ); + registry.register( store ); expect( registry.select( store ).getPriceWithShippingAndTax() ).toEqual( 1105.5 ); From 3aa08c6ba89965f29e898c915487542425e62dc7 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Tue, 30 May 2023 10:22:14 +0200 Subject: [PATCH 6/6] Pre-bind private selectors so that registry selectors are bound to registry --- packages/data/src/redux-store/index.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/data/src/redux-store/index.js b/packages/data/src/redux-store/index.js index ac1ef1012ceb65..3e3e680d0ab0f2 100644 --- a/packages/data/src/redux-store/index.js +++ b/packages/data/src/redux-store/index.js @@ -248,6 +248,13 @@ export default function createReduxStore( key, options ) { } const boundPrivateSelectors = createBindingCache( bindSelector ); + + // Pre-bind the private selectors that have been registered by the time of + // instantiation, so that registry selectors are bound to the registry. + for ( const privateSelector of Object.values( privateSelectors ) ) { + boundPrivateSelectors.get( privateSelector ); + } + const allSelectors = new Proxy( () => {}, { get: ( target, prop ) => { const privateSelector = privateSelectors[ prop ];