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

Handbook - Fix incorrect example code for withSelect HOC #9079

Merged
merged 2 commits into from Aug 17, 2018

Conversation

Projects
None yet
3 participants
@mattheu
Contributor

mattheu commented Aug 17, 2018

Description

The docs in the handbook for the @wordpress/core-data package have a small omission in the example code for the withSelect HOC.

The docs in the package readme correctly show how this should be done.

How has this been tested?

NA - docs only.

Types of changes

Update documentation

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@mattheu mattheu changed the title from Fix incorrect example code for withSelect HOC to Handbook - Fix incorrect example code for withSelect HOC Aug 17, 2018

@tofumatt

Good catch, thanks! I tweaked the variable naming a bit so we we're redeclaring the function name (would that even work? I forget if function definitions can be overwritten like that).

@tofumatt tofumatt merged commit 38a6eff into WordPress:master Aug 17, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
authors: select( 'core' ).getAuthors(),
} ) );
} ) )( MyAuthorsListBase );

This comment has been minimized.

@aduth

aduth Aug 17, 2018

Member

AFAIK the only change necessary would have been to add ( MyAuthorsList ) invocation. The "Base" separation can make for being more explicit, though maybe an unnecessary / confusion distinction to make for those not already familiar with concepts of presentational and container components.

@aduth

aduth Aug 17, 2018

Member

AFAIK the only change necessary would have been to add ( MyAuthorsList ) invocation. The "Base" separation can make for being more explicit, though maybe an unnecessary / confusion distinction to make for those not already familiar with concepts of presentational and container components.

This comment has been minimized.

@tofumatt

tofumatt Aug 17, 2018

Member

That's true, but other examples use different variable names too, and I think it's less confusing to have different variable names. 🤷‍♂️

I think we should be encouraging this generally… but if I'm off-base we can change it.

@tofumatt

tofumatt Aug 17, 2018

Member

That's true, but other examples use different variable names too, and I think it's less confusing to have different variable names. 🤷‍♂️

I think we should be encouraging this generally… but if I'm off-base we can change it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment