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

PoC: Support private hooks to explore Data Views actions #61412

Closed
wants to merge 1 commit into from

Conversation

mcsf
Copy link
Contributor

@mcsf mcsf commented May 6, 2024

Motivation

Actions and commands

In the context of Data Views, there has been a lot of recent work towards providing a set of actions operating on posts, templates, patterns (e.g. rename post, edit post, duplicate template), and ultimately other entities. These actions, however, aren't unique to Data Views, and indeed exist in several different contexts (e.g. Site Editor inner sidebar, new Admin "shell" sidebar, Pages index view, Post Editor), so the next step was to unify actions across packages (e.g. #60486, #60754).

The first unification effort led to an abstraction around a hook, usePostActions, but the consensus now is to remove it and expose the actions directly (#61040).

Meanwhile, it has been noted that there is a strong parallel between these actions and the Command Palette's commands, which has its own API already. This isn't a 1:1 mapping, but we should determine what the overlap is.

Actions and side effects

There is a limit to how much we can unify, because the context in which actions are triggered will determine what secondary effects are desired. For example, trashing a post inside the post editor should result in the client navigating elsewhere (e.g. edit.php), but there should be no such effect when trashing from a Data View index.

The current solution for this is to let consumers of the PostActions component pass a callback as onActionPerformed. It works but there's a risk that it's too flexible, so I kept wondering about what kind of generalisations we could make here before we opened this up as an API.

Extensibility

As tracked in #61084, our system -- what ever it turns to be -- needs to become extensible soon. Somewhere in our GitHub conversations there was a suggestion to set up an imperative API like registerAction that third parties could leverage. I think that's fair, though we'll need to determine what kind of registry we want (scope and plurality).

An imperative API that can be called in an initialisation step rather than as a call inside the render tree (e.g. <Provider value=...> or useRegisterAction(...)) is more convenient for developers, but introduces indirection. In this scenario, how do we implement those aforementioned contextual side effects (e.g. navigate to page)?

The experiment

It was in this context that I had the terrible thought of leveraging wp.hooks to provide a private API (to dogfood in Gutenberg core packages). But, of course, hooks are keyed by strings, and so they are necessarily public -- i.e., a third party can call applyFilters('privateFilter', even if privateFilter is not meant to be used outside of core.

This branch changes that assumption: hook names must be strings, except if they match a small set of hard-coded symbols. These symbols are only accessible via the lock/unlock API powered by the private-apis package. Thus, core packages can communicate amongst each other via hooks that no third party can use. For example:

  • An action triggers doAction with a symbol corresponding to its name (e.g. postActions.renamePost).
  • A consumer of actions, like the Page index view (PagePages), triggers a more contextual action (e.g. pagePages.renamePost).
  • A different component hooks to one of these actions, according to the intended specificity, to trigger a side effect like navigation.

See for yourself: upon pagePages.editPost, the necessary navigation to said post is triggered by a subscriber of that action.

Assessment

Having tried it, I think this is a poor idea. "Private hooks" as a concept is a cool way to see how far private-apis can take us, but they seem like the wrong tool for the current problem. Still, I wanted to share the work, hence this verbose commit.

I think our next steps should be:

  • Finish the actions refactor (Refactor: Remove post actions hook. #61040)
  • Impose constraints on ourselves to try to achieve our feature goals with less powerful constructs than onActionPerformed. I'm still convinced we haven't done enough work to generalise side effects. Consider it along with the commands API.
  • Try a more classic registry-based approach for actions (registerAction)

Motivation
==========

Actions and commands
--------------------

In the context of Data Views, there has been a lot of recent work
towards providing a set of actions operating on posts, templates,
patterns (e.g. rename post, edit post, duplicate template), and
ultimately other entities. These actions, however, aren't unique to Data
Views, and indeed exist in several different contexts (e.g. Site Editor
inner sidebar, new Admin "shell" sidebar, Pages index view, Post
Editor), so the next step was to unify actions across packages
(e.g. #60486, #60754).

The first unification effort led to an abstraction around a hook,
`usePostActions`, but the consensus now is to remove it and expose the
actions directly (#61040).

Meanwhile, it has been noted that there is a strong parallel between
these _actions_ and the Command Palette's _commands_, which has its own
API already. This isn't a 1:1 mapping, but we should determine what the
overlap is.

Actions and side effects
------------------------

There is a limit to how much we can unify, because the context in which
actions are triggered will determine what secondary effects are desired.
For example, trashing a post inside the post editor should result in the
client navigating elsewhere (e.g. edit.php), but there should be no such
effect when trashing from a Data View index.

The current solution for this is to let consumers of the `PostActions`
component pass a callback as `onActionPerformed`. It works but there's a
risk that it's too flexible, so I kept wondering about what kind of
generalisations we could make here before we opened this up as an API.

Extensibility
-------------

As tracked in #61084, our system -- what ever it turns to be -- needs to
become extensible soon. Somewhere in our GitHub conversations there was
a suggestion to set up an imperative API like `registerAction` that
third parties could leverage. I think that's fair, though we'll need to
determine what kind of registry we want (scope and plurality).

An imperative API that can be called in an initialisation step rather
than as a call inside the render tree (e.g. `<Provider value=...>` or
`useRegisterAction(...)`) is more convenient for developers, but
introduces indirection. In this scenario, how do we implement those
aforementioned _contextual side effects_ (e.g. navigate to page)?

The experiment
==============

It was in this context that I had the terrible thought of leveraging
wp.hooks to provide a private API (to dogfood in Gutenberg core
packages). But, of course, hooks are keyed by strings, and so they are
necessarily public -- i.e., a third party can call
`applyFilters('privateFilter'`, even if `privateFilter` is not meant to
be used outside of core.

This branch changes that assumption: hook names *must* be strings,
*except* if they match a small set of hard-coded symbols. These symbols
are only accessible via the lock/unlock API powered by the
`private-apis` package. Thus, core packages can communicate amongst each
other via hooks that no third party can use. For example:

- An action triggers `doAction` with a symbol corresponding to its name
  (e.g. `postActions.renamePost`).
- A consumer of actions, like the Page index view (PagePages), triggers
  a more contextual action (e.g. `pagePages.renamePost`).
- A different component hooks to one of these actions, according to the
  intended specificity, to trigger a side effect like navigation.

See for yourself: upon `pagePages.editPost`, the necessary navigation to
said post is triggered by a subscriber of that action.

Assessment
==========

Having tried it, I think this is a poor idea. "Private hooks" as a
concept is a cool way to see how far `private-apis` can take us, but
they seem like the wrong tool for the current problem. Still, I wanted
to share the work, hence this verbose commit.

I think our next steps should be:

- Finish the actions refactor (#61040)
- Impose constraints on ourselves to try to achieve our feature goals
  with less powerful constructs than `onActionPerformed`. I'm still
  convinced we haven't done enough work to generalise side effects.
  Consider it along with the commands API.
- Try a more classic registry-based approach for actions
  (`registerAction`)
@mcsf mcsf added [Type] Experimental Experimental feature or API. [Feature] Data Views Work surrounding upgrading and evolving views in the site editor and beyond labels May 6, 2024
@mcsf
Copy link
Contributor Author

mcsf commented May 21, 2024

I don't think there's much to pursue here — closing.

@mcsf mcsf closed this May 21, 2024
@mcsf mcsf deleted the try/hooks-with-symbols branch May 21, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Data Views Work surrounding upgrading and evolving views in the site editor and beyond [Type] Experimental Experimental feature or API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant