Skip to content

Commit

Permalink
PoC: Support private hooks to explore Data Views actions
Browse files Browse the repository at this point in the history
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`)
  • Loading branch information
mcsf committed May 6, 2024
1 parent 3d8b192 commit bb1b06e
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 15 deletions.
6 changes: 4 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 17 additions & 7 deletions packages/edit-site/src/components/page-pages/index.js
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import { camelCase } from 'change-case';

/**
* WordPress dependencies
*/
Expand All @@ -11,6 +16,7 @@ import { privateApis as routerPrivateApis } from '@wordpress/router';
import { useSelect, useDispatch } from '@wordpress/data';
import { DataViews } from '@wordpress/dataviews';
import { privateApis as editorPrivateApis } from '@wordpress/editor';
import { doAction, privateApis as hooksPrivateApis } from '@wordpress/hooks';

/**
* Internal dependencies
Expand Down Expand Up @@ -362,13 +368,17 @@ export default function PagePages() {
);
const onActionPerformed = useCallback(
( actionId, items ) => {
if ( actionId === 'edit-post' ) {
const post = items[ 0 ];
history.push( {
postId: post.id,
postType: post.type,
canvas: 'edit',
} );
// Dispatch a private action corresponding to `actionId` under the
// `pagePages` namespace, if such a private action is registered.
// For instance, `pagePages.editPost`.
//
// @see packages/hooks/src/private-hooks.js
const hookName = unlock( hooksPrivateApis ).privateHooksMap.get(
`pagePages.${ camelCase( actionId ) }`
);

if ( hookName ) {
doAction( hookName, items, history );
}
},
[ history ]
Expand Down
6 changes: 6 additions & 0 deletions packages/editor/src/components/post-actions/actions.js
Expand Up @@ -9,6 +9,7 @@ import { store as coreStore } from '@wordpress/core-data';
import { __, _n, sprintf, _x } from '@wordpress/i18n';
import { store as noticesStore } from '@wordpress/notices';
import { useMemo, useState } from '@wordpress/element';
import { doAction, privateApis } from '@wordpress/hooks';

import {
Button,
Expand Down Expand Up @@ -455,6 +456,11 @@ const renamePostAction = {
createSuccessNotice( __( 'Name updated' ), {
type: 'snackbar',
} );
doAction(
unlock( privateApis ).privateHooksMap.get(
'postActions.renamePost'
)
);
onActionPerformed?.( items );
} catch ( error ) {
const errorMessage =
Expand Down
3 changes: 2 additions & 1 deletion packages/hooks/package.json
Expand Up @@ -26,7 +26,8 @@
"react-native": "src/index",
"types": "build-types",
"dependencies": {
"@babel/runtime": "^7.16.0"
"@babel/runtime": "^7.16.0",
"@wordpress/private-apis": "^0.39.0"
},
"publishConfig": {
"access": "public"
Expand Down
2 changes: 1 addition & 1 deletion packages/hooks/src/createAddHook.js
Expand Up @@ -9,7 +9,7 @@ import validateHookName from './validateHookName.js';
*
* Adds the hook to the appropriate hooks container.
*
* @param {string} hookName Name of hook to add
* @param {string|symbol} hookName Name of hook to add
* @param {string} namespace The unique namespace identifying the callback in the form `vendor/plugin/function`.
* @param {import('.').Callback} callback Function to call when the hook is run
* @param {number} [priority=10] Priority of this hook
Expand Down
52 changes: 51 additions & 1 deletion packages/hooks/src/index.js
Expand Up @@ -25,7 +25,7 @@ import createHooks from './createHooks';
*/

/**
* @typedef {Record<string, Hook> & {__current: Current[]}} Store
* @typedef {Record<string|symbol, Hook> & {__current: Current[]}} Store
*/

/**
Expand Down Expand Up @@ -80,3 +80,53 @@ export {
actions,
filters,
};

import { privateApis as hooksPrivateApis } from './private-apis';
import { unlock } from './lock-unlock';

/**
* This will fire whenever a post is renamed, regardless of which UI we're
* accessing from.
*/
addAction(
unlock( hooksPrivateApis ).privateHooksMap.get( 'postActions.renamePost' ),
'my/handle-rename-post',
function () {
// eslint-disable-next-line no-console
console.log( 'Post renamed in postActions.renamePost' );
}
);

/**
* This will fire only when accessing from PagePages
* (/wp-admin/site-editor.php?path=%2Fpage&layout=table)
*/
addAction(
unlock( hooksPrivateApis ).privateHooksMap.get( 'pagePages.renamePost' ),
'my/handle-rename-post',
function () {
// eslint-disable-next-line no-console
console.log( 'Post renamed in pagePages.renamePost' );
}
);

/**
* That specificity helps to implement routing. Note that the contextual
* history object is passed via the hook.
*/
addAction(
unlock( hooksPrivateApis ).privateHooksMap.get( 'pagePages.editPost' ),
'my/handle-edit-post',
function ( items, history ) {
const post = items[ 0 ];
// eslint-disable-next-line no-console
console.log( `Editing post ${ post?.id }` );
history.push( {
postId: post.id,
postType: post.type,
canvas: 'edit',
} );
}
);

export * from './private-apis';
9 changes: 9 additions & 0 deletions packages/hooks/src/lock-unlock.js
@@ -0,0 +1,9 @@
/**
* WordPress dependencies
*/
import { __dangerousOptInToUnstableAPIsOnlyForCoreModules } from '@wordpress/private-apis';

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Playwright - 1

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Playwright - 6

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Playwright - 7

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Playwright - 8

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Playwright - 2

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Build Release Artifact

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Playwright - 3

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Playwright - 5

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Playwright - 4

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Build JavaScript assets for PHP unit tests

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Check

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.

Check failure on line 4 in packages/hooks/src/lock-unlock.js

View workflow job for this annotation

GitHub Actions / Run performance tests

Cannot find module '@wordpress/private-apis' or its corresponding type declarations.
export const { lock, unlock } =
__dangerousOptInToUnstableAPIsOnlyForCoreModules(
'I know using unstable features means my theme or plugin will inevitably break in the next version of WordPress.',
'@wordpress/hooks'
);
9 changes: 9 additions & 0 deletions packages/hooks/src/private-apis.js
@@ -0,0 +1,9 @@
/**
* Internal dependencies
*/
import { lock } from './lock-unlock';
import { privateHooksMap } from './private-hooks';

export const privateApis = {};

lock( privateApis, { privateHooksMap } );
27 changes: 27 additions & 0 deletions packages/hooks/src/private-hooks.js
@@ -0,0 +1,27 @@
// Define a list of "private hooks" that only core packages can use. This is
// implemented by producing Symbols from these hook names, the access to which
// will be mediated by the lock/unlock interface.
//
// Note that the standard Hooks API only accepts valid strings as hook names,
// but an exception will be made for Symbols on this list.
//
// @see validateHookName.
const privateHooks = [
'postActions.renamePost',
'pagePages.renamePost',
'pagePages.editPost',
];

// Used by consumers of the hooks API
//
// @example
// ```js
// const { privateHooksMap } = unlock( privateApis );
// const MY_HOOK = privateHooksMap.get( 'myHook' );
// doAction( MY_HOOK );
// ```
export const privateHooksMap = new Map(
privateHooks.map( ( label ) => [ label, Symbol( label ) ] )
);

export const privateHooksSet = new Set( privateHooksMap.values() );
17 changes: 14 additions & 3 deletions packages/hooks/src/validateHookName.js
@@ -1,13 +1,24 @@
/**
* Internal dependencies
*/

import { privateHooksSet } from './private-hooks';

/**
* Validate a hookName string.
*
* @param {string} hookName The hook name to validate. Should be a non empty string containing
* only numbers, letters, dashes, periods and underscores. Also,
* the hook name cannot begin with `__`.
* @param {string|symbol} hookName The hook name to validate. Should be a non
* empty string containing only numbers,
* letters, dashes, periods and underscores.
* Also, the hook name cannot begin with `__`.
*
* @return {boolean} Whether the hook name is valid.
*/
function validateHookName( hookName ) {
if ( 'symbol' === typeof hookName && privateHooksSet.has( hookName ) ) {
return true;
}

if ( 'string' !== typeof hookName || '' === hookName ) {
// eslint-disable-next-line no-console
console.error( 'The hook name must be a non-empty string.' );
Expand Down
1 change: 1 addition & 0 deletions packages/private-apis/src/implementation.js
Expand Up @@ -25,6 +25,7 @@ const CORE_MODULES_USING_PRIVATE_APIS = [
'@wordpress/edit-widgets',
'@wordpress/editor',
'@wordpress/format-library',
'@wordpress/hooks',
'@wordpress/interface',
'@wordpress/patterns',
'@wordpress/preferences',
Expand Down

0 comments on commit bb1b06e

Please sign in to comment.