Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

Rename wp_store to something else #136

Closed
luisherranz opened this issue Jan 23, 2023 · 25 comments
Closed

Rename wp_store to something else #136

luisherranz opened this issue Jan 23, 2023 · 25 comments

Comments

@luisherranz
Copy link
Member

luisherranz commented Jan 23, 2023

We've wanted to find a better name for what we've been calling wpx() for a while, and @michalczaplinski just suggested using store():

// render.php
<?php
store([
  "state" => [
    "isOpen" => false
  ]
]);
?>

<div>
  <button data-wp-on:click="actions.toggle">Toggle</button>
  <div data-wp-show="state.isOpen">
    Some text...
  </div>
</div>
// view.js
import { store } from '@wordpress/interactivity';

store({
  actions: {
    toggle: ({ context }) => {
      context.isOpen = !context.isOpen
    },
  },
});

I'm not sure if the WordPress community would accept such a generic name, but I like it because it is explicit, and if we can, I'd prefer to avoid more than one word to avoid the camelcase vs snakecase confusion. I.e., addStore vs add_store.

Any thoughts? Other ideas?

@michalczaplinski
Copy link
Collaborator

I like the store() name! My second-place suggestion was to call it createStore() but I agree with @luisherranz that it adds to the camelcase vs snakecase confusion.

There's some prior art for naming in this space:

  • Solid uses the create* naming: createStore(), createEffect()
  • Vue's Composition API went with single-word names that are somewhat similar. Their reactive() is our store(), the ref() is basically signal(), etc.

@SantosGuillamot
Copy link
Contributor

I like store() as well. I agree that keeping it with just one word would be better. Anyway, if we finally have to add something more verbose, I believe it won't be a big issue. It is something we are already doing. For example, registerBlockType vs register_block_type.

@DAreRodz
Copy link
Collaborator

I agree with Michal and Mario―maybe we are a bit biased with store(), though, as that's the name we're most used to. 😝

@luisherranz
Copy link
Member Author

Let's use store() for now, then. Thanks, folks.

@luisherranz
Copy link
Member Author

@gziolo mentioned to me that store is too general and can conflict with plugins. We would need to use wp_store or something similar.

I'm reopening so we can discuss again.

@gziolo
Copy link
Member

gziolo commented Feb 17, 2023

@gziolo mentioned to me that store is too general and can conflict with plugins. We would need to use wp_store or something similar.

It only applies to PHP. On the JavaScript side, devs import store from the npm package, so it's namespaced and won't ever become an issue.

Technically speaking, store() could be used as the name for the function, as it isn't used in WordPress core. The best way is to check Developer Resources page. I'm not aware of exact naming conventions enforced today, but many recently added functions tend to get prefixed with wp_ because WordPress core didn't adopt namespaces.

@luisherranz
Copy link
Member Author

Pardon my ignorance, but is the convention that plugins must not use function names that start with wp_, hence wp_store is safe, but they can use function names that don't have a prefix, hence store is unsafe? Is that the case?

Is there any other resource about function names other than what is in the PHP Coding Standards?

@luisherranz luisherranz changed the title Rename wpx to store Rename wpx to something else Feb 17, 2023
@gziolo
Copy link
Member

gziolo commented Feb 17, 2023

Pardon my ignorance, but is the convention that plugins must not use function names that start with wp_, hence wp_store is safe, but they can use function names that don't have a prefix, hence store is unsafe? Is that the case?

Yes, I know how it sounds, and your response is spot on.

Is there any other resource about function names other than what is in the PHP Coding Standards?

I think it's becoming "safer" when plugin developers follow Best Practice: Prefix Everything. The best approach would be to ask on WordPress Slack in the #core channel.

@luisherranz
Copy link
Member Author

luisherranz commented Feb 20, 2023

So, what do you think about wp_store for PHP and store (import { store } from ...) in JavaScript?

@gziolo
Copy link
Member

gziolo commented Feb 20, 2023

So, what do you think about wp_store for PHP and store (import { store } from ...) in JavaScript?

It made me think that someone had to make similar decisions when bringing WP Hooks and i18n helpers to JavaScript. @adamsilverstein, or @swissspidy do you have any recommendations?

I see that for hooks, API looks the same but follows language-specific coding styles, in effect:

  • doAction vs do_action
  • addAction vs add_action
  • applyFilters vs apply_filters
  • addFilter vs add_filter

For i18n it seems like it was possible to achieve one to one match for equivalent methods.

@swissspidy
Copy link
Member

Not sure if these are good examples, because there we ported already very established PHP functions to JS, not vice-versa. Here it's a new thing for both sides.

My 2 cents after a quick glance:

"store" is a very generic term and without additional context a (new) developer would probably not understand what a store() or wp_store() function does in PHP.

For JS it doesn't seem a big issue as you import from the package, which gives you that additional context. Keeping store() there sounds reasonable.

But for PHP, could it perhaps be named more verbose, e.g. wp_block_store() or something?

@luisherranz
Copy link
Member Author

Thanks, Pascal 🙂

I think it makes sense to find something more verbose. But until we do, let's switch from store to wp_store to adapt to this convention:

[...] plugins must not use function names that start with wp_, hence wp_store is safe, but they can use function names that don't have a prefix, hence store is unsafe.

@SantosGuillamot
Copy link
Contributor

Closing the issue as the decision was taken: We'll use wp_store for PHP and store in JavaScript.

We can always reconsider it and reopen the issue if needed.

@mtias
Copy link
Member

mtias commented Mar 25, 2023

I also think wp_store in php context is way too generic and hard to relate to what it does. Is it about saving things to the database? Is it about commerce? It just feels rather inscrutable.

@michalczaplinski
Copy link
Collaborator

Reopening in light of the comment from Matias above.

Suggestions to brainstorm this some more:

  • wp_interactivity_store
  • wp_data_store
  • wp_create_interactive_store (that might imply adding the create_ prefix to the JS function.
  • Some other combination of wp_, data, store & interactivity ?

@michalczaplinski michalczaplinski changed the title Rename wpx to something else Rename wp_store to something else Mar 27, 2023
@westonruter
Copy link
Member

What about state? That's what is being stored here, right?

  • wp_init_block_state()
  • wp_set_state()
  • wp_interactivity_state()

For me, speaking of state feels more clear than store.

@michalczaplinski
Copy link
Collaborator

What about state? That's what is being stored here, right?

Yes, that's true at the moment, and I like those suggestions! 🙂

That said, there are some (still very distant) ideas of serializing more than just the state. Whatever name we come up with, I'd like to plan for that eventuality. So, either we have to be fine with potentially creating a new function in the future (if we end up serializing more than just the state), or we find a more flexible name now.

@johnhooks
Copy link

Total new voice here, but interactive is a really long prefix, and this really is intended to be like the way (in my mind) to do block based UI, what about wp_ui_state? Is the wp_ui prefix used by any other system in WordPress?

@westonruter
Copy link
Member

there are some (still very distant) ideas of serializing more than just the state

Do tell 😄

@michalczaplinski
Copy link
Collaborator

Do tell 😄

I really can't say too much! Not because I don't want to, it's just that the ideas are not yet well-formed. Also don't want to derail this thread 🙂.

There are some ideas from over a year ago in WordPress/gutenberg#38224 exploring compiling a template format to both JS and PHP.

More recently, some of us working in this repo have talked about building a similar template compiler where the data-wp-* directives would be the "compile target". There is not even a prototype of this, and it might never even happen. How would it work exactly? We also have no idea yet 🙂 .

If you want to talk about it some more, let's open a new issue, and let's cc @luisherranz, who might have some more thoughts on this front.

@griffbrad
Copy link

I think wp_block_store or wp_block_state could work well.

@luisherranz
Copy link
Member Author

@luisherranz, who might have some more thoughts on this front

I don't think we need to serialize anything else than state. The state property is the source of truth and everything else should be derived from it.

The store, however, can contain other properties, like selectors:

$counter = 0;

wp_store( array(
  'state' => array(
    'myPlugin' => array(
      'counter' => $counter,
    ),
  ),
  'selectors' => array(
    'myPlugin' => array(
      'double' => $counter * 2,
    ),
  ),
) );
store({
  state: {
    myPlugin: {
      counter: 0, // Not really needed if you have declared it in the server
    },
  },
  selectors: {
    myPlugin: {
      double: ({ state }) => state.myPlugin.counter * 2,
    },
  },
});

Whether there's a fixed number of properties (state, selectors, actions, effects) or not, it's up for discussion. I'm inclined to say people can add whatever they want, and only state is special (serialized and reactive).


I reckon store may not be a straightforward name, but I'd rather not refer to it using state because it includes the state property and maybe others (selectors) so I think it could also be confusing.

wp_xxx_state( array(
  'state' => array(
    'myPlugin' => array(
      'counter' => $counter,
    ),
  ),
  'selectors' => array(
    'myPlugin' => array(
      'double' => $counter * 2,
    ),
  ),
) );

@gziolo
Copy link
Member

gziolo commented Mar 31, 2023

I reckon store may not be a straightforward name, but I'd rather not refer to it using state because it includes the state property and maybe others (selectors) so I think it could also be confusing.

One more note here, the state is global, so the name would end up somewhere close to wp_blocks_state. It sounds really weird when you read it without underscores 🤣

@ajgagnon
Copy link

ajgagnon commented Jun 1, 2023

What about state? That's what is being stored here, right?

wp_init_block_state()
wp_set_state()
wp_interactivity_state()
For me, speaking of state feels more clear than store.

The store, however, can contain other properties, like selectors:

I would highly suggest to use the word store. It's already the standard naming convention for state/store mechanisms like redux, vuex, etc.

No need to reinvent the wheel. Developers will already know what this means which will eliminate the cognitive load of learning new naming conventions.

@luisherranz
Copy link
Member Author

I'm going to close this issue as part of the migration to the Gutenberg repository. Please, feel free to open a new discussion in the Interactivity API category to discuss this matter further.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests