Packages: Add new @wordpress/data-controls package.

[nerrad]

Description

This is a new package triggered in part by this and this comment.

The @wordpress/data-controls package exposes a set of common controls consumers can use when registering custom stores. In the initial iteration the following controls are included:

• select: For selecting from another data store in the registry.
• dispatch: For dispatching actions to another store in the registry.
• apiFetch: For doing a request via the @wordpress/api-fetch module.

Notes:

• The select control has been made resolver aware. Essentially this means if the named selector has a sibling resolver registered in the given store, then a promise is returned that resolves to the value for the selector after any necessary resolution is completed. If the selector does not have a resolver sibling then it is called directly. This allows for client code to not have to worry about resolution when using this control (Note: This requires a change in @wordpress/data: see @wordpress/data: Expose hasResolver property on returned selectors #15436).
• I've implemented this new package with the @wordpress/editor. Primarily I did this so that things could be tested but I'm on the fence with whether this should be done in its own pull. So I'm looking from feedback from reviewers on this. It turns out the the changes aren't that much (other than eliminating code 🎉) so I think it'd be good to merge with an initial implementation of the package.

Todos:

• Write package
• Update main package.json for the mono-repo
• Do pull for @wordpress/data exposing whether a selector has a resolver (see @wordpress/data: Expose hasResolver property on returned selectors #15436)
• Write README.md
• Write CHANGELOG.md
• Update docs builder and make sure auto-generated docs look okay.
• Add tests
• Implement this somewhere (@wordpress/editor) to ensure api is usable and works as expected.
• If the above ^^ stays in this branch and is released with this pull, then the appropriate CHANGELOG.md and tests will need updated.

How has this been tested?

• New unit tests added
• Implemented with @wordpress/editor and ensure the editor continues to work via user testing.
• e2e tests should still pass with this work.

Types of changes

• This is a non-breaking change introducing a new package.
• As an example of implementation, this has been implemented with the core/editor store. Thus the editor is impacted but impacted areas are covered by e2e and unit tests.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.

[gziolo]

I guess this should be reviewed by @youknowriad or @aduth. I can try myself but I didn't follow recent changes in this area.

I left my comments related to the package itself. Everything looks great, there are some tiny things to updated which I highlighted.

[gziolo]

@nosolosw, have you had a chance to revisit this list so we could use blacklist instead to make it easier to add new packages?

[oandregal]

It has grown on me that an allow list works better in this case for two reasons:

• I know we've talked about detecting the entry point from package.json. Note, however, how some packages will need more than this (example: core-data. This will require an allow list anyway.
• Using a deny list will do unnecessary extra work. Currently, docgen will report an error if it doesn't find a token in a README file, but this can be improved to gracefully continue. However, we'll need parsing the package entry point files anyway to know whether there is a token or not - wasting those cycles.

Adding/Removing a package is not a common operation, so I think is fine to require authors to do this explicitely if they want API doc generation.

[aduth]

First thought: Is apiFetch too specific to WordPress use case for broad applicability in such a package? I could imagine fetch being useful, and I forget the exact reasons @wordpress/api-fetch is not marketed as a generic fetch utility.

[aduth]

Thought: Should these (at least select, dispatch) be offered by the data package itself? Possibly also by default? The more I think on it, the more I think "No" (prefer opt-in), but mentioning as it occurs to me. Similarly there's the thought of respective packages offering controls (api-fetch), but I also think this bloats the scope of the modules to become aware of the data package and its specific mechanics.

[aduth]

This allows for client code to not have to worry about resolution when using this control

I like this enhancement 👍

[aduth]

I'd be more okay to start with just select and dispatch, but I could be convinced of apiFetch if the arguments are made. The code looks good.

[aduth]

We need to update this, since it has a default export. Otherwise the window global doesn't work as expected (wp.dataControls.default):

gutenberg/webpack.config.js

Lines 71 to 78 in 8960509

	new LibraryExportDefaultPlugin( [
	'api-fetch',
	'deprecated',
	'dom-ready',
	'redux-routine',
	'token-list',
	'shortcode',
	].map( camelCaseDash ) ),

Although, this is a unique case of needing both default and named exports. How do you propose this might be handled from the global? Is referencing the controls object by .default acceptable? It doesn't seem quite so obvious to me.

[nerrad]

I'd be more okay to start with just select and dispatch, but I could be convinced of apiFetch if the arguments are made.

Arguments for:

• arbitrary requests to an api is one of the more common use-cases for a control. Having a default utility in place for this is helpful for the end user.
• The package is only useful for consumers of the @wordpress/data package and registries implementing the controls plugin (available by default on the exposed defaultRegistry). Thus it makes sense that a fetch control would use the apiFetch utility from the @wordpress namespace.
• There is immediate benefit within the GB project using this library as nearly always these three controls will be sufficient.
• exposing these controls does not preclude implementors overriding or supplementing with their own controls. It just helps expose a set of sensible defaults that in most cases will be sufficient.

Although, this is a unique case of needing both default and named exports. How do you propose this might be handled from the global? Is referencing the controls object by .default acceptable? It doesn't seem quite so obvious to me.

Interesting, this was a specific about the build process I wasn't aware of. I'm also a bit surprised that the implementation here of the new package (in @wordpress/editor) didn't break because of that.

Typical usage will be importing the default for registering with the store (as done here) Are you thinking it'd be better to export it as a named export so that it's a bit more evident it's purpose on the global (i.e. wp.dataControls.controls)?

[nerrad]

Oh I see, the LibraryExportDefaultPlugin exports defaults onto the named global (eg. wp.apiFetch as opposed to wp.apiFetch.default)? I don't use ES5 that much so I don't notice these things. So to clarify, this means:

With configuration in webpack plugin

• wp.dataControls for default
• wp.dataControls.select, wp.dataControls.dispatch... etc for named exports? Edit: I tried this, no go.

Without configuration in webpack plugin

• wp.dataControls.default for default
• wp.dataControls.select, wp.dataControls.dispatch ...etc for named exports?

[aduth]

I agree controls as a named export seems sensible (arguably even preferable, since I don't know that it's as intuitive as the default export).

[jorgefilipecosta]

Merging this PR as it seems ready and was approved, and I have a PR depending on it that I would like to merge. If there are is any other improvement I think it can be shipped in a follow-up PR.

[jorgefilipecosta]

@jorgefilipecosta since you've implemented this in another branch, do you have a preference or some ideas here?

Sorry, I missed this question, personally like the version that was proposed:
wp.dataControls.controls for the controls object.
wp.dataControls.select
wp.dataControls.dispatch
wp.dataControls.apiFetch