Update wordpress/data documentation in order to prefer store object instead of store name to access the store #41210

renatho · 2022-05-20T20:09:25Z

What?

It updates part of the documentation (@wordpress/data) to use store object instead of store names when accessing the stores through the select and dispatch functions.

Why?

It's a new standard. The other way should be considered as a legacy way only for backward compatibility.

it might be clearer if we do have an import, since actual code doing this will have to have the import as well. we can try and make the filename something that's obviously example code:

import { dispatch } from '@wordpress/data'; import { store as myCustomStore } from 'my-custom-store'; dispatch( myCustomStore ).setPrice( 'hammer', 9.75 );

By the way the descriptions of these functions are generated from the actual code. Instead of editing it here we need to edit it in the source, which for this one is data/src/index.js.

Otherwise this change will be wiped out the next time someone makes a change to the source file and it will be confusing.

It would be worth updating the description of the above-mentioned source file (data/src/index.js) because it shouldn't say "Given the name of a registered store" but should say something like "Given a store descriptor returns an object of…(also supports legacy calling convention accepting the name of a registered store)."

@dmsnell, thank you for the great suggestions! I applied them and I updated the documentation with npm run docs:build.

2e3fabd

It would be worth updating the description of the above-mentioned source file (data/src/index.js) because it shouldn't say "Given the name of a registered store" but should say something like "Given a store descriptor returns an object of…(also supports legacy calling convention accepting the name of a registered store)."

a79b8a7

dmsnell · 2022-05-20T20:33:29Z

packages/data/README.md

@@ -481,7 +485,7 @@ _Usage_
 ```js
 import { dispatch } from '@wordpress/data';

-dispatch( 'my-shop' ).setPrice( 'hammer', 9.75 );
+dispatch( store ).setPrice( 'hammer', 9.75 );


it might be clearer if we do have an import, since actual code doing this will have to have the import as well. we can try and make the filename something that's obviously example code:

import { dispatch } from '@wordpress/data'; import { store as myCustomStore } from 'my-custom-store'; dispatch( myCustomStore ).setPrice( 'hammer', 9.75 );

dmsnell · 2022-05-20T20:36:08Z

packages/data/README.md

@@ -481,7 +485,7 @@ _Usage_
 ```js
 import { dispatch } from '@wordpress/data';

-dispatch( 'my-shop' ).setPrice( 'hammer', 9.75 );
+dispatch( store ).setPrice( 'hammer', 9.75 );


By the way the descriptions of these functions are generated from the actual code. Instead of editing it here we need to edit it in the source, which for this one is data/src/index.js.

Otherwise this change will be wiped out the next time someone makes a change to the source file and it will be confusing.

dmsnell · 2022-05-20T20:37:29Z

packages/data/README.md

@@ -481,7 +485,7 @@ _Usage_
 ```js
 import { dispatch } from '@wordpress/data';

-dispatch( 'my-shop' ).setPrice( 'hammer', 9.75 );
+dispatch( store ).setPrice( 'hammer', 9.75 );


It would be worth updating the description of the above-mentioned source file (data/src/index.js) because it shouldn't say "Given the name of a registered store" but should say something like "Given a store descriptor returns an object of…(also supports legacy calling convention accepting the name of a registered store)."

dmsnell · 2022-05-20T23:00:47Z

packages/data/src/index.js

- * The selector functions are been pre-bound to pass the current state automatically.
+ * Given a store descriptor (also supports legacy calling convention accepting the name of a
+ * registered store), returns an object of the store's selectors. The selector functions are
+ * been pre-bound to pass the current state automatically.


given that we want to discourage use of passing the string I feel like we could de-emphasize explaining the legacy usage and move it either to the end of the description here, or as an additional note inside the JSDoc area

/** * Given a store descriptor returns an object of the store's selectors. * The selector functions are pre-bound to pass the current state automatically. * * Note: The legacy calling convention of passing the store name is also supported. * * @example * … */

Same applies for all these case, but take my suggestion with a grain of salt. Others might feel differently.

I updated it to add the explanation of the param in the JSDoc param. And I also updated the type to StoreDescriptor|string instead of string|StoreDescriptor. WDYT?

973c18a

thanks for rearranging those types. although it doesn't change anything from a TypeScript parameter, I do think you made it clearer still, and I had almost suggested it but didn't want to bog down this change.

packages/data/README.md

dmsnell

Seems like an overall positive move for the documentation. I'm sure we can continue to improve it, but this is good enough to be clearer than what we have IMO.

cc: @jsnajdr since you also have worked on these.

renatho · 2022-06-02T18:09:46Z

Hey folks! Should we go ahead and merge this?

renatho · 2022-06-29T20:25:09Z

Hey folks! 👋
I don't have the authorization to merge this PR. I'd appreciate it if someone could do that to update the documentation. 🙂

dmsnell · 2022-06-30T08:28:02Z

Sorry @renatho I missed that you couldn't merge. Can you rebase to trigger test runs again? Looks like a flakey test falsely failed this.

renatho · 2022-06-30T14:31:57Z

Sorry @renatho I missed that you couldn't merge. Can you rebase to trigger test runs again? Looks like a flakey test falsely failed this.

Hey @dmsnell! No problem! Thank you for checking this! 😊
I rebased it and everything is green now. ✅

renatho requested a review from nerrad as a code owner May 20, 2022 20:09

renatho commented May 20, 2022

View reviewed changes

dmsnell requested changes May 20, 2022

View reviewed changes

renatho force-pushed the update/wordpress-data-documentation branch from 7bb1010 to a79b8a7 Compare May 20, 2022 21:55

renatho requested a review from dmsnell May 20, 2022 21:58

dmsnell reviewed May 20, 2022

View reviewed changes

ZebulanStanphill reviewed May 21, 2022

View reviewed changes

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

gziolo added [Type] Developer Documentation Documentation for developers [Package] Data /packages/data labels May 21, 2022

renatho requested review from ZebulanStanphill and dmsnell May 23, 2022 17:20

dmsnell approved these changes May 25, 2022

View reviewed changes

renatho added 5 commits June 30, 2022 10:49

Update documentation in order to use store object instead of store name

5918227

Update the documentation in the source of truth

e0a84dc

Update text description in order to suggest using store descriptor

3232834

Update documentation explaining store descriptor param

ea8a38d

Update storeNameOrDescriptor description

c8fb588

renatho force-pushed the update/wordpress-data-documentation branch from 2ff83cd to c8fb588 Compare June 30, 2022 13:50

dmsnell merged commit 993836f into WordPress:trunk Jun 30, 2022

github-actions bot added this to the Gutenberg 13.7 milestone Jun 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update wordpress/data documentation in order to prefer store object instead of store name to access the store #41210

Update wordpress/data documentation in order to prefer store object instead of store name to access the store #41210

renatho commented May 20, 2022 •

edited

Loading

renatho May 20, 2022

dmsnell May 20, 2022

dmsnell May 20, 2022

dmsnell May 20, 2022

renatho May 20, 2022

dmsnell May 20, 2022

dmsnell May 20, 2022

dmsnell May 20, 2022

dmsnell May 20, 2022 •

edited

Loading

renatho May 23, 2022 •

edited

Loading

dmsnell May 23, 2022

dmsnell left a comment

renatho commented Jun 2, 2022

renatho commented Jun 29, 2022

dmsnell commented Jun 30, 2022

renatho commented Jun 30, 2022

Update wordpress/data documentation in order to prefer store object instead of store name to access the store #41210

Update wordpress/data documentation in order to prefer store object instead of store name to access the store #41210

Conversation

renatho commented May 20, 2022 • edited Loading

What?

Why?

Next

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmsnell May 20, 2022 • edited Loading

Choose a reason for hiding this comment

renatho May 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmsnell left a comment

Choose a reason for hiding this comment

renatho commented Jun 2, 2022

renatho commented Jun 29, 2022

dmsnell commented Jun 30, 2022

renatho commented Jun 30, 2022

renatho commented May 20, 2022 •

edited

Loading

dmsnell May 20, 2022 •

edited

Loading

renatho May 23, 2022 •

edited

Loading