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

Adds documentation about selectors #52941

Merged
merged 11 commits into from Jul 28, 2023

Conversation

mburridge
Copy link
Contributor

Closes #17082

What?

Adds documentation for the following selectors:

  • hasFinishedResolution
  • hasStartedResolution
  • isResolving

to the README file for the @wordpress/data package.

Why?

As per #17082

How?

Doc blocks copied from selectors.js

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

@mburridge mburridge added the [Type] Developer Documentation Documentation for developers label Jul 25, 2023
@mburridge mburridge requested a review from nerrad as a code owner July 25, 2023 14:43
Copy link
Member

@ndiego ndiego left a comment

Choose a reason for hiding this comment

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

I think this is good to merge, but in looking at the linked issue, the author is looking for examples of how to use each. So I would add a follow-up issue to add examples to the doc blocks of each selector.

@mburridge
Copy link
Contributor Author

@ndiego I've added an example. Please review.

@ryanwelcher Please sanity check the code example. Is it too specific? Should it be more generalised?

@mburridge mburridge requested a review from ndiego July 25, 2023 16:17
Copy link
Contributor

@ryanwelcher ryanwelcher left a comment

Choose a reason for hiding this comment

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

There are a few issues with the snippet mentioned below. Additionally, we should also adhere to the JavaScript coding standards when it comes to spacing. I have added some suggestions for spacing but there are also some changes needed for line length etc.

I have take the liberty of formatting the code locally for reference:

import { useSelect } from '@wordpress/data';
import { store as coreDataStore } from '@wordpress/core-data';

function Component() {
	const result = useSelect( ( select ) => {
		const query = { per_page: 20 };
		const selectorArgs = [ 'postType', 'page', query ];

		return {
			pages: select( coreDataStore ).getEntityRecords( ...selectorArgs ),
			hasStartedResolution: select( coreDataStore ).hasStartedResolution(
				'getEntityRecords', // _selectorName_
				selectorArgs
			),
			hasFinishedResolution: select(
				coreDataStore
			).hasFinishedResolution( 'getEntityRecords', selectorArgs ),
			isResolving: select( coreDataStore ).isResolving(
				'getEntityRecords',
				selectorArgs
			),
		};
	} );

	console.log( result.hasStartedResolution );
	console.log( result.hasFinishedResolution );
	console.log( result.isResolving );
	console.log( result.pages );

	return null;
}

packages/data/README.md Outdated Show resolved Hide resolved
packages/data/README.md Outdated Show resolved Hide resolved
packages/data/README.md Outdated Show resolved Hide resolved

console.log(result.hasStartedResolution);
console.log(result.hasFinishedResolution);
console.log(result.isResolving);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.log(result.isResolving);
console.log(result.isResolving);
console.log( result.pages );

Adding for completeness.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is necessary as we're talking about the selectors from the @wordpress/data package here.

packages/data/README.md Outdated Show resolved Hide resolved
packages/data/README.md Outdated Show resolved Hide resolved
packages/data/README.md Outdated Show resolved Hide resolved
packages/data/README.md Outdated Show resolved Hide resolved
packages/data/README.md Outdated Show resolved Hide resolved
mburridge and others added 8 commits July 27, 2023 16:14
Co-authored-by: Ryan Welcher <me@ryanwelcher.com>
Co-authored-by: Ryan Welcher <me@ryanwelcher.com>
Co-authored-by: Ryan Welcher <me@ryanwelcher.com>
Co-authored-by: Ryan Welcher <me@ryanwelcher.com>
Co-authored-by: Ryan Welcher <me@ryanwelcher.com>
Co-authored-by: Ryan Welcher <me@ryanwelcher.com>
Co-authored-by: Ryan Welcher <me@ryanwelcher.com>
Copy link
Contributor

@ryanwelcher ryanwelcher left a comment

Choose a reason for hiding this comment

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

LTGM!

@mburridge mburridge merged commit 5c3d93a into WordPress:trunk Jul 28, 2023
48 checks passed
@mburridge mburridge deleted the docs/core-data-selectors branch July 28, 2023 09:17
@github-actions github-actions bot added this to the Gutenberg 16.4 milestone Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation: Add information about core/data isResolving
3 participants