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

Remove extra locking from Block Editor store #47550

Merged
merged 1 commit into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 85 additions & 53 deletions docs/contributors/code/coding-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,13 @@ span multiple WordPress releases for others.
Make your experimental APIs private and don't expose them to WordPress extenders.

This way they'll remain internal implementation details that can be changed or removed
without a warning and without breaking WordPress plugins.
without a warning and without breaking WordPress plugins.

The tactical guidelines below will help you write code without introducing new experimental APIs.

#### General guidelines

Some `__experimental` functions are exported in *package A* and only used in a single *package B* and nowhere else. Consider removing such functions from *package A* and making them private and non-exported members of *package B*.
Some `__experimental` functions are exported in _package A_ and only used in a single _package B_ and nowhere else. Consider removing such functions from _package A_ and making them private and non-exported members of _package B_.

If your experimental API is only meant for the Gutenberg Plugin but not for the next WordPress major release, consider limiting the export to the plugin environment. For example, `@wordpress/components` could do that to receive early feedback about a new Component, but avoid bringing that component to WordPress core:

Expand All @@ -179,45 +179,49 @@ Sometimes a non-exported React hook suffices as a substitute for introducing a n

```js
// Instead of this:
// selectors.js:
export function __unstableHasActiveBlockOverlayActive( state, parent ) { /* ... */ }
export function __unstableIsWithinBlockOverlay( state, clientId ) {
let parent = state.blocks.parents[ clientId ];
while ( !! parent ) {
if ( __unstableHasActiveBlockOverlayActive( state, parent ) ) {
return true;
}
parent = state.blocks.parents[ parent ];
// selectors.js:
export function __unstableHasActiveBlockOverlayActive( state, parent ) {
/* ... */
}
export function __unstableIsWithinBlockOverlay( state, clientId ) {
let parent = state.blocks.parents[ clientId ];
while ( !! parent ) {
if ( __unstableHasActiveBlockOverlayActive( state, parent ) ) {
return true;
}
return false;
}
// MyComponent.js:
function MyComponent({ clientId }) {
const { __unstableIsWithinBlockOverlay } = useSelect( myStore );
const isWithinBlockOverlay = __unstableIsWithinBlockOverlay( clientId );
// ...
parent = state.blocks.parents[ parent ];
}
return false;
}
// MyComponent.js:
function MyComponent( { clientId } ) {
const { __unstableIsWithinBlockOverlay } = useSelect( myStore );
const isWithinBlockOverlay = __unstableIsWithinBlockOverlay( clientId );
// ...
}

// Consider this:
// MyComponent.js:
function hasActiveBlockOverlayActive ( selectors, parent ) { /* ... */ }
function useIsWithinBlockOverlay( clientId ) {
return useSelect( ( select ) => {
const selectors = select( blockEditorStore );
let parent = selectors.getBlockRootClientId( clientId );
while ( !!parent ) {
if ( hasActiveBlockOverlayActive( selectors, parent ) ) {
return true;
}
parent = selectors.getBlockRootClientId( parent );
// MyComponent.js:
function hasActiveBlockOverlayActive( selectors, parent ) {
/* ... */
}
function useIsWithinBlockOverlay( clientId ) {
return useSelect( ( select ) => {
const selectors = select( blockEditorStore );
let parent = selectors.getBlockRootClientId( clientId );
while ( !! parent ) {
if ( hasActiveBlockOverlayActive( selectors, parent ) ) {
return true;
}
return false;
});
}
function MyComponent({ clientId }) {
const isWithinBlockOverlay = useIsWithinBlockOverlay( clientId );
// ...
}
parent = selectors.getBlockRootClientId( parent );
}
return false;
} );
}
function MyComponent( { clientId } ) {
const isWithinBlockOverlay = useIsWithinBlockOverlay( clientId );
// ...
}
```

#### Dispatch experimental actions in thunks
Expand All @@ -227,10 +231,10 @@ enables dispatching private actions inline:

```js
export function toggleFeature( scope, featureName ) {
return function ( { dispatch } ) {
dispatch({ type: '__experimental_BEFORE_TOGGLE' })
return function ( { dispatch } ) {
dispatch( { type: '__experimental_BEFORE_TOGGLE' } );
// ...
};
};
}
```

Expand All @@ -246,7 +250,7 @@ export const { lock, unlock } =
__dangerousOptInToUnstableAPIsOnlyForCoreModules(
'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.',
'@wordpress/block-editor' // Name of the package calling __dangerousOptInToUnstableAPIsOnlyForCoreModules,
// (not the name of the package whose APIs you want to access)
// (not the name of the package whose APIs you want to access)
);
```

Expand Down Expand Up @@ -338,13 +342,14 @@ 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{},
lock( experiments, {
__experimentalCallback: function () {},
__experimentalReactComponent: function ExperimentalComponent() {
return <div />;
},
__experimentalClass: class Experiment {},
__experimentalVariable: 5,
});

} );

// In packages/package2/index.js:
import { experiments } from '@wordpress/package1';
Expand All @@ -354,10 +359,38 @@ const {
__experimentalCallback,
__experimentalReactComponent,
__experimentalClass,
__experimentalVariable
__experimentalVariable,
} = unlock( experiments );
```

Remember to always register the private actions and selectors on the **registered** store.

Sometimes that's easy:
```js
export const store = createReduxStore( STORE_NAME, storeConfig() );
// `register` uses the same `store` object created from `createReduxStore`.
register( store );
unlock( store ).registerPrivateActions( {
// ...
} );
```

However some package might call both `createReduxStore` **and** `registerStore`. In this case, always choose the store that gets registered:

```js
export const store = createReduxStore( STORE_NAME, {
...storeConfig,
persist: [ 'preferences' ],
} );
const registeredStore = registerStore( STORE_NAME, {
...storeConfig,
persist: [ 'preferences' ],
} );
unlock( registeredStore ).registerPrivateActions( {
// ...
} );
```

#### Experimental function arguments

To add an experimental argument to a stable function you'll need
Expand All @@ -370,7 +403,7 @@ inside it:
import { lock } from './experiments';

// The experimental function contains all the logic
function __experimentalValidateBlocks(formula, __experimentalIsStrict) {
function __experimentalValidateBlocks( formula, __experimentalIsStrict ) {
let isValid = false;
// ...complex logic we don't want to duplicate...
if ( __experimentalIsStrict ) {
Expand All @@ -383,19 +416,18 @@ function __experimentalValidateBlocks(formula, __experimentalIsStrict) {

// 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);
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);
const __experimentalValidateBlocks = unlock( validateBlocks );
__experimentalValidateBlocks( blocks, true );
```

#### Experimental React Component properties
Expand Down
2 changes: 0 additions & 2 deletions packages/block-editor/src/store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ export const store = createReduxStore( STORE_NAME, {
...storeConfig,
persist: [ 'preferences' ],
} );
unlock( store ).registerPrivateActions( privateActions );
unlock( store ).registerPrivateSelectors( privateSelectors );

// We will be able to use the `register` function once we switch
// the "preferences" persistence to use the new preferences package.
Expand Down