Make WordPress Core

Opened 13 months ago

Closed 10 months ago

Last modified 10 months ago

#58634 closed enhancement (fixed)

Update uses of script registration functions to use new function signature

Reported by: joemcgill's profile joemcgill Owned by: flixos90's profile flixos90
Milestone: 6.4 Priority: normal
Severity: normal Version: 6.3
Component: General Keywords: good-first-bug has-patch
Focuses: javascript, performance Cc:

Description (last modified by joemcgill)

In [56033] the function signatures for wp_register_script and wp_enqueue_script are updated so that the sixth parameter, $in_footer, now accepts an array of args with the following shape:

 * @param array|bool       $args     {
 *      Optional. An array of additional script loading strategies. Default empty array.
 *      Otherwise, it may be a boolean in which case it determines whether the script is printed in the footer. Default false.
 *
 *      @type string    $strategy     Optional. If provided, may be either 'defer' or 'async'.
 *      @type bool      $in_footer    Optional. Whether to print the script in the footer. Default 'false'.
 *
}

There are several places in core (primarily default themes) where the previous boolean value of true is being used to indicate that those scripts should load in the footer (default is false). While this is still supported, we should update these occurrences to use the new preferred args shape as a good housekeeping task.


Here's a list of current places where these could be updated:

src/wp-content/plugins/akismet/class.akismet.php:
  1756  			) {
  1757: 			wp_register_script( 'akismet-frontend', plugin_dir_url( __FILE__ ) . '_inc/akismet-frontend.js', array(), filemtime( plugin_dir_path( __FILE__ ) . '_inc/akismet-frontend.js' ), true );
  1758  			wp_enqueue_script( 'akismet-frontend' );

src/wp-content/themes/twentyeleven/inc/theme-options.php:
  675  function twentyeleven_customize_preview_js() {
  676: 	wp_enqueue_script( 'twentyeleven-customizer', get_template_directory_uri() . '/inc/theme-customizer.js', array( 'customize-preview' ), '20150401', true );
  677  }

src/wp-content/themes/twentyfifteen/functions.php:
  446  	// Skip-link fix is no longer enqueued by default.
  447: 	wp_register_script( 'twentyfifteen-skip-link-focus-fix', get_template_directory_uri() . '/js/skip-link-focus-fix.js', array(), '20230526', true );
  448  

  456  
  457: 	wp_enqueue_script( 'twentyfifteen-script', get_template_directory_uri() . '/js/functions.js', array( 'jquery' ), '20221101', true );
  458  	wp_localize_script(

src/wp-content/themes/twentyfifteen/inc/customizer.php:
  361  function twentyfifteen_customize_control_js() {
  362: 	wp_enqueue_script( 'color-scheme-control', get_template_directory_uri() . '/js/color-scheme-control.js', array( 'customize-controls', 'iris', 'underscore', 'wp-util' ), '20141216', true );
  363  	wp_localize_script( 'color-scheme-control', 'colorScheme', twentyfifteen_get_color_schemes() );

  372  function twentyfifteen_customize_preview_js() {
  373: 	wp_enqueue_script( 'twentyfifteen-customize-preview', get_template_directory_uri() . '/js/customize-preview.js', array( 'customize-preview' ), '20141216', true );
  374  }

src/wp-content/themes/twentyfourteen/functions.php:
  369  	if ( is_front_page() && 'slider' === get_theme_mod( 'featured_content_layout' ) ) {
  370: 		wp_enqueue_script( 'twentyfourteen-slider', get_template_directory_uri() . '/js/slider.js', array( 'jquery' ), '20150120', true );
  371  		wp_localize_script(

  380  
  381: 	wp_enqueue_script( 'twentyfourteen-script', get_template_directory_uri() . '/js/functions.js', array( 'jquery' ), '20230526', true );
  382  }

src/wp-content/themes/twentyfourteen/inc/customizer.php:
  144  function twentyfourteen_customize_preview_js() {
  145: 	wp_enqueue_script( 'twentyfourteen_customizer', get_template_directory_uri() . '/js/customizer.js', array( 'customize-preview' ), '20141015', true );
  146  }

src/wp-content/themes/twentyfourteen/inc/featured-content.php:
  436  	public static function enqueue_scripts() {
  437: 		wp_enqueue_script( 'featured-content-suggest', get_template_directory_uri() . '/js/featured-content-admin.js', array( 'jquery', 'suggest' ), '20211130', true );
  438  	}

src/wp-content/themes/twentynineteen/functions.php:
  261  	if ( has_nav_menu( 'menu-1' ) ) {
  262: 		wp_enqueue_script( 'twentynineteen-priority-menu', get_theme_file_uri( '/js/priority-menu.js' ), array(), '20200129', true );
  263: 		wp_enqueue_script( 'twentynineteen-touch-navigation', get_theme_file_uri( '/js/touch-keyboard-navigation.js' ), array(), '20221101', true );
  264  	}

src/wp-content/themes/twentynineteen/inc/customizer.php:
  127  function twentynineteen_customize_preview_js() {
  128: 	wp_enqueue_script( 'twentynineteen-customize-preview', get_theme_file_uri( '/js/customize-preview.js' ), array( 'customize-preview' ), '20181214', true );
  129  }

  135  function twentynineteen_panels_js() {
  136: 	wp_enqueue_script( 'twentynineteen-customize-controls', get_theme_file_uri( '/js/customize-controls.js' ), array(), '20181214', true );
  137  }

src/wp-content/themes/twentyseventeen/functions.php:
  479  	// Skip-link fix is no longer enqueued by default.
  480: 	wp_register_script( 'twentyseventeen-skip-link-focus-fix', get_theme_file_uri( '/assets/js/skip-link-focus-fix.js' ), array(), '20161114', true );
  481  
  482: 	wp_enqueue_script( 'twentyseventeen-global', get_theme_file_uri( '/assets/js/global.js' ), array( 'jquery' ), '20211130', true );
  483  

  488  	if ( has_nav_menu( 'top' ) ) {
  489: 		wp_enqueue_script( 'twentyseventeen-navigation', get_theme_file_uri( '/assets/js/navigation.js' ), array( 'jquery' ), '20210122', true );
  490  		$twentyseventeen_l10n['expand']   = __( 'Expand child menu', 'twentyseventeen' );

  501  
  502: 	wp_enqueue_script( 'jquery-scrollto', get_theme_file_uri( '/assets/js/jquery.scrollTo.js' ), array( 'jquery' ), '2.1.3', true );
  503  

src/wp-content/themes/twentyseventeen/inc/customizer.php:
  247  function twentyseventeen_customize_preview_js() {
  248: 	wp_enqueue_script( 'twentyseventeen-customize-preview', get_theme_file_uri( '/assets/js/customize-preview.js' ), array( 'customize-preview' ), '20161002', true );
  249  }

  255  function twentyseventeen_panels_js() {
  256: 	wp_enqueue_script( 'twentyseventeen-customize-controls', get_theme_file_uri( '/assets/js/customize-controls.js' ), array(), '20161020', true );
  257  }

src/wp-content/themes/twentysixteen/functions.php:
  415  	// Skip-link fix is no longer enqueued by default.
  416: 	wp_register_script( 'twentysixteen-skip-link-focus-fix', get_template_directory_uri() . '/js/skip-link-focus-fix.js', array(), '20230526', true );
  417  

  425  
  426: 	wp_enqueue_script( 'twentysixteen-script', get_template_directory_uri() . '/js/functions.js', array( 'jquery' ), '20211130', true );
  427  

src/wp-content/themes/twentysixteen/inc/customizer.php:
  488  function twentysixteen_customize_control_js() {
  489: 	wp_enqueue_script( 'color-scheme-control', get_template_directory_uri() . '/js/color-scheme-control.js', array( 'customize-controls', 'iris', 'underscore', 'wp-util' ), '20170530', true );
  490  	wp_localize_script( 'color-scheme-control', 'colorScheme', twentysixteen_get_color_schemes() );

  499  function twentysixteen_customize_preview_js() {
  500: 	wp_enqueue_script( 'twentysixteen-customize-preview', get_template_directory_uri() . '/js/customize-preview.js', array( 'customize-preview' ), '20170530', true );
  501  }

src/wp-content/themes/twentythirteen/functions.php:
  323  	// Loads JavaScript file with functionality specific to Twenty Thirteen.
  324: 	wp_enqueue_script( 'twentythirteen-script', get_template_directory_uri() . '/js/functions.js', array( 'jquery' ), '20230526', true );
  325  

  845  function twentythirteen_customize_preview_js() {
  846: 	wp_enqueue_script( 'twentythirteen-customizer', get_template_directory_uri() . '/js/theme-customizer.js', array( 'customize-preview' ), '20200516', true );
  847  }

src/wp-content/themes/twentytwelve/functions.php:
  190  	// Adds JavaScript for handling the navigation menu hide-and-show behavior.
  191: 	wp_enqueue_script( 'twentytwelve-navigation', get_template_directory_uri() . '/js/navigation.js', array( 'jquery' ), '20141205', true );
  192  

  694  function twentytwelve_customize_preview_js() {
  695: 	wp_enqueue_script( 'twentytwelve-customizer', get_template_directory_uri() . '/js/theme-customizer.js', array( 'customize-preview' ), '20200516', true );
  696  }

src/wp-content/themes/twentytwenty/functions.php:
  433  	// Enqueue the editor script.
  434: 	wp_enqueue_script( 'twentytwenty-block-editor-script', get_theme_file_uri( '/assets/js/editor-script-block.js' ), array( 'wp-blocks', 'wp-dom' ), wp_get_theme()->get( 'Version' ), true );
  435  }

  649  
  650: 	wp_enqueue_script( 'twentytwenty-customize-preview', get_theme_file_uri( '/assets/js/customize-preview.js' ), array( 'customize-preview', 'customize-selective-refresh', 'jquery' ), $theme_version, true );
  651  	wp_localize_script( 'twentytwenty-customize-preview', 'twentyTwentyBgColors', twentytwenty_get_customizer_color_vars() );

src/wp-content/themes/twentytwentyone/functions.php:
  472  
  473: 	wp_enqueue_script( 'twentytwentyone-editor', get_theme_file_uri( '/assets/js/editor.js' ), array( 'wp-blocks', 'wp-dom' ), wp_get_theme()->get( 'Version' ), true );
  474  }

src/wp-includes/script-loader.php:
  2712  
  2713: 	wp_register_script( 'wp-block-styles', false, array( 'wp-blocks' ), true, true );
  2714  	wp_add_inline_script( 'wp-block-styles', $inline_script );

Attachments (1)

58634.diff (22.2 KB) - added by huzaifaalmesbah 11 months ago.
I added a new update patch.

Download all attachments as: .zip

Change History (27)

#1 @joemcgill
13 months ago

  • Description modified (diff)

This ticket was mentioned in PR #4720 on WordPress/wordpress-develop by nirav7707.


13 months ago
#2

  • Keywords has-patch added; needs-patch removed

## Description

  • The pull request (PR) updates the 6th parameter of the functions wp_register_script and wp_enqueue_script from a boolean value to an array, aligning it with the updated version of the function signature.

This ticket was mentioned in Slack in #core by chaion07. View the logs.


13 months ago

#4 @swissspidy
13 months ago

Given that the old way still works, this is really an enhancement, not a bug. We can just as well do this in 6.4

This ticket was mentioned in Slack in #core by oglekler. View the logs.


13 months ago

#6 @oglekler
13 months ago

  • Keywords needs-testing added
  • Milestone changed from 6.3 to 6.4
  • Type changed from defect (bug) to enhancement

Due to lack of testing and according to suggestion that this is more an enhancement than bug fix, I am moving this into 6.4 milestone and change type into enhancement.

This ticket was mentioned in Slack in #core by oglekler. View the logs.


11 months ago

#8 @oglekler
11 months ago

This ticket was discussed during a bug scrub.

It just needs to be tested,

Plus:

  • Check that wp_enqueue_script() needs what is changed and it is accurate
  • Check that all places covered

Add props: @mukesh27

@huzaifaalmesbah
11 months ago

I added a new update patch.

#9 @huzaifaalmesbah
11 months ago

Thanks for the report @joemcgill and also, thanks for adding the patch https://github.com/WordPress/wordpress-develop/pull/4720 by @niravsherasiya7707

Environment

  • WordPress: 6.4-alpha-56267-src
  • PHP: 7.4.33
  • Server: nginx/1.25.2
  • Database: MySQL Community Server (GPL) v5.7.43

Result

All is working fine, but I face two issues in 4720.diff on 6.4-alpha-56267-src and twentytwentyone v1.9. line does not match.

src/wp-content/themes/twentytwentyone/functions.php
@@ -470,7 +470,7 @@

src/wp-includes/script-loader.php
@@ -2698,7 +2698,7 @@

I solved this issue, and I added a new update patch

src/wp-content/themes/twentytwentyone/functions.php
@@ -473,7 +473,7 @@ add_action( 'wp_enqueue_scripts', 'twenty_twenty_one_scripts' );

src/wp-includes/script-loader.php
@@ -2692,7 +2692,7 @@ function enqueue_editor_block_styles_assets() {
Last edited 11 months ago by huzaifaalmesbah (previous) (diff)

This ticket was mentioned in PR #5073 on WordPress/wordpress-develop by @mrinal013.


11 months ago
#10

In the @nirav7707's PR, at twentytwentyone/functions.php line #424, 433 wp_register_script() missed array at 6th paramerter. Here, added this.
Ticket: https://core.trac.wordpress.org/ticket/58634

Trac ticket:

This ticket was mentioned in Slack in #core by mrinal. View the logs.


11 months ago

#12 @bosskhj
11 months ago

This issue should be resolved as soon as possible.

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


11 months ago

@flixos90 commented on PR #4720:


11 months ago
#14

Closing this in favor of the newer PR #5073 with a few more changes covered. Thank you @nirav7707, will make sure to give you props since you did a lot of the relevant work! 🙌

#15 @flixos90
11 months ago

@joemcgill Question since you originally opened the ticket: The PR https://github.com/WordPress/wordpress-develop/pull/5073 only covers relevant instances in the default themes. This makes sense to me to handle as its own thing, but I wonder whether your original intention for the ticket was to cover also core usage?

#16 @joemcgill
11 months ago

Thanks for asking @flixos90. I was indeed a bit sloppy with my search of instances in the original description, because I was really intending for this to apply primarily to instances in core. I'm not opposed to also trying to do this in default themes, but if those themes are running in older versions of WP, then a more refined approach may be needed. I'd be happy for those to be addressed as a separate ticket.

#17 follow-up: @flixos90
11 months ago

@joemcgill

if those themes are running in older versions of WP, then a more refined approach may be needed. I'd be happy for those to be addressed as a separate ticket.

I think we can make the current PR work in a way that's backward compatible. As outlined in the script loading strategy dev note, as long as we don't use array( 'in_footer' => false ) anywhere, it will be backward compatible.

Given where the current PR is at, maybe we rename this ticket to focus on the bundled themes, and open a new one for updating the core usage?

#18 in reply to: ↑ 17 @joemcgill
11 months ago

Replying to flixos90:

I think we can make the current PR work in a way that's backward compatible. As outlined in the script loading strategy dev note, as long as we don't use array( 'in_footer' => false ) anywhere, it will be backward compatible.

That makes sense to me and would be a good example of how this could be implemented in a backwards compatible way.

Given where the current PR is at, maybe we rename this ticket to focus on the bundled themes, and open a new one for updating the core usage?

I'd suggest leaving this ticket open as a tracking ticket and just making separate commits for the themes and the core instances. Once all issues that we want to address have been updated, we can close this one.

#19 @flixos90
11 months ago

In 56526:

Bundled Theme: Update default themes to use new script function signature.

In WordPress 6.3, the last parameter of wp_register_script() and wp_enqueue_script() was changed to an array rather than a boolean. While a boolean is still supported for backward compatibility, it makes sense to update the codebase to use the new signature.

The updates are fully backward compatible:

  • In places where true was provided, array( 'in_footer' => true ) will still be interpreted as a boolean true in WordPress versions prior to 6.3.
  • In places where false was provided, the parameter is omitted which will work correctly throughout all WordPress versions given that is and has been the default value anyway.

Props mrinal013, huzaifaalmesbah, niravsherasiya7707, joemcgill.
Fixes #59302.
See #58634.

#20 @flixos90
11 months ago

  • Focuses performance added
  • Keywords needs-patch added; has-patch needs-testing removed

See above, the theme specific updates were committed in [56526]. As such, I'm "resetting" this ticket to now be focused on the core specific updates to use the new wp_register_script() / wp_enqueue_script() signatures.

As such, this ticket needs a new patch / PR :)

I'm adding the performance keyword, since, while this ticket itself is not going to change performance, it is a direct outcome of the performance related script loading strategy effort that launched in WordPress 6.3.

This ticket was mentioned in PR #5180 on WordPress/wordpress-develop by @incursadesigns.


11 months ago
#22

  • Keywords has-patch added; needs-patch removed

Updated wp_register_script in core to use new function signature.

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


10 months ago

#24 @flixos90
10 months ago

  • Owner set to flixos90
  • Status changed from new to reviewing

#25 @flixos90
10 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 56568:

General: Update wp_register_script() calls in core to use new function signature.

Since most core scripts are registered using WP_Scripts::add(), which is not affected by the signature change, only calls to wp_register_script() using the old signature need to be updated. There is only a single instance left, which is updated in this changeset.

Props incursadesigns.
Fixes #58634.

Note: See TracTickets for help on using tickets.