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.

Sign up for GitHub

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

Core Data: Remove dependencies on the block editor package #67687

Closed
wants to merge 1 commit into from
Closed

Core Data: Remove dependencies on the block editor package #67687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion package-lock.json
View file
Open in desktop

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

1 change: 1 addition & 0 deletions packages/core-data/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -988,6 +988,7 @@ _Parameters_
- _name_ `string`: The entity name.
- _options_ `Object`:
- _options.id_ `[string]`: An entity ID to use instead of the context-provided one.
- _options.onEditEntityRecord_ `[Function]`: A callback to run before the entity record gets edited.

_Returns_

Expand Down
1 change: 0 additions & 1 deletion packages/core-data/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
"dependencies": {
"@babel/runtime": "7.25.7",
"@wordpress/api-fetch": "*",
"@wordpress/block-editor": "*",
"@wordpress/blocks": "*",
"@wordpress/compose": "*",
"@wordpress/data": "*",
Expand Down
30 changes: 20 additions & 10 deletions packages/core-data/src/hooks/use-entity-block-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { parse, __unstableSerializeAndClean } from '@wordpress/blocks';
*/
import { STORE_NAME } from '../name';
import useEntityId from './use-entity-id';
import { updateFootnotesFromMeta } from '../footnotes';

const EMPTY_ARRAY = [];
const parsedBlocksCache = new WeakMap();
Expand All @@ -26,14 +25,22 @@ const parsedBlocksCache = new WeakMap();
* `BlockEditorProvider` and are intended to be used with it,
* or similar components or hooks.
*
* @param {string} kind The entity kind.
* @param {string} name The entity name.
* @param {Object} options
* @param {string} [options.id] An entity ID to use instead of the context-provided one.
* @param {string} kind The entity kind.
* @param {string} name The entity name.
* @param {Object} options
* @param {string} [options.id] An entity ID to use instead of the context-provided one.
* @param {Function} [options.onEditEntityRecord] A callback to run before the entity record gets edited.
*
* @return {[unknown[], Function, Function]} The block array and setters.
*/
export default function useEntityBlockEditor( kind, name, { id: _id } = {} ) {
export default function useEntityBlockEditor(
kind,
name,
{
id: _id,
onEditEntityRecord = ( blocks, meta ) => ( { blocks, meta } ),
} = {}
) {
const providerId = useEntityId( kind, name );
const id = _id ?? providerId;
const { getEntityRecord, getEntityRecordEdits } = useSelect( STORE_NAME );
Expand Down Expand Up @@ -106,7 +113,7 @@ export default function useEntityBlockEditor( kind, name, { id: _id } = {} ) {
selection,
content: ( { blocks: blocksForSerialization = [] } ) =>
__unstableSerializeAndClean( blocksForSerialization ),
...updateFootnotesFromMeta( newBlocks, meta ),
...onEditEntityRecord( newBlocks, meta ),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be implemented with a filter? useEntityBlockEditor could run all edits through a filter before passing them to editEntityRecord:

const edits = applyFilters( 'editor.editEntityRecord', { selection, content, blocks, meta } );

There is prior art with the editor.preSavePost filter in the editor package.

Then footnotes could act as a "feature plugin" that adds footnote functionality to various places with hooks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that works for me. I wanted to propose something with a real code to get the discussion going 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if using a filter just because of a dependency issue is the right solution. I think "core-data" knowing of "blocks" and "footnotes" is ok, it's actually the right thing to do (core-data knows about the WP entities and their content/structure). What we could get rid of is the dependency towards "block-editor" package which only exists because of the InnerBlocks.Content if I'm not wrong.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we could get rid of is the dependency towards "block-editor" package which only exists because of the InnerBlocks.Content if I'm not wrong.

There are other dependencies involved, which are probably less unwanted:

import {
	getSaveElement,
	__unstableGetBlockProps as getBlockProps,
} from '@wordpress/blocks';
import { RichTextData } from '@wordpress/rich-text';

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative is to reimplement InnerBlock.Content and the rest using some primitives mirroring:

gutenberg/packages/block-editor/src/components/inner-blocks/index.js

Lines 306 to 312 in c20ad9f

useInnerBlocksProps.save = getInnerBlocksProps;
// Expose default appender placeholders as components.
ForwardedInnerBlocks.DefaultBlockAppender = DefaultBlockAppender;
ForwardedInnerBlocks.ButtonBlockAppender = ButtonBlockAppender;
ForwardedInnerBlocks.Content = () => useInnerBlocksProps.save().children;

Copy link
Member

@jsnajdr jsnajdr Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we could get rid of is the dependency towards "block-editor" package which only exists because of the InnerBlocks.Content if I'm not wrong.

Yes, it's right that core-data knows about blocks, footnotes and rich text, the only problem is that it depends on block-editor to do that. It should depend only on blocks and rich-text packages.

And eliminating block-editor looks doable. The only function that core-data uses from block-editor is getRichTextValues. That function is implemented using:

  1. some helpers from blocks and rich-text
  2. the RichText.Content and InnerBlocks.Content components

In turn, RichText.Content and InnerBlocks.Content are very simple components implemented again using helpers from blocks and rich-text.

The code also relies on the identity of the RichText.Content and InnerBlocks.Content. These must be globally unique functions, because they are used like:

// create JSX element somewhere
const el = <InnerBlocks.Content />;
// inspect the element type somewhere else:
if ( el.type === InnerBlocks.Content ) { /* ... */ }

What we could do is:

  1. Move getRichTextValues from block-editor to blocks. It's easy because i) it's private API; ii) is used only at one place, in core-data; iii) depends only on code from element, blocks and rich-text, which are already dependencies of blocks.
  2. Move the RichText.Content and InnerBlocks.Content components also from block-editor to blocks. Also easy, they also depend only on blocks and rich-text code.
  3. The RichText and InnerBlocks components in block-editor will import and use the Content components like this: 
    import { RichTextContent, InnerBlocksContent } from '@wordpress/blocks';
RichText.Content = RichTextContent;
InnerBlocks.Content = InnerBlocksContent;

That's all. The block-editor dependency is eliminated and everything works as before. Also all the three functions/components naturally belong to blocks because they are part of the save pipeline. And parsing and serializing blocks is one of the main things the blocks package does.

The RichText and InnerBlocks components in block-editor have a lot of "edit" logic that belongs to block-editor and should stay there. But the .Content sub-components which contain the "save" logic can be imported. And anyone who checks the InnerBlocks.Content value (from block-editor) or the InnerBlocksContent value (from block) will see the same function. That's important for a lot of the serializing logic (renderToString etc.) which uses code like

switch ( el.type ) {
  case InnerBlocks.Content:
    /* ... */
    break;
  case RichText.Content:
    /* ... */
    break;
}

How does that sound? @ellatrix should be much more familiar with this code than me, are there any gotchas I'm missing?

};

editEntityRecord( kind, name, id, edits, {
Expand All @@ -122,21 +129,24 @@ export default function useEntityBlockEditor( kind, name, { id: _id } = {} ) {
meta,
__unstableCreateUndoLevel,
editEntityRecord,
onEditEntityRecord,
]
);

const onInput = useCallback(
( newBlocks, options ) => {
const { selection, ...rest } = options;
const footnotesChanges = updateFootnotesFromMeta( newBlocks, meta );
const edits = { selection, ...footnotesChanges };
const edits = {
selection,
...onEditEntityRecord( newBlocks, meta ),
};

editEntityRecord( kind, name, id, edits, {
isCached: true,
...rest,
} );
},
[ kind, name, id, meta, editEntityRecord ]
[ kind, name, id, meta, editEntityRecord, onEditEntityRecord ]
);

return [ blocks, onInput, onChange ];
Expand Down
0 ...data/src/footnotes/get-footnotes-order.js → ...provider/footnotes/get-footnotes-order.js
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion .../footnotes/get-rich-text-values-cached.js → .../footnotes/get-rich-text-values-cached.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor';
/**
* Internal dependencies
*/
import { unlock } from '../lock-unlock';
import { unlock } from '../../../lock-unlock';

// TODO: The following line should have been:
//
Expand Down
0 packages/core-data/src/footnotes/index.js → ...rc/components/provider/footnotes/index.js
View file
Open in desktop
File renamed without changes.
3 changes: 2 additions & 1 deletion packages/editor/src/components/provider/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import EditorKeyboardShortcuts from '../global-keyboard-shortcuts';
import PatternRenameModal from '../pattern-rename-modal';
import PatternDuplicateModal from '../pattern-duplicate-modal';
import TemplatePartMenuItems from '../template-part-menu-items';
import { updateFootnotesFromMeta } from './footnotes';

const { ExperimentalBlockEditorProvider } = unlock( blockEditorPrivateApis );
const { PatternsMenuItems } = unlock( editPatternsPrivateApis );
Expand Down Expand Up @@ -77,7 +78,7 @@ function useBlockEditorProps( post, template, mode ) {
const [ postBlocks, onInput, onChange ] = useEntityBlockEditor(
'postType',
post.type,
{ id: post.id }
{ id: post.id, onEditEntityRecord: updateFootnotesFromMeta }
);
const [ templateBlocks, onInputTemplate, onChangeTemplate ] =
useEntityBlockEditor( 'postType', template?.type, {
Expand Down
9 changes: 7 additions & 2 deletions ...ges/core-data/src/test/entity-provider.js → ...mponents/provider/test/entity-provider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,18 @@ import {
getBlockTypes,
} from '@wordpress/blocks';
import { RichText, useBlockProps } from '@wordpress/block-editor';
import {
store as coreDataStore,
useEntityBlockEditor,
} from '@wordpress/core-data';
import { createRegistry, RegistryProvider } from '@wordpress/data';
import { registerCoreBlocks } from '@wordpress/block-library';
import { unregisterFormatType } from '@wordpress/rich-text';

/**
* Internal dependencies
*/
import { store as coreDataStore } from '../index';
import useEntityBlockEditor from '../hooks/use-entity-block-editor';
import { updateFootnotesFromMeta } from '../footnotes';

const postTypeConfig = {
kind: 'postType',
Expand Down Expand Up @@ -154,6 +157,7 @@ describe( 'useEntityBlockEditor', () => {
const TestComponent = () => {
[ blocks, , onChange ] = useEntityBlockEditor( 'postType', 'post', {
id: 1,
onEditEntityRecord: updateFootnotesFromMeta,
} );

return <div />;
Expand Down Expand Up @@ -229,6 +233,7 @@ describe( 'useEntityBlockEditor', () => {
const TestComponent = () => {
[ blocks, , onChange ] = useEntityBlockEditor( 'postType', 'post', {
id: 1,
onEditEntityRecord: updateFootnotesFromMeta,
} );

return <div />;
Expand Down
Loading