Skip to content

Data: Fix wp.data.query backwards compatibility with props#5220

Merged
aduth merged 1 commit intomasterfrom
fix/query-deprecated
Mar 2, 2018
Merged

Data: Fix wp.data.query backwards compatibility with props#5220
aduth merged 1 commit intomasterfrom
fix/query-deprecated

Conversation

@aduth
Copy link
Copy Markdown
Member

@aduth aduth commented Feb 23, 2018

Related: #5137

This pull request seeks to resolve an issue where the wp.data.query backwards-compatibility added with the introduction of wp.data.withSelect does not correctly pass props. In the course of revisions to #5137, it was overlooked to use the final signature of ( select, props ) on withSelect. No errors occurred only because none of the core components had used the ownProps argument of the query higher-order component.

Testing instructions:

Ensure unit tests pass:

npm run test-unit

As of #5215, no core components use wp.data.query, so this cannot be easily tested in the browser without manual changes.

@aduth aduth added the Framework Issues related to broader framework topics, especially as it relates to javascript label Feb 23, 2018
registerReducer( 'reactReducer', () => ( { reactKey: 'reactState' } ) );
registerSelectors( 'reactReducer', {
reactSelector: ( state, key ) => state[ key ],
function cases( withSelectImpl, extraAssertions = noop ) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

None of the tests themselves have changed, just putting in a function to allow testing both withSelect and query separately with same expectations (except console.warn in the latter).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The function name is unclear to me :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The function name is unclear to me :)

I broke my own rule of no abbreviations 😄 It's a semi-common convention for "Implementation". I've unabbreviated it.

Copy link
Copy Markdown
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@aduth aduth force-pushed the fix/query-deprecated branch from 66fdda5 to 9e5ccd6 Compare March 2, 2018 13:41
@aduth aduth merged commit 6b47489 into master Mar 2, 2018
@aduth aduth deleted the fix/query-deprecated branch March 2, 2018 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

2 participants