wp-rocket icon indicating copy to clipboard operation
wp-rocket copied to clipboard

Delay JavaScript Execution compatibility with the MinimalistBlogger

Open DahmaniAdame opened this issue 2 years ago • 4 comments

Before submitting an issue please check that you’ve completed the following steps:

  • [x] Made sure you’re on the latest version 3.11.0.2
  • [x] Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug When Delay JavaScript Execution is using with the theme MinimalistBlogger, the following error shows on the browser console on user interaction:

Uncaught TypeError: Cannot read properties of undefined (reading 'on')
    at script.js?ver=1649406382:4:2030
    at script.js?ver=1649406382:4:2096
(anonymous) @ script.js?ver=1649406382:4
(anonymous) @ script.js?ver=1649406382:4

Referring to the following file:

/themes/minimalistblogger/js/script.js

The issue only happens when the jquery library is not excluded. It's fixed once it's excluded.

To Reproduce Steps to reproduce the behavior:

  1. Use the MinimalistBlogger theme
  2. Enable Delay JavaScript Execution
  3. See error

Expected behavior MinimalistBlogger should work out of the box with Delay JavaScript Execution.

Screenshots N/A

Additional context Ticket - https://secure.helpscout.net/conversation/1839489447/336073

Backlog Grooming (for WP Media dev team use only)

  • [x] Reproduce the problem
  • [x] Identify the root cause
  • [x] Scope a solution
  • [x] Estimate the effort

DahmaniAdame avatar Apr 08 '22 08:04 DahmaniAdame

Reproduce the issue :heavy_multiplication_x:

Has I don't have minimalistblogger I cannot reproduce the issue.

However from the informations you gave me I can clearly see that the problem comes from jQuery that is defined too late as .on is a common function from jQuery.

Scope a solution :white_check_mark:

As jQuery is a common library widely used on Wordpress we can't exclude it directly in the core.

That's why I propose to create a Thirdparty class for minimalistblogger that check in get_subscribed_events check if the theme is active and add: $events['rocket_delay_js_exclusions'] = 'exclude_jquery_from_delay_js';

public function exclude_jquery_from_delay_js( array $exclusions = [] ) {
	$exclusions[] = '/jquery-?[0-9.](.*)(.min|.slim|.slim.min)?.js';
	$exclusions[] = '/jquery-migrate(.min)?.js';
	return $exclusions;
}

Create Unit and integration tests for the new class

Estimate the effort :white_check_mark:

Effort [S]

CrochetFeve0251 avatar Apr 11 '22 12:04 CrochetFeve0251

@DahmaniAdame You mentioned here:

The issue only happens when the jquery library is not excluded. It's fixed once it's excluded.

To confirm:

  1. We can delay the jQuery without any problems
  2. We need to always exclude from the DelayJS /themes/minimalistblogger/js/script.js script. It'll then work correctly when jQuery is delayed or not

piotrbak avatar Apr 11 '22 12:04 piotrbak

@piotrbak this can't be confirmed unless we test by changing themes. But since the dependency with /themes/minimalistblogger/js/script.js was the only one to trigger the error, I would assume it would have been possible to delay jQuery only if using a different theme.

DahmaniAdame avatar Apr 15 '22 07:04 DahmaniAdame

@CrochetFeve0251 Groomed solution is the correct one.

Thank you @DahmaniAdame 🙏

piotrbak avatar Apr 19 '22 12:04 piotrbak