Private experimental cross-module selectors and actions#44521
Merged
adamziel merged 13 commits intoexperiments/pivot-to-lock-unlockfrom Dec 22, 2022
Merged
Private experimental cross-module selectors and actions#44521adamziel merged 13 commits intoexperiments/pivot-to-lock-unlockfrom
adamziel merged 13 commits intoexperiments/pivot-to-lock-unlockfrom
Conversation
adamziel
requested review from
ajitbohra,
ellatrix and
nerrad
as code owners
adamziel
added
[Type] Enhancement
adamziel
force-pushed
the
experiments/private-selectors-and-actions
branch
from
27fbb8f to
a09263f
Compare
|
Size Change: +717 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
adamziel
changed the title
adamziel
changed the title
adamziel
commented
Sep 29, 2022
adamziel
commented
Sep 29, 2022
adamziel
commented
Sep 29, 2022
This was referenced Sep 29, 2022
jsnajdr
reviewed
Sep 29, 2022
packages/data/src/index.js
Outdated
|
|
||
| function __experimentalPrivateSelector( { name }, selector ) { | ||
| return ( ...args ) => | ||
| selector( defaultRegistry.stores[ name ].store.getState(), ...args ); |
Member
There was a problem hiding this comment.
It is a major shortcoming that the selector always selects from the default registry, and not the one that's in React context and is returned by useRegistry. I wonder if we can do anything about it?
Member
There was a problem hiding this comment.
// registration
registerPrivateSelectors( blockEditorStore, {
getContentLockingParent
} );
// plain usage
const { privateOf } = unlock( dataExperiments );
privateOf( registry.select( blockEditorStore ) ).getContentLockingParent();
// usage in React
useSelect( ( select ) => ( {
parent: privateOf( select( blockEditorStore ) ).getContentLockingParent();
} ) );I can register private selectors and actions on a store, and then the object returned by select has the good old public methods, but can also be "unlocked" with privateOf to access the private parts.
The same can be done with actions and dispatch.
Ideally, I would not need a custom privateOf function, but could use the general purpose unlock to unlock any object. Could be implemented by having a Symbol.for( 'accessKey' ) property on an object. Very similar to how, to a for of loop, you can pass either an object with a Symbol.iterator field or an actual iterator.
Contributor
Author
There was a problem hiding this comment.
@jsnajdr I really like this idea and I'm exploring an implementation. Would you mind taking a look? I think the new lockObj/unlockObj API could very well replace the old register/unlock API – I just need some better names.
Member
There was a problem hiding this comment.
I'm adding this to my todo-list after finishing the React 18 migration. Which currrently utilizes me 100% 🙂
adamziel
force-pushed
the
experiments/private-selectors-and-actions
branch
from
7550dd2 to
cf3ae62
Compare
adamziel
commented
Nov 27, 2022
adamziel
commented
Nov 27, 2022
adamziel
commented
Nov 27, 2022
adamziel
changed the title
adamziel
changed the base branch from
trunk
to
experiments/pivot-to-lock-unlock
adamziel
force-pushed
the
experiments/private-selectors-and-actions
branch
2 times, most recently
from
c3c874f to
3b3eaca
Compare
Contributor
Author
|
I just updated this PR as @jsnajdr suggested: // registration
registerPrivateSelectors( blockEditorStore, {
getContentLockingParent
} );
// plain usage
unlock( registry.select( blockEditorStore ) ).getContentLockingParent();
// usage in React
useSelect( ( select ) => ( {
parent: unlock( select( blockEditorStore ) ).getContentLockingParent();
} ) );You can register private selectors and actions on a store, and the object returned by This now works with all the registries, not just the default one. It's easy to migrate the current code to the new one – we just need to add |
adamziel
force-pushed
the
experiments/private-selectors-and-actions
branch
from
d418a57 to
172e63b
Compare
Contributor
Author
|
The JavaScript unit tests seems to fail due to |
…electors, registerPrivateActions, and privateOf methods
adamziel
force-pushed
the
experiments/private-selectors-and-actions
branch
from
172e63b to
e444d1d
Compare
Contributor
Author
|
I'm merging this PR into #46131 – the two are closely related, updating them separately takes effort, and keeping the code separate makes the dependency less clear. |
…ate-selectors-and-actions
adamziel
added a commit
that referenced
this pull request
Dec 22, 2022
Introduces a private selectors APIs in `@wordpress/data` via [the new `@wordpress/experimental`](#43386 (comment)) package: ```js // Package wordpress/block-data: import { unlock } from '../experiments'; import { experiments as dataExperiments } from '@wordpress/data'; const { registerPrivateActionsAndSelectors } = unlock( dataExperiments ); import { store as blockEditorStore } from './store'; import { __unstableSelectionHasUnmergeableBlock } from './store/selectors'; registerPrivateActionsAndSelectors( store, {}, { __experimentalHasContentRoleAttribute } ); // plain usage unlock( registry.select( blockEditorStore ) ).getContentLockingParent(); // usage in React useSelect( ( select ) => ( { parent: privateOf( select( blockEditorStore ) ).__unstableSelectionHasUnmergeableBlock(); } ) ); ```
mcsf
reviewed
Dec 22, 2022
Comment on lines
+525
to
+529
| import { __unstableSelectionHasUnmergeableBlock } from './store/selectors'; | ||
| registerPrivateSelectors( store, { | ||
| __experimentalHasContentRoleAttribute, | ||
| } ); | ||
| ``` |
Contributor
There was a problem hiding this comment.
These selectors don't match, which adds confusion.
Contributor
Author
There was a problem hiding this comment.
ah snap good spot, I'll fix this in #46131
adamziel
added a commit
that referenced
this pull request
Jan 17, 2023
## Description
This commit introduces a more convenient API for managing the private experiments as the former `register`-based API was quite cumbersome to use.
The idea is to "lock" private data inside public objects:
```js
const { lock, unlock } = __dangerousOptInToUnstableAPIsOnlyForCoreModules(
'<CONSENT STRING>',
'@wordpress/blocks'
);
export const publicObject = {};
lock( __experiments, "Shh, private data!" );
publicObject
// {}
unlock( publicObject )
// "Shh, private data!"
```
This new `lock()`/`unlock()` API enables private functions, classes, components, selectors, actions, arguments, and properties. Any package that opted-in to private APIs can call `unlock()` on publicly available artifacts to retrieve the related private API.
Kudos to @jsnajdr for [identifying an opportunity to simplify the API](#44521 (comment))!
## Examples
### Private selectors:
```js
// In wordpress/block-data:
import { store as blockEditorStore } from './store';
import { unlock } from '../experiments';
import { __unstableSelectionHasUnmergeableBlock } from './store/selectors';
unlock( store ).registerPrivateSelectors( {
__unstableSelectionHasUnmergeableBlock
} );
// In a React component:
function MyComponent() {
const hasRole = useSelect( ( select ) => (
unlock( select( blockEditorStore ) ).__unstableSelectionHasUnmergeableBlock()
) );
// ...
}
```
### Private functions, classes, and variables
```js
// In packages/package1/index.js:
import { lock } from './experiments';
export const experiments = {};
/* Attach private data to the exported object */
lock(experiments, {
__experimentalCallback: function() {},
__experimentalReactComponent: function ExperimentalComponent() { return <div/>; },
__experimentalClass: class Experiment{},
__experimentalVariable: 5,
});
// In packages/package2/index.js:
import { experiments } from '@wordpress/package1';
import { unlock } from './experiments';
const {
__experimentalCallback,
__experimentalReactComponent,
__experimentalClass,
__experimentalVariable
} = unlock( experiments );
```
### Private function arguments
To add an experimental argument to a stable function you'll need
to prepare a stable and an experimental version of that function.
Then, export the stable function and `lock()` the unstable function
inside it:
```js
// In @wordpress/package1/index.js:
import { lock } from './experiments';
// The experimental function contains all the logic
function __experimentalValidateBlocks(formula, __experimentalIsStrict) {
let isValid = false;
// ...complex logic we don't want to duplicate...
if ( __experimentalIsStrict ) {
// ...
}
// ...complex logic we don't want to duplicate...
return isValid;
}
// The stable public function is a thin wrapper that calls the
// experimental function with the experimental features disabled
export function validateBlocks(blocks) {
__experimentalValidateBlocks(blocks, false);
}
lock( validateBlocks, __experimentalValidateBlocks );
// In @wordpress/package2/index.js:
import { validateBlocks } from '@wordpress/package1';
import { unlock } from './experiments';
// The experimental function may be "unlocked" given the stable function:
const __experimentalValidateBlocks = unlock(validateBlocks);
__experimentalValidateBlocks(blocks, true);
```
### Private React Component properties
To add an experimental argument to a stable component you'll need
to prepare a stable and an experimental version of that component.
Then, export the stable function and `lock()` the unstable function
inside it:
```js
// In @wordpress/package1/index.js:
import { lock } from './experiments';
// The experimental component contains all the logic
const ExperimentalMyButton = ( { title, __experimentalShowIcon = true } ) => {
// ...complex logic we don't want to duplicate...
return (
<button>
{ __experimentalShowIcon && <Icon src={some icon} /> } { title }
</button>
);
}
// The stable public component is a thin wrapper that calls the
// experimental component with the experimental features disabled
export const MyButton = ( { title } ) =>
<ExperimentalMyExistingButton title={ title } __experimentalShowIcon={ false } />
lock(MyButton, ExperimentalMyButton);
// In @wordpress/package2/index.js:
import { MyButton } from '@wordpress/package1';
import { unlock } from './experiments';
// The experimental component may be "unlocked" given the stable component:
const ExperimentalMyButton = unlock(MyButton);
export function MyComponent() {
return (
<ExperimentalMyButton data={data} __experimentalShowIcon={ true } />
)
}
```
Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
Co-authored-by: Robert Anderson <robert@noisysocks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
This PR introduces a private selectors APIs in
@wordpress/datavia the new@wordpress/experimentalpackage:This PR demonstrates the usage by making the
__unstableSelectionHasUnmergeableBlockselector private. I intend to roll back this change before merging, and then follow-up with a migration PR to ensure new experimental selectors won't be added to the public API.Why?
Exporting
__unstableand__experimentalselectors makes them available to extenders and locks them in as a part of the public API. Let's not add new such exports if it can be avoided.Testing
Confirm the new API makes sense and that all the tests pass.
cc @jorgefilipecosta @ellatrix @talldan @draganescu @peterwilsoncc @jsnajdr