-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Framework: Make sure block assets are always registered before wp-edit-post script #6448
Conversation
lib/client-assets.php
Outdated
* | ||
* @since 0.4.0 | ||
*/ | ||
do_action( 'enqueue_block_editor_assets' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if a plugin enqueue blocks but also register a plugin depending on wp-edit-post
module?
My proposal here is to add a JavaScript actiion registerBlocks
called on domReach
(the WP package). This way no matter the script loading order, the blocks will always be registered at the right moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. An alternative would be to put inline script which initializes the editor under their own name and make wp-edit-post its dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I had in mind is something like that:
wp.domReady( () => {
wp.hooks.doAction( 'registerBlocks' );
wp.editPost.initializeEditor();
} );
And ask plugin authors to move their registerBlockType
calls too the registerBlocks
action. I know it might be a bit late for that, so the current method should work as well, and we could warn if registerBlockType
is called before the hook.
471aedb - is not production ready :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
471aedb
to
0ca49c9
Compare
target | ||
); | ||
return new Promise( ( resolve ) => { | ||
domReady( () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be up to the editor to decide the specific time it should be initialized? Or should the prerequisite of the initialization be considered by that which is initializing it (thinking outside the specific "WP + Plugins" context).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. you mean moving this to client-assets
instead. Sounds like a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that you asked it this way, it sounds reasonable 😃
I fully focused on fixing the issue with blocks loaded too late and I didn’t think about it.
We will have to make dom-ready its own module but it would happen sooner or later anyway.
Description
Fixes #6443 and #5661.
Raised by @korobochkin:
Raised by @theleemon:
In my tests it turned out that all blocks related assets are loaded too late when their are forced to be included in the footer. It works perfectly fine when they are included in the header. This patch moves those assets to be include before
wp-edit-post
which moves them in the flow to be always included first regardless where there are located.How has this been tested?
Manually. I registered the plugin with block using the code snippet generated with WP-CLI as explained in https://wordpress.org/gutenberg/handbook/blocks/generate-blocks-with-wp-cli/. I also updated:
to ensure it works also when the script is loaded in footer.
I added the test block and made sure it still loads properly when I refreshed the page.
Types of changes
Bug fix (non-breaking change which fixes an issue).
Checklist: