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

Use defer loading strategy for frontend view scripts #52536

Merged
merged 28 commits into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7e469ec
Load interactivity API scripts with defer loading strategy
westonruter Jul 11, 2023
23b7aad
Load comment-reply script with defer loading strategy
westonruter Jul 11, 2023
aefd67a
Load search view script with defer loading strategy
westonruter Jul 11, 2023
f89e31a
Merge branch 'trunk' of https://github.com/WordPress/gutenberg into a…
westonruter Jul 12, 2023
3dd37c2
Defer the view scripts for the Navigation and File blocks
westonruter Jul 12, 2023
4f7301b
Use DCA event instead of window load event for Navigation viewScripts
westonruter Jul 13, 2023
3e7c73e
Merge branch 'trunk' of https://github.com/WordPress/gutenberg into a…
westonruter Jul 13, 2023
34faf8f
Optimize submenu-on-click view script
westonruter Jul 13, 2023
5329284
Optimize modal view script
westonruter Jul 13, 2023
2bafd37
Utilize asset file for wp-block--search-view
westonruter Jul 13, 2023
4b86ce4
Conditionally enqueue navigation view scripts only if needed
westonruter Jul 13, 2023
95beb24
Update Navigation block to remove/add view scripts in the same way th…
westonruter Jul 13, 2023
404128f
Update Search block to include view script in same way as File block …
westonruter Jul 13, 2023
4ccd709
Refactor Search block view script
westonruter Jul 14, 2023
b7863e9
Use more DOM properties instead of attributes
westonruter Jul 14, 2023
c41d35b
Use more precise reflected terminology
westonruter Jul 14, 2023
b50a2b8
Remove resolved TODO comment
westonruter Jul 14, 2023
a18d055
Use event delegation to open MicroModal
westonruter Jul 14, 2023
712c4cf
Remove excessive type check
westonruter Jul 14, 2023
b2db932
Fix closing submenus when there are multiple Navigation blocks on a page
westonruter Jul 14, 2023
4a7fc46
Clarify block.json comment
westonruter Jul 14, 2023
7fe5534
Merge branch 'trunk' into add/defer-script-loading-strategy
westonruter Jul 18, 2023
ffd9e5c
Add core merge note for comment-reply script strategy
westonruter Jul 18, 2023
26c06ab
Use defer strategy instead of async for Navigation and Search blocks
westonruter Jul 20, 2023
86b8e36
Defer all block view scripts
westonruter Jul 20, 2023
aab0b4b
Ensure interactivity scripts remain in footer in WP<6.3
westonruter Jul 20, 2023
73e3564
Remove defer from comment-reply for now
westonruter Jul 20, 2023
da2380a
Merge remote-tracking branch 'origin/trunk' into add/defer-script-loa…
westonruter Jul 20, 2023
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
34 changes: 34 additions & 0 deletions lib/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,40 @@ function gutenberg_reregister_core_block_types() {

add_action( 'init', 'gutenberg_reregister_core_block_types' );

/**
* Adds the defer loading strategy to all registered blocks.
*
* This function would not be part of core merge. Instead, the register_block_script_handle() function would be patched
* as follows.
*
* ```
* --- a/wp-includes/blocks.php
* +++ b/wp-includes/blocks.php
* @ @ -153,7 +153,8 @ @ function register_block_script_handle( $metadata, $field_name, $index = 0 ) {
* $script_handle,
* $script_uri,
* $script_dependencies,
* - isset( $script_asset['version'] ) ? $script_asset['version'] : false
* + isset( $script_asset['version'] ) ? $script_asset['version'] : false,
* + array( 'strategy' => 'defer' )
Copy link
Member

Choose a reason for hiding this comment

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

@westonruter, do we have a patch against WordPress core that sets the default strategy for viewScripts to defer?

Copy link
Member

@gziolo gziolo Aug 14, 2023

Choose a reason for hiding this comment

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

I'm catching up after an extended leave, so I might find the answer myself in the meantime as I'm reading the communication related to the defer strategy. I'm mostly willing to ensure we have better defaults in WP core, so people have performant configuration by design.

At the moment, I don't see any changes applied in core:

https://github.com/WordPress/wordpress-develop/blob/f107073f39827c2ab2b3b3198e194f844f357213/src/wp-includes/blocks.php#L169-L174

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 patch hasn't been backported to core yet. I wasn't sure when that should be done, if it should sit in Gutenberg a bit first to test and then propose for core, or to add it to core at the same time. I'm happy to open a backport ticket and open a PR to apply this patch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Committed to core: r56398

Copy link
Member

Choose a reason for hiding this comment

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

Let's enable it for all blocks in both the Gutenberg plugin, and in WordPress core to get as much testing as necessary with 3rd party blocks that use block.json. I see that you discussed it in the context of view scripts in #52536 (comment) and ruled out as a potential issue.

Committed to core: r56398

Noting that with this patch, defer is going to be used with editorScript, script, and viewScript. In the case of editorScript, the dependencies aren't using defer like wp-block or wp-block-editor so it will probably fallback to the legacy handling. I'll comment on Trac, too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's enable it for all blocks in both the Gutenberg plugin, and in WordPress core to get as much testing as necessary with 3rd party blocks that use block.json. I see that you discussed it in the context of view scripts in #52536 (comment) and ruled out as a potential issue.

Yes, it should be enabled for all blocks, whether core or 3rd party.

Noting that with this patch, defer is going to be used with editorScript, script, and viewScript. In the case of editorScript, the dependencies aren't using defer like wp-block or wp-block-editor so it will probably fallback to the legacy handling. I'll comment on Trac, too.

Oh, good catch!

* );
* if ( ! $result ) {
* return false;
* ```
*
* @see register_block_script_handle()
*/
function gutenberg_defer_block_view_scripts() {
$block_types = WP_Block_Type_Registry::get_instance()->get_all_registered();
foreach ( $block_types as $block_type ) {
foreach ( $block_type->view_script_handles as $view_script_handle ) {
wp_script_add_data( $view_script_handle, 'strategy', 'defer' );
fabiankaegy marked this conversation as resolved.
Show resolved Hide resolved
}
}
aristath marked this conversation as resolved.
Show resolved Hide resolved
}

add_action( 'init', 'gutenberg_defer_block_view_scripts', 100 );

/**
* Deregisters the existing core block type and its assets.
*
Expand Down
24 changes: 18 additions & 6 deletions lib/experimental/interactivity-api/scripts.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,32 @@
*/

/**
* Move interactive scripts to the footer. This is a temporary measure to make
* it work with `wp_store` and it should be replaced with deferred scripts or
* modules.
* Makes sure that interactivity scripts execute after all `wp_store` directives have been printed to the page.
*
* In WordPress 6.3+ this is achieved by printing in the head but marking the scripts with defer. This has the benefit
* of early discovery so the script is loaded by the browser, while at the same time not blocking rendering. In older
* versions of WordPress, this is achieved by loading the scripts in the footer.
*
* @link https://make.wordpress.org/core/2023/07/14/registering-scripts-with-async-and-defer-attributes-in-wordpress-6-3/
*/
function gutenberg_interactivity_move_interactive_scripts_to_the_footer() {
// Move the @wordpress/interactivity package to the footer.
wp_script_add_data( 'wp-interactivity', 'group', 1 );
$supports_defer = version_compare( strtok( get_bloginfo( 'version' ), '-' ), '6.3', '>=' );
if ( $supports_defer ) {
// Defer execution of @wordpress/interactivity package but continue loading in head.
wp_script_add_data( 'wp-interactivity', 'strategy', 'defer' );
wp_script_add_data( 'wp-interactivity', 'group', 0 );
} else {
// Move the @wordpress/interactivity package to the footer.
wp_script_add_data( 'wp-interactivity', 'group', 1 );
}

// Move all the view scripts of the interactive blocks to the footer.
$registered_blocks = \WP_Block_Type_Registry::get_instance()->get_all_registered();
foreach ( array_values( $registered_blocks ) as $block ) {
if ( isset( $block->supports['interactivity'] ) && $block->supports['interactivity'] ) {
foreach ( $block->view_script_handles as $handle ) {
wp_script_add_data( $handle, 'group', 1 );
// Note that all block view scripts are already made defer by default.
wp_script_add_data( $handle, 'group', $supports_defer ? 0 : 1 );
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/file/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function gutenberg_block_core_file_update_interactive_view_script( $metadata ) {
}

/**
* When the `core/file` block is rendering, check if we need to enqueue the `'wp-block-file-view` script.
* When the `core/file` block is rendering, check if we need to enqueue the `wp-block-file-view` script.
*
* @param array $attributes The block attributes.
* @param string $content The block content.
Expand Down
40 changes: 27 additions & 13 deletions packages/block-library/src/navigation/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -671,19 +671,33 @@ function render_block_core_navigation( $attributes, $content, $block ) {
$inner_blocks_html .= '</ul>';
}

// If the script already exists, there is no point in removing it from viewScript.
$should_load_view_script = ( $is_responsive_menu || ( $has_submenus && ( $attributes['openSubmenusOnClick'] || $attributes['showSubmenuIcon'] ) ) );
$view_js_file = 'wp-block-navigation-view';
if ( ! wp_script_is( $view_js_file ) ) {
$script_handles = $block->block_type->view_script_handles;

// If the script is not needed, and it is still in the `view_script_handles`, remove it.
if ( ! $should_load_view_script && in_array( $view_js_file, $script_handles, true ) ) {
$block->block_type->view_script_handles = array_diff( $script_handles, array( $view_js_file, 'wp-block-navigation-view-2' ) );
}
// If the script is needed, but it was previously removed, add it again.
if ( $should_load_view_script && ! in_array( $view_js_file, $script_handles, true ) ) {
$block->block_type->view_script_handles = array_merge( $script_handles, array( $view_js_file, 'wp-block-navigation-view-2' ) );
$needed_script_map = array(
'wp-block-navigation-view' => ( $has_submenus && ( $attributes['openSubmenusOnClick'] || $attributes['showSubmenuIcon'] ) ),
'wp-block-navigation-view-2' => $is_responsive_menu,
);

$should_load_view_script = false;
if ( gutenberg_should_block_use_interactivity_api( 'core/navigation' ) ) {
// TODO: The script is still loaded even when it isn't needed when the Interactivity API is used.
$should_load_view_script = count( array_filter( $needed_script_map ) ) > 0;
} else {
foreach ( $needed_script_map as $view_script_handle => $is_view_script_needed ) {

// If the script already exists, there is no point in removing it from viewScript.
if ( wp_script_is( $view_script_handle ) ) {
continue;
}

$script_handles = $block->block_type->view_script_handles;

// If the script is not needed, and it is still in the `view_script_handles`, remove it.
if ( ! $is_view_script_needed && in_array( $view_script_handle, $script_handles, true ) ) {
$block->block_type->view_script_handles = array_diff( $script_handles, array( $view_script_handle ) );
}
// If the script is needed, but it was previously removed, add it again.
if ( $is_view_script_needed && ! in_array( $view_script_handle, $script_handles, true ) ) {
$block->block_type->view_script_handles = array_merge( $script_handles, array( $view_script_handle ) );
}
}
}

Expand Down
127 changes: 88 additions & 39 deletions packages/block-library/src/navigation/view-modal.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
/*eslint-env browser*/
/**
* External dependencies
*/
import MicroModal from 'micromodal';

// Responsive navigation toggle.
function navigationToggleModal( modal ) {

/**
* Toggles responsive navigation.
*
* @param {HTMLDivElement} modal
* @param {boolean} isHidden
*/
function navigationToggleModal( modal, isHidden ) {
const dialogContainer = modal.querySelector(
`.wp-block-navigation__responsive-dialog`
);

const isHidden = 'true' === modal.getAttribute( 'aria-hidden' );

modal.classList.toggle( 'has-modal-open', ! isHidden );
dialogContainer.toggleAttribute( 'aria-modal', ! isHidden );

Expand All @@ -23,10 +29,15 @@ function navigationToggleModal( modal ) {
}

// Add a class to indicate the modal is open.
const htmlElement = document.documentElement;
htmlElement.classList.toggle( 'has-modal-open' );
document.documentElement.classList.toggle( 'has-modal-open' );
}

/**
* Checks whether the provided link is an anchor on the current page.
*
* @param {HTMLAnchorElement} node
* @return {boolean} Is anchor.
*/
function isLinkToAnchorOnCurrentPage( node ) {
return (
node.hash &&
Expand All @@ -37,42 +48,80 @@ function isLinkToAnchorOnCurrentPage( node ) {
);
}

window.addEventListener( 'load', () => {
MicroModal.init( {
onShow: navigationToggleModal,
onClose: navigationToggleModal,
openClass: 'is-menu-open',
/**
* Handles effects after opening the modal.
*
* @param {HTMLDivElement} modal
*/
function onShow( modal ) {
navigationToggleModal( modal, false );
modal.addEventListener( 'click', handleAnchorLinkClicksInsideModal, {
passive: true,
} );
}

// Close modal automatically on clicking anchor links inside modal.
const navigationLinks = document.querySelectorAll(
'.wp-block-navigation-item__content'
);
/**
* Handles effects after closing the modal.
*
* @param {HTMLDivElement} modal
*/
function onClose( modal ) {
navigationToggleModal( modal, true );
modal.removeEventListener( 'click', handleAnchorLinkClicksInsideModal, {
passive: true,
} );
}

navigationLinks.forEach( function ( link ) {
// Ignore non-anchor links and anchor links which open on a new tab.
if (
! isLinkToAnchorOnCurrentPage( link ) ||
link.attributes?.target === '_blank'
) {
return;
}
/**
* Handle clicks to anchor links in modal using event delegation by closing modal automatically
*
* @param {UIEvent} event
*/
function handleAnchorLinkClicksInsideModal( event ) {
const link = event.target.closest( '.wp-block-navigation-item__content' );
if ( ! ( link instanceof HTMLAnchorElement ) ) {
return;
}

// Find the specific parent modal for this link
// since .close() won't work without an ID if there are
// multiple navigation menus in a post/page.
const modal = link.closest(
'.wp-block-navigation__responsive-container'
);
const modalId = modal?.getAttribute( 'id' );
// Ignore non-anchor links and anchor links which open on a new tab.
if (
! isLinkToAnchorOnCurrentPage( link ) ||
link.attributes?.target === '_blank'
) {
return;
}

link.addEventListener( 'click', () => {
// check if modal exists and is open before trying to close it
// otherwise Micromodal will toggle the `has-modal-open` class
// on the html tag which prevents scrolling
if ( modalId && modal.classList.contains( 'has-modal-open' ) ) {
MicroModal.close( modalId );
}
} );
} );
} );
// Find the specific parent modal for this link
// since .close() won't work without an ID if there are
// multiple navigation menus in a post/page.
const modal = link.closest( '.wp-block-navigation__responsive-container' );
const modalId = modal?.getAttribute( 'id' );
if ( ! modalId ) {
return;
}

// check if modal exists and is open before trying to close it
// otherwise Micromodal will toggle the `has-modal-open` class
// on the html tag which prevents scrolling
if ( modalId && modal.classList.contains( 'has-modal-open' ) ) {
MicroModal.close( modalId );
}
}

// MicroModal.init() does not support event delegation for the open trigger, so here MicroModal.show() is called manually.
document.addEventListener(
'click',
( event ) => {
/** @type {HTMLElement} */
const target = event.target;

if ( target.dataset.micromodalTrigger ) {
MicroModal.show( target.dataset.micromodalTrigger, {
onShow,
onClose,
openClass: 'is-menu-open',
} );
}
},
{ passive: true }
);
Loading