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.

Sign up for GitHub

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

Merge selector-binding code (except resolvers) into a new bindSelector function #51176

Merged
merged 1 commit into from
Jun 7, 2023
Merged

Merge selector-binding code (except resolvers) into a new bindSelector function #51176

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 28 additions & 51 deletions packages/data/src/redux-store/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,25 +197,31 @@ export default function createReduxStore( key, options ) {
} )
);

let selectors = mapSelectors(
{
...mapValues(
metadataSelectors,
( selector ) =>
( state, ...args ) =>
selector( state.metadata, ...args )
),
...mapValues( options.selectors, ( selector ) => {
if ( selector.isRegistrySelector ) {
selector.registry = registry;
}
function bindSelector( selector ) {
if ( selector.isRegistrySelector ) {
selector.registry = registry;
}
const boundSelector = ( ...args ) => {
const state = store.__unstableOriginalGetState();
return selector( state.root, ...args );
};
boundSelector.hasResolver = false;
return boundSelector;
Comment on lines +204 to +209
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this part is the same as bindMetadataSelector(), the only difference being where the selector reads state from, I wonder if we could further abstract this fragment into a separate function that receives the state tree name. Then we could reuse that in both bindSelector and bindMetadataSelector.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally did this, if you look at the implementation in #51051, there are calls bindSelector( 'root' ) and bindSelector( 'metadata' ).

But now I plan to diverge the two versions even more. The metadata selectors only need the binding, we cal remove the hasResolver part. And when binding the classic selector, I would like to try merging the resolver fulfillment also into bindSelector. So that there is only one wrapper and when in debugger, you step into a call like:

registry.select( 'core/block-editor' ).getBlocks();

you are just one step away from the actual implementation of getBlocks. Until recently, you'd have to step through several layers of wrappers.

}

return ( state, ...args ) =>
selector( state.root, ...args );
} ),
},
store
);
function bindMetadataSelector( selector ) {
const boundSelector = ( ...args ) => {
const state = store.__unstableOriginalGetState();
return selector( state.metadata, ...args );
};
boundSelector.hasResolver = false;
return boundSelector;
}

let selectors = {
...mapValues( metadataSelectors, bindMetadataSelector ),
...mapValues( options.selectors, bindSelector ),
};

let resolvers;
if ( options.resolvers ) {
Expand All @@ -232,19 +238,10 @@ export default function createReduxStore( key, options ) {
selectors,
new Proxy( privateSelectors, {
get: ( target, prop ) => {
return (
mapSelectors(
mapValues( privateSelectors, ( selector ) => {
if ( selector.isRegistrySelector ) {
selector.registry = registry;
}

return ( state, ...args ) =>
selector( state.root, ...args );
} ),
store
)[ prop ] || selectors[ prop ]
);
const privateSelector = privateSelectors[ prop ];
return privateSelector
? bindSelector( privateSelector )
: selectors[ prop ];
},
} )
);
Expand Down Expand Up @@ -362,26 +359,6 @@ function instantiateReduxStore( key, options, registry, thunkArgs ) {
);
}

/**
* Maps selectors to a store.
*
* @param {Object} selectors Selectors to register. Keys will be used as the
* public facing API. Selectors will get passed the
* state as first argument.
* @param {Object} store The store to which the selectors should be mapped.
* @return {Object} Selectors mapped to the provided store.
*/
function mapSelectors( selectors, store ) {
const createStateSelector = ( registrySelector ) => {
const selector = ( ...args ) =>
registrySelector( store.__unstableOriginalGetState(), ...args );
selector.hasResolver = false;
return selector;
};

return mapValues( selectors, createStateSelector );
}

/**
* Maps selectors to functions that return a resolution promise for them
*
Expand Down