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

Enqueue assets for rendered blocks only #22754

Merged
merged 2 commits into from
Jun 13, 2020
Merged

Conversation

aduth
Copy link
Member

@aduth aduth commented May 29, 2020

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 and style properties of a register_block_type settings to determine the assets relevant for the block type. The optimization will not take place if a plugin uses enqueue_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.

  1. With these changes, 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.
    • See example patch
    • Related: It may also be worth reevaluating whether it makes sense that a block's 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.
  2. The assets associated with core block's are always enqueued (source). Thus, they won't be able to take advantage of the optimizations here. At a minimum, the core blocks should be updated to reference the script and style asset handles themselves, rather than creating an ad hoc enqueuing implementation. This should serve as a simplifying refactoring, and ensure consistent behavior of assets enqueuing for all blocks.
    • A separate effort should seek to try to split the core blocks' script and style assets to per-block assets. In the meantime, there's not much benefit to these optimizations for core blocks.

Also note a few subtleties of the differences in behavior:

  • Enqueuing during the rendering of a block is likely to occur later in the page lifecycle (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.
    • Technically this could be remedied, if the parsing of blocks from content of the current post occurs earlier in the page lifecycle, closer to the wp_enqueue_scripts as it had been implemented previously.
  • The enqueueing will occur as a side effect of render_block. Thus, it could potentially impact usage which is expecting pure behavior of render_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:

<?php

/**
 * Plugin Name: My Block Plugin
 */

function my_block_plugin_register_block() {
	wp_register_script( 'my-block-plugin-block-js', null );
	wp_add_inline_script( 'my-block-plugin-block-js', 'console.log( "loaded" );' );
	wp_register_script( 'my-block-plugin-block-editor-js', null, array( 'wp-blocks' ) );
	wp_add_inline_script(
		'my-block-plugin-block-editor-js',
		'wp.blocks.registerBlockType( "my-block-plugin/block", { edit() { return "edit my-block-plugin/block"; } } );'
	);
	register_block_type( 'my-block-plugin/block', [
		'title'           => 'My Block',
		'script'          => 'my-block-plugin-block-js',
		'editor_script'   => 'my-block-plugin-block-editor-js',
		'render_callback' => function() {
			return 'render my-block-plugin/block';
		},
	] );
}
add_action( 'init', 'my_block_plugin_register_block' );

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.

@aduth aduth added [Feature] Block API API that allows to express the block paradigm. Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Performance Related to performance efforts labels May 29, 2020
@github-actions
Copy link

github-actions bot commented May 29, 2020

Size Change: +6.71 kB (0%)

Total Size: 1.12 MB

Filename Size Change
build/a11y/index.js 1.14 kB +1 B
build/annotations/index.js 3.62 kB -1 B
build/api-fetch/index.js 3.4 kB +2 B (0%)
build/autop/index.js 2.83 kB -1 B
build/block-directory/index.js 6.75 kB +270 B (4%)
build/block-directory/style-rtl.css 892 B +105 B (11%) ⚠️
build/block-directory/style.css 892 B +105 B (11%) ⚠️
build/block-editor/index.js 106 kB +624 B (0%)
build/block-editor/style-rtl.css 11.4 kB +51 B (0%)
build/block-editor/style.css 11.4 kB +50 B (0%)
build/block-library/editor-rtl.css 7.87 kB -4 B (0%)
build/block-library/editor.css 7.87 kB -3 B (0%)
build/block-library/index.js 127 kB +1.01 kB (0%)
build/block-library/style-rtl.css 7.72 kB +31 B (0%)
build/block-library/style.css 7.72 kB +31 B (0%)
build/block-serialization-default-parser/index.js 1.88 kB +1 B
build/block-serialization-spec-parser/index.js 3.1 kB -1 B
build/blocks/index.js 48.1 kB -28 B (0%)
build/components/index.js 193 kB +3.22 kB (1%)
build/components/style-rtl.css 19.5 kB +9 B (0%)
build/components/style.css 19.5 kB +9 B (0%)
build/compose/index.js 9.31 kB -4 B (0%)
build/core-data/index.js 11.4 kB +11 B (0%)
build/data/index.js 8.46 kB +30 B (0%)
build/date/index.js 5.47 kB +4 B (0%)
build/deprecated/index.js 771 B -1 B
build/dom/index.js 3.17 kB +55 B (1%)
build/edit-navigation/index.js 8.25 kB +61 B (0%)
build/edit-navigation/style-rtl.css 918 B +61 B (6%) 🔍
build/edit-navigation/style.css 919 B +63 B (6%) 🔍
build/edit-post/index.js 302 kB -41 B (0%)
build/edit-site/index.js 15 kB +871 B (5%) 🔍
build/edit-widgets/index.js 8.83 kB -48 B (0%)
build/editor/index.js 44.7 kB +74 B (0%)
build/element/index.js 4.65 kB +1 B
build/format-library/index.js 7.72 kB +8 B (0%)
build/html-entities/index.js 621 B -1 B
build/keyboard-shortcuts/index.js 2.52 kB +4 B (0%)
build/keycodes/index.js 1.94 kB +1 B
build/list-reusable-blocks/index.js 3.12 kB -2 B (0%)
build/media-utils/index.js 5.3 kB +7 B (0%)
build/notices/index.js 1.79 kB +1 B
build/nux/index.js 3.4 kB +1 B
build/plugins/index.js 2.56 kB -1 B
build/primitives/index.js 1.5 kB +1 B
build/redux-routine/index.js 2.85 kB -2 B (0%)
build/rich-text/index.js 14.8 kB +13 B (0%)
build/server-side-render/index.js 2.68 kB +5 B (0%)
build/url/index.js 4.06 kB +39 B (0%)
build/viewport/index.js 1.85 kB +10 B (0%)
build/wordcount/index.js 1.17 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/blob/index.js 620 B 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 B 0 B
build/data-controls/index.js 1.29 kB 0 B
build/dom-ready/index.js 568 B 0 B
build/edit-post/style-rtl.css 5.43 kB 0 B
build/edit-post/style.css 5.43 kB 0 B
build/edit-site/style-rtl.css 2.96 kB 0 B
build/edit-site/style.css 2.96 kB 0 B
build/edit-widgets/style-rtl.css 2.4 kB 0 B
build/edit-widgets/style.css 2.4 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/style-rtl.css 4.26 kB 0 B
build/editor/style.css 4.27 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/priority-queue/index.js 789 B 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/warning/index.js 1.14 kB 0 B

compressed-size-action

@aduth aduth force-pushed the try/dynamic-enqueue-assets branch from 3d4b3df to c0babc4 Compare June 2, 2020 20:33
Copy link
Member

@gziolo gziolo left a 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?

@spacedmonkey
Copy link
Member

Can this be seen as a breaking change?

@aduth
Copy link
Member Author

aduth commented Jun 4, 2020

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?

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 wp_enqueue_registered_block_scripts_and_styles.

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.

Can this be seen as a breaking change?

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:

  • If one was to rely on some block script to define some functionality leveraged outside the script and on pages where the block isn't present, those scripts would now only be enqueued on pages where the block is present. This is very similar case to "incidental availability" of script functionality from dependencies of other scripts, and is more an error on the part of the developer where any such script should be defined as an explicit dependency of the script.
  • If one was to rely on some block stylesheet to style pages where that block is not present, it would now only be enqueued on pages where the block is present. It is not clear there would be any reason someone would have done this, since the purpose of a block stylesheet should be to style the block (or even if it was styling the page, at least only the page where the block is present).
  • If one was to rely on the particular page enqueue lifecycle (header, footer placement), those expectations could change. This is documented in more detail in the original comment, and is potentially not an unsolvable challenge to preserve the current behavior if problematic.

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.

@oxyc
Copy link
Member

oxyc commented Jun 4, 2020

If one was to rely on some block stylesheet to style pages where that block is not present, it would now only be enqueued on pages where the block is present. It is not clear there would be any reason someone would have done this, since the purpose of a block stylesheet should be to style the block (or even if it was styling the page, at least only the page where the block is present).

I regulary do this for archive pages, or providing a fallback for legacy content that hasnt been updated to blocks. For example:

  • show old articles that's not using blocks as if they had a Cover block using get_the_post_thumbnail() and get_the_title() (oh yeah i dont print the page title when there's a <h1> in post_content. that's just how i build my themes).
  • load more buttons on archive pages use the button markup. I actually sprinkle the button markup a bit everywhere. ACF blocks, footers, headers wherever

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' );
Copy link
Member

@gziolo gziolo Jun 4, 2020

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?

Copy link
Member Author

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.

@aduth
Copy link
Member Author

aduth commented Jun 4, 2020

I regulary do this for archive pages, or providing a fallback for legacy content that hasnt been updated to blocks. For example:

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.

@oxyc
Copy link
Member

oxyc commented Jun 4, 2020

I regulary do this for archive pages, or providing a fallback for legacy content that hasnt been updated to blocks. For example:

Hm, I'm having a bit of a hard time following this use-case. Would it be possible to share some code?

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.

@gziolo
Copy link
Member

gziolo commented Jun 5, 2020

@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.

Copy link
Member

@gziolo gziolo left a 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.

@oxyc
Copy link
Member

oxyc commented Jun 5, 2020

@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.

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".

@aduth
Copy link
Member Author

aduth commented Jun 5, 2020

Core ticket: https://core.trac.wordpress.org/ticket/50328

@gziolo gziolo merged commit 5b38d71 into master Jun 13, 2020
@gziolo gziolo deleted the try/dynamic-enqueue-assets branch June 13, 2020 10:14
@github-actions github-actions bot added this to the Gutenberg 8.4 milestone Jun 13, 2020
@westonruter

This comment has been minimized.

@westonruter
Copy link
Member

Moved comment to issue: #21838 (comment).

@kamauwanjoroge
Copy link

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.

@youknowriad youknowriad added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Jun 29, 2020
@ellatrix ellatrix mentioned this pull request Jul 3, 2020
12 tasks
@claudiulodro
Copy link
Contributor

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 $this->block_type is null in WP_Block::render. It's very easy to reproduce the problem using a simple block, though.

You can verify with Organic Profile Block (also available on wporg) with Gutenberg 8.4.0. (cc @itsdavidmorgan):

  1. Create a profile block. View on frontend. In the page source observe organic-profile-block-frontend-style-css is not enqueued. Block should look large and (mostly) unstyled:

Screen Shot 2020-07-03 at 9 34 04 AM

  1. Add the following somewhere: add_action( 'enqueue_block_assets', 'wp_enqueue_registered_block_scripts_and_styles' );

  2. View block on frontend. In the page source observe organic-profile-block-frontend-style-css is enqueued. Block should look nicely styled:

Screen Shot 2020-07-03 at 9 37 29 AM

@gziolo
Copy link
Member

gziolo commented Jul 4, 2020

It’s also reported in #23485. The best thing will be reverting this change for now.

@gziolo
Copy link
Member

gziolo commented Jul 6, 2020

Reverting will happen in #23708. I reopened #5445.

@claudiulodro
Copy link
Contributor

Thanks for following up!

@mtias mtias mentioned this pull request Jul 13, 2020
@youknowriad youknowriad removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Jul 29, 2020
@aristath
Copy link
Member

aristath commented Dec 9, 2020

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 organic/profile-block and not profile/block).

@Tropicalista
Copy link

I have tried the code above:

<?php

/**
 * Plugin Name: My Block Plugin
 */

function my_block_plugin_register_block() {
	wp_register_script( 'my-block-plugin-block-js', null );
	wp_add_inline_script( 'my-block-plugin-block-js', 'console.log( "loaded" );' );
	wp_register_script( 'my-block-plugin-block-editor-js', null, array( 'wp-blocks' ) );
	wp_add_inline_script(
		'my-block-plugin-block-editor-js',
		'wp.blocks.registerBlockType( "my-block-plugin/block", { edit() { return "edit my-block-plugin/block"; } } );'
	);
	register_block_type( 'my-block-plugin/block', [
		'title'           => 'My Block',
		'script'          => 'my-block-plugin-block-js',
		'editor_script'   => 'my-block-plugin-block-editor-js',
		'render_callback' => function() {
			return 'render my-block-plugin/block';
		},
	] );
}
add_action( 'init', 'my_block_plugin_register_block' );

and the loaded message is showed on every page. What's wrong here?

@stevygee
Copy link

stevygee commented Mar 3, 2021

@Tropicalista This only seems to work with block based themes. I created a block-templates/index.html with a post content block inside, tested using Gutenberg 10.0.2.

Is there any way that regular themes can also use this feature?

@aristath
Copy link
Member

aristath commented Mar 3, 2021

Is there any way that regular themes can also use this feature?

Try this:

add_filter( 'load_separate_block_styles', '__return_true' );

@stevygee
Copy link

stevygee commented Mar 3, 2021

@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 enqueue_block_assets.
For my blocks, I use register_block_type's script and style arguments.

@Tropicalista
Copy link

@aristath Adding

add_filter( 'load_separate_block_styles', '__return_true' );

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?

@aristath
Copy link
Member

aristath commented Mar 5, 2021

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)
FSE themes will work out of the box, but for "classic" themes the theme-author should opt-in 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update docs for conditional loading assets using render_callback Bundling Front-end Assets