Skip to content

Conversation

@aduth
Copy link
Member

@aduth aduth commented Feb 12, 2018

Related: #4105

This pull request seeks to update the data module selector pattern to expose selectors as functions directly, not invoke via named string arguments.

Before:

select( 'core/editor', 'getEditedPostAttribute', 'title' );

After:

select( 'core/editor' ).getEditedPostAttribute( 'title' );

Implementation notes:

Since this was included in 2.1.0 of the plugin, do we need to include a deprecation upgrade path? Added in 338e5a8.

Testing instructions:

Ensure unit tests pass:

npm test

Verify there are no regressions in the current usage of data module select.

@aduth aduth added the Framework Issues related to broader framework topics, especially as it relates to javascript label Feb 12, 2018
@aduth
Copy link
Member Author

aduth commented Feb 12, 2018

Added upgrade path for deprecated usage in 338e5a8.

Copy link
Member

@atimmer atimmer left a comment

Choose a reason for hiding this comment

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

I really like this. Makes it much more discoverable. In a similar version I explored in #4421 I had all selectors exposed at wp.data.selectors. Did you also consider this?

Anyhow, looks good to me.

@gziolo
Copy link
Member

gziolo commented Feb 12, 2018

This is how I always wanted to have it shaped 💯

Copy link
Contributor

@mcsf mcsf left a comment

Choose a reason for hiding this comment

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

A worthwhile change for me. 👌

selectors[ reducerKey ] = newSelectors;
const store = stores[ reducerKey ];
const createStateSelector = ( selector ) => ( ...args ) => selector( store.getState(), ...args );
selectors[ reducerKey ] = mapValues( newSelectors, createStateSelector );
Copy link
Contributor

Choose a reason for hiding this comment

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

Initially, I wanted to avoid having to create a wrapper for each selector but I do see the DevX benefits from this new API

@youknowriad youknowriad added the [Type] Breaking Change For PRs that introduce a change that will break existing functionality label Feb 13, 2018
@gziolo gziolo added the [Feature] Extensibility The ability to extend blocks or the editing experience label Feb 13, 2018
@aduth aduth removed the [Type] Breaking Change For PRs that introduce a change that will break existing functionality label Feb 13, 2018
@aduth aduth force-pushed the update/data-bound-selectors branch 2 times, most recently from e306914 to 08a47de Compare February 13, 2018 15:39
@aduth aduth force-pushed the update/data-bound-selectors branch from 08a47de to 0cbecd1 Compare February 13, 2018 16:31
@aduth aduth merged commit 94cc0b0 into master Feb 13, 2018
@aduth aduth deleted the update/data-bound-selectors branch February 13, 2018 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Extensibility The ability to extend blocks or the editing experience Framework Issues related to broader framework topics, especially as it relates to javascript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants