Core Data: Remove dependencies on the block editor package #67687
Conversation
This file contains 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
2 tasks
gziolo
added
[Package] Core data
|
Size Change: +120 B (+0.01%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
|
CI is fine with the refactoring. I would appreciate feedback if that direction is promising enough to take it to the finish line. |
jsnajdr
reviewed
Dec 9, 2024
| @@ -106,7 +113,7 @@ export default function useEntityBlockEditor( kind, name, { id: _id } = {} ) { | |||
| selection, | |||
| content: ( { blocks: blocksForSerialization = [] } ) => | |||
| __unstableSerializeAndClean( blocksForSerialization ), | |||
| ...updateFootnotesFromMeta( newBlocks, meta ), | |||
| ...onEditEntityRecord( newBlocks, meta ), | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Sure, that works for me. I wanted to propose something with a real code to get the discussion going 😄
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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';There was a problem hiding this comment.
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
There was a problem hiding this comment.
What we could get rid of is the dependency towards "block-editor" package which only exists because of the
InnerBlocks.Contentif 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:
- some helpers from
blocksandrich-text - the
RichText.ContentandInnerBlocks.Contentcomponents
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:
- Move
getRichTextValuesfromblock-editortoblocks. It's easy because i) it's private API; ii) is used only at one place, incore-data; iii) depends only on code fromelement,blocksandrich-text, which are already dependencies ofblocks. - Move the
RichText.ContentandInnerBlocks.Contentcomponents also fromblock-editortoblocks. Also easy, they also depend only onblocksandrich-textcode. - The
RichTextandInnerBlockscomponents inblock-editorwill import and use theContentcomponents 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?
|
@jsnajdr outlined the path forward in #67687 (comment) that seems the most reasonable taking into account that there is a shared agreement that |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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?
Fixes #64604.
The same concern has been raised in the original PR
@wordpress/core-datashould not depend on@wordpress/block-editoras they are independent packages that should be combined together to build any editor.Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast