-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Enqueue assets for rendered blocks only #22754
Conversation
Size Change: +6.71 kB (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
3d4b3df
to
c0babc4
Compare
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.
This is cool, thank you for working on it. I'm more than happy to approve it. There are two technical questions though. How should we proceed with integration with the WordPress core to make sure it lands in WordPress 5.5. Is there a Trac ticket that is going to add WP_Block
class there?
Should we add a deprecation hint in the plugin for the line that removes wp_enqueue_registered_block_scripts_and_styles
action?
Can this be seen as a breaking change? |
There is a ticket here, yes: https://core.trac.wordpress.org/ticket/49926 It may warrant a separate ticket which describes the overarching goal and changes necessary to achieve it. For example, the note in the original comment about avoiding consideration of the editor screen in
I'm not entirely sure what you mean by this.
I think it's something that, if implemented, should warrant some release devnotes explaining the change. The broad answer for me is that it does change some behaviors, but not anything that was ever particularly documented as expected, or practically speaking would be common to depend upon. Specifically, these changes could include:
It could also use some further testing regarding the comment at #21838 (comment) of more specialized use-cases, though the later comment #21838 (comment) seems to imply that the general approach is compatible. |
I regulary do this for archive pages, or providing a fallback for legacy content that hasnt been updated to blocks. For example:
Because I do things that aren't common I would always check the changelog before updating though. Overall this sounds like a good change 👍 |
* | ||
* @see WP_Block::render | ||
*/ | ||
remove_action( 'enqueue_block_assets', 'wp_enqueue_registered_block_scripts_and_styles' ); |
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 we add a deprecation hint in the plugin for the line that removes wp_enqueue_registered_block_scripts_and_styles action?
I'm not entirely sure what you mean by this.
Should we remove it after the logic is backported into WP Core? At the time of or after WP 5.5?
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.
Ah, yes. Most everything in this file should be associated with a ticket for removal (per initial file comment). Based on my previous comment, this is yet to be created, but I can put a "TBD" for now.
Hm, I'm having a bit of a hard time following this use-case. Would it be possible to share some code? If you have legacy content which is intended to be styled as blocks, are you applying styles to the base elements? And including that in the block stylesheet? To me, I'd see this as something where the common styles would be placed somewhere common, and the block stylesheet contains styles which exclusively apply to the block. |
Basically whenever I want to use a button in the interface (until FSE) I add it in a template and reuse the block markup but just write it manually so I don't have to style the same element twice. <a class="wp-block-button wp-block-button__link is-style-outline">Read more</a> If I have a post archive page which needs a banner i add this in the template before the loop: <div class="wp-block-cover" style="background-image: url(...);">
<div class="wp-block-cover__inner-container">
<h1><?php echo get_the_archive_title(); ?></h1>
</div>
</div>
<?php if (have_posts()): ?>
<?php while (have_posts()): the_post() ?>
<?php get_template_part( 'template-parts/content', get_post_type() ); ?>
<?php endwhile; ?>
<?php endif; ?> The use case for legacy articles would look something like this: <article <?php post_class(); ?> id="post-<?php the_ID(); ?>">
<div class="entry-content">
<?php if (strpos(get_the_content(), '</h1>') === false) : ?>
<div class="wp-block-cover" style="background-image: url('<?php echo get_the_post_thumbnail_url('full'); ?>');">
<div class="wp-block-cover__inner-container">
<h1><?php echo get_the_title(); ?></h1>
</div>
</div>
<?php endif; ?>
<?php the_content(); ?>
</div>
</article> Then I'd set the post type template of articles to have the same cover block + heading block as a default. Not sure if I'm alone in doing things this way but there could potentially be others who reuse the block markup outside the post content. |
@oxyc, I don’t think that Cover block uses and front-end specific JavaScript code so it will continue to work. Do I miss anything? If you replicate another blocks, you probably should manually enqueue CSS and JS to be on the safe side anyway. |
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.
Let’s give it a try in the Gutenberg plugin to see whether it doesn’t regress for existing websites. If we would want to replicate it in WordPress core we will need the dev note to make folks aware.
Right, the breaking change is that I'd need to enqueue the CSS manually (no, there's no js). Not a problem since I always read changelogs–I only mention it to provide a real world use case where this is a breaking change in response to "It is not clear there would be any reason someone would have done this". |
Core ticket: https://core.trac.wordpress.org/ticket/50328 |
This comment has been minimized.
This comment has been minimized.
Moved comment to issue: #21838 (comment). |
I load all my CSS conditionally. My front page CSS is less than 20kb. Block editor core blocks add 56kb to my CSS file! I hope this feature comes to core blocks sooner than later. |
This optimization doesn't seem to work with a certain sort of block. I believe the issue is with blocks that don't have any sort of server-side handling, however I haven't had good luck tracking down the problem farther than that it happens when You can verify with Organic Profile Block (also available on wporg) with Gutenberg 8.4.0. (cc @itsdavidmorgan):
|
It’s also reported in #23485. The best thing will be reverting this change for now. |
Thanks for following up! |
Just a followup for the comment above: That plugin registers the block using different names in PHP & JS which is what was causing the issue. See #25220 (comment) for more details, the PHP function should use the same block-name as the JS function (so |
I have tried the code above:
and the loaded message is showed on every page. What's wrong here? |
@Tropicalista This only seems to work with block based themes. I created a Is there any way that regular themes can also use this feature? |
Try this: add_filter( 'load_separate_block_styles', '__return_true' ); |
@aristath Thanks! That seems to work for inline scripts, but now I don't see any script/style files of core/custom blocks being loaded? I only see those which are enqueued using |
@aristath Adding
breake my blog if I activate gutenberg plugin. Wrks as expected without Gutenberg plugin activated. Probably this is due to my theme not supporting it? |
Most likely yes. There are some things that themes should do to ensure that their styles override the default styles when loaded... They will either have to make their CSS more specific, or load separate styles using a method similar to what is described in this comment: #25220 (comment) |
Closes #21838
Closes #5445
This pull request seeks to explore optimizing block asset enqueueing. Currently, all scripts and styles for all blocks will be rendered in every front-end page. This is hugely wasteful, since in reality only the scripts and styles associated with blocks relevant for that page should be enqueued.
The changes proposed here seek to achieve this behavior by bypassing the default script asset enqueueing associated with the
enqueue_block_assets
action, instead deferring enqueueing to occur at the time that a block is rendering.This leverages the
script
andstyle
properties of aregister_block_type
settings to determine the assets relevant for the block type. The optimization will not take place if a plugin usesenqueue_block_assets
, which will run for every page load.While the changes here are quite minimal, it may require more effort in (a) how this impacts existing core code and (b) how this can take effect for core blocks.
wp_enqueue_registered_block_scripts_and_styles
will now only be called by default in the core editor screen. Therefore, its purpose should be considered as changing to the more generic behavior of "enqueue all assets for all blocks", and existing logic for evaluating the current screen can be removed.script
asset is enqueued in the editor. It works this way today, though arguably it may be causing some conflict or confusion for a block's "front-end" JavaScript to load in the editor.Also note a few subtleties of the differences in behavior:
the_content
) than it would have previously (wp_enqueue_scripts
). While this should still work, it's quite likely that if an asset was registered as intending to enqueue during the page header, the actual enqueue could occur later.wp_enqueue_scripts
as it had been implemented previously.render_block
. Thus, it could potentially impact usage which is expecting pure behavior ofrender_block
to generate markup. It's not clear what these conflicting circumstances may be, or whether there's some better indication to know the rendering context of a block (i.e. preparing for front-end display).Testing Instructions:
As noted above, core blocks are currently not able to take advantage of this optimization due to the fact that their assets will always be enqueued via
wp_common_block_scripts_and_styles
.However, it's possible to verify the behavior by implementing a custom block.
Example code:
With the above, you should only see logging for "loaded" in the front-end if the block is included in the contents of the post being viewed. This is contrasted with
master
, where the message will be logged on every front-end page of the site.