Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data: allow binding registry selector to multiple registries #57943

Merged
merged 2 commits into from Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 8 additions & 4 deletions packages/data/src/factory.js
Expand Up @@ -39,15 +39,19 @@
* @return {Function} Registry selector that can be registered with a store.
*/
export function createRegistrySelector( registrySelector ) {
let selector;
let lastRegistry;
const selectorsByRegistry = new WeakMap();
// Create a selector function that is bound to the registry referenced by `selector.registry`
// and that has the same API as a regular selector. Binding it in such a way makes it
// possible to call the selector directly from another selector.
const wrappedSelector = ( ...args ) => {
if ( ! selector || lastRegistry !== wrappedSelector.registry ) {
let selector = selectorsByRegistry.get( wrappedSelector.registry );
// We want to make sure the cache persists even when new registry
// instances are created. For example patterns create their own editors
// with their own core/block-editor stores, so we should keep track of
// the cache for each registry instance.
if ( ! selector ) {
selector = registrySelector( wrappedSelector.registry.select );
lastRegistry = wrappedSelector.registry;
selectorsByRegistry.set( wrappedSelector.registry, selector );
}
return selector( ...args );
};
Expand Down
5 changes: 5 additions & 0 deletions packages/data/src/redux-store/index.js
Expand Up @@ -237,6 +237,11 @@ export default function createReduxStore( key, options ) {
const boundSelector = ( ...args ) => {
args = normalize( selector, args );
const state = store.__unstableOriginalGetState();
// Before calling the selector, switch to the correct
// registry.
if ( selector.isRegistrySelector ) {
selector.registry = registry;
}
return selector( state.root, ...args );
};

Expand Down
23 changes: 21 additions & 2 deletions packages/data/src/test/registry-selectors.js
Expand Up @@ -78,8 +78,7 @@ describe( 'createRegistrySelector', () => {
expect( registry.select( uiStore ).getElementCount() ).toBe( 1 );
} );

// Even without createSelector, this fails in trunk.
it.skip( 'can bind one selector to multiple registries', () => {
it( 'can bind one selector to multiple registries (createRegistrySelector)', () => {
const registry1 = createRegistry();
const registry2 = createRegistry();

Expand All @@ -102,6 +101,26 @@ describe( 'createRegistrySelector', () => {
expect( registry2.select( uiStore ).getElementCount() ).toBe( 1 );
} );

it( 'can bind one selector to multiple registries (createRegistrySelector + createSelector)', () => {
const registry1 = createRegistry();
registry1.register( elementsStore );
registry1.register( uiStore );
registry1.dispatch( elementsStore ).add( 'Carbon' );

const registry2 = createRegistry();
registry2.register( elementsStore );
registry2.register( uiStore );
registry2.dispatch( elementsStore ).add( 'Helium' );

// Expects the `getFilteredElements` to be bound separately and independently to the two registries
expect( registry1.select( uiStore ).getFilteredElements() ).toEqual( [
'Carbon',
] );
expect( registry2.select( uiStore ).getFilteredElements() ).toEqual( [
'Helium',
] );
} );

it( 'can bind a memoized selector to a registry', () => {
const registry = createRegistry();
registry.register( elementsStore );
Expand Down