Make WordPress Core

Opened 5 years ago

Last modified 5 years ago

#45130 new defect (bug)

Defer jQuery WordPress Admin & Customizer doesn't work properly

Reported by: remzicavdar's profile remzicavdar Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Administration Keywords: needs-patch
Focuses: javascript Cc:

Description

If I defer jQuery or put jQuery in the footer. I get Uncaught ReferenceError: jQuery is not defined errors in the WordPress Customizer and the Customizer doesn't work, like for example changing theme logo, adding widgets or other things directly possible in the page.

When I remove defer, everything works as expected.

Maybe this is because document ready is not used? I have been checking this issue myself.
The following files are effected:

  • wp-includes/js/mediaelement/mediaelement-migrate.min.js
  • wp-includes/js/wp-a11y.min.js
  • wp-includes/js/customize-base.min.js
  • wp-includes/js/customize-preview.min.js
  • wp-includes/js/mediaelement/wp-mediaelement.min.js
  • wp-includes/js/wp-util.min.js
  • wp-includes/js/mediaelement/wp-playlist.min.js
  • wp-includes/js/customize-selective-refresh.min.js
  • wp-includes/js/customize-preview-widgets.min.js
  • wp-includes/js/customize-preview-nav-menus.min.js

Change History (12)

#1 @dlh
5 years ago

  • Keywords reporter-feedback added
  • Version 4.9.8 deleted

Hi @remzicavdar, and welcome to WordPress Trac!

Could you please share more details about the technique you're using to defer jQuery?

Regarding moving jQuery to the footer, it looks to me like that has been previously discussed in #22244.

#2 @remzicavdar
5 years ago

Hi @dlh
I am sorry to have kept you waiting for so long. I live in a different time zone.
I use the following code:

<?php
function add_defer_attribute($tag, $handle) {
   // add script handles to the array below
   $scripts_to_defer = array('jquery', 'jquery-migrate', 'bootstrap-bundle-js');

   foreach($scripts_to_defer as $defer_script) {
      if ($defer_script === $handle) {
         return str_replace(' src', ' defer="defer" src', $tag);
      }
   }
   return $tag;
}
add_filter('script_loader_tag', 'add_defer_attribute', 10, 2);

It is also good practice to put all the scripts into the footer, I understand that this has been discussed before. Even when the team decides not to, it's best to use document ready, otherwise everything breaks if jQuery has been placed in the footer. If a theme developer decides to put jQuery in the footer in his or her theme. In this case, we would preface the function with a document ready function as seen below:

jQuery(document).ready(function($) {
   // $ Works! You can test it with next line if you like
   // console.log($);
});

I think using document ready is always a good thing to do, even if you put your code in the header.

This is also possible, but it will defeat the purpose of using jQuery noConflict mode:

var $ = jQuery.noConflict();

$( ".whatever" ).hide();
Last edited 5 years ago by remzicavdar (previous) (diff)

#3 @remzicavdar
5 years ago

  • Keywords needs-patch added; reporter-feedback removed

#4 @dlh
5 years ago

  • Component changed from Customize to Administration
  • Focuses javascript added

Thanks for providing the code snippet.

When I tested deferring jQuery, I got errors throughout the admin, not just the Customizer. For that reason, I'm moving this ticket out of the Customize component, as it doesn't appear to be specific to the Customizer. (#30277 might be the most likely source of any Customizer-specific refactoring.)

You raise fair points about how theme developers can load jQuery, but I wonder whether theme developers should also limit their changes to ! is_admin() or ! is_customize_preview() so the effects are limited to theme templates.

#5 @remzicavdar
5 years ago

  • Summary changed from Defer jQuery WordPress Customizer doesn't work property to Defer jQuery WordPress Admin & Customizer doesn't work properly

#6 @remzicavdar
5 years ago

Okay, thanks for your reply. What do you suggest we should do?
Do a document ready on every script or something else. I'm just a junior web dev and I know WordPress, but not really in depth or each component.

Edit: I know WordPress is using an older version of jQuery for compatibility reasons, but document ready is deprecated as of jQuery 1.8 and removed in jQuery 3.0. See link: https://api.jquery.com/ready/

Before and as of jQuery 3.0 ineffective:

$( document ).ready(function() {
  // Handler for .ready() called.
});

Which is equivalent to:

$(function() {
  // Handler for .ready() called.
});

Which is problematic for WordPress, when $.noConflict() is used to avoid conflicts the $ shortcut is no longer available. However, we could still pass a reference to the jQuery object. This allows us to use the jQuery object with a dollar sign $, like this:

WordPress = jQuery.noConflict();

WordPress(function( $ ) {
  // Code using $ as usual goes here; the actual jQuery object is jq2
});

Edit: Could we upgrade jQuery (3.3.1) to the latest version with jQuery Migrate Plugin to help out all other parties involved? Theme and plugin developers will get notices and this will improve overall effectiveness for improving code.

Last edited 5 years ago by remzicavdar (previous) (diff)

#7 @dlh
5 years ago

I'm sorry, but I'm not sure what the next steps about this topic would be. I would only imagine that any work in changing how jQuery loads would be both a matter of updating core scripts and of ensuring backwards-compatibility for plugins that intentionally affect the admin and rely on core's current behavior.

#8 @remzicavdar
5 years ago

If we upgrade jQuery we could do something like this:

jQuery(function( $ ) {
   // $ Works! You can test it with next line if you like
   // console.log($);
});

In this way we could ensure backwards compatibility. Or if we are not upgrading jQuery we should do something like this:

jQuery = jQuery.noConflict(); // We are already doing this at the end of the jquery file: jQuery.noConflict(); 

jQuery(document).ready(function($) {
   // $ Works! You can test it with next line if you like
   // console.log($);
});

Shall I check/test this? I could first test with the old jQuery?

Edit: I see this is already done at the end in the jQuery file: wp-includes\js\jquery\jquery.js

jQuery.noConflict();

In jQuery 3 there's no need to put this. I have updated my examples.

PS: Could give me an example how I could use ! is_admin() and ! is_customize_preview()to exclude. This is what I do:

<?php
function jquery_updater()
{
    
    // jQuery
    // Deregister core jQuery
    wp_deregister_script('jquery');
    // Register
    wp_enqueue_script('jquery', plugins_url('/js/jquery-3.3.1.min.js', __FILE__), false, '3.3.1');
    
    // jQuery Migrate
    // Deregister core jQuery Migrate
    wp_deregister_script('jquery-migrate');
    // Register
    wp_enqueue_script('jquery-migrate', plugins_url('/js/jquery-migrate-3.0.0.min.js', __FILE__), array(
        'jquery'
    ), '3.0.0'); // require jquery, as loaded above
}

add_action('wp_enqueue_scripts', 'jquery_updater');
Last edited 5 years ago by remzicavdar (previous) (diff)

#9 @dlh
5 years ago

Regarding updating jQuery, it might be better to bring your suggestions to the discussion at #37110, where more contributors are likely to see it. (I was referring to backwards-compatibility just with respect to deferring jQuery — sorry for the confusion.)

The checks for is_admin() or is_customize_preview() could go at the beginning of your script_loader_tag filter:

<?php
function add_defer_attribute( $tag, $handle ) {
        if ( is_admin() || is_customize_preview() ) {
                return $tag;
        }

        // add script handles to the array below
        $scripts_to_defer = array(
                'jquery',
                'jquery-migrate',
                'bootstrap-bundle-js'
        );

        foreach ( $scripts_to_defer as $defer_script ) {
                if ( $defer_script === $handle ) {
                        return str_replace( ' src', ' defer="defer" src', $tag );
                }
        }

        return $tag;
}
add_filter( 'script_loader_tag', 'add_defer_attribute', 10, 2 );

However, Trac isn't meant for support, so you might want to ask additional questions about specific implementations at the support forums or WordPress Development Stack Exchange.

#10 @remzicavdar
5 years ago

Hi @dlh

I made a WordPress plugin to deal with these problems, see: https://github.com/Remzi1993/wp-jquery-updater

My plugin is adds the latest stable version of jQuery and jQuery Migrate to WordPress and only on the frontend. (without affecting the WP backend and Customizer)

<?php
// Front-end not executed in the admin and the customizer (for compatibility reasons)
// See: https://core.trac.wordpress.org/ticket/45130 and https://core.trac.wordpress.org/ticket/37110
function wpj_updater_plugin_front_end_scripts() {
    $wp_admin = is_admin();
    $wp_customizer = is_customize_preview();

    if ( $wp_admin || $wp_customizer ) {
        // echo 'We are in the WP Admin or in the WP Customizer';
    }
    else {
        $plugin = new wpj_updater_plugin;

        // Deregister WP core jQuery
        wp_deregister_script('jquery');
        // Enqueue jQuery in the footer
        wp_enqueue_script( 'jquery', $plugin->jquery, array(), null, true );

        // Deregister WP core jQuery Migrate
        wp_deregister_script('jquery-migrate');
        // Enqueue jQuery Migrate in the footer
        wp_enqueue_script( 'jquery-migrate', $plugin->jquery_migrate, array(), null, true );
    }

}
add_action( 'wp_enqueue_scripts', 'wpj_updater_plugin_front_end_scripts' );

Before that I used jQuery Updater plugin: https://wordpress.org/plugins/jquery-updater/
This is considered bad practice, because it's also affecting WP backend and WP customizer:

<?php
function rw_jquery_updater()
{
    
    // jQuery
    // Deregister core jQuery
    wp_deregister_script('jquery');
    // Register
    wp_enqueue_script('jquery', plugins_url('/js/jquery-3.3.1.min.js', __FILE__), false, '3.3.1');
    
    // jQuery Migrate
    // Deregister core jQuery Migrate
    wp_deregister_script('jquery-migrate');
    // Register
    wp_enqueue_script('jquery-migrate', plugins_url('/js/jquery-migrate-3.0.0.min.js', __FILE__), array(
        'jquery'
    ), '3.0.0'); // require jquery, as loaded above
}

/**
 * Front-End
 */
add_action('wp_enqueue_scripts', 'rw_jquery_updater');
Version 1, edited 5 years ago by remzicavdar (previous) (next) (diff)

#11 @WraithKenny
5 years ago

The convenience of jQuery's .ready() is that it's available early in the page's javascript execution (the head), such that functions can be queue'd (mid page) for execution on document's DOM ready event.

Moving jQuery to the footer, or deferring the script, means that the ready handler is useless, since it'd not be available until wp_footer which is essentially at DOM ready anyway.

There are many plugins and themes that expect and rely on jQuery's other methods to be available on the jQuery object, so even if we had a small shim for the ready method, it'd not solve that problem.

Moving, or deferring, jQuery is fundamentally incompatible with WordPress.

(Note, none of these "issues" are just with any particular version of jQuery, but rather they are *features* of all versions of jQuery.)

Typically, the "best practice" advice of moving *non-critical* scripts to the bottom of the page is good advice that simply doesn't apply to *critical* scripts like jQuery in the WP ecosystem. Besides, we have http2 now, and you can push assets in parallel (the page, and jQuery) which is a far better optimization than literally delaying the downloading of jQuery.

However, if you are optimizing the theme side, for a particular client site, you can take a different route to this optimization, which is to use webpack or similar to bundle jQuery 3 directly from npm while avoiding plugins that rely on early jQuery.

#12 @remzicavdar
5 years ago

We could also provide a set of options. Like what I did with my plugin: https://github.com/Remzi1993/wp-jquery-manager

The default would be what you describe, in the head and jQuery 1x for maximum backward compatibility. And settings for (advanced) users / developers so that they can decide what they want enabled or disabled.


https://ps.w.org/jquery-manager/assets/screenshot-1.png


https://ps.w.org/jquery-manager/assets/screenshot-2.png

Last edited 5 years ago by remzicavdar (previous) (diff)
Note: See TracTickets for help on using tickets.