In Likes and Comment Likes modules, prevent blocking page load with 2 scripts

[kienstra]

Fixes #13929

Changes proposed in this Pull Request:

• If likes are disabled globally or for a post, prevents enqueueing scripts for the Likes or Comment Likes modules
• Passes to wp_enqueue_script() the 5th $in_footer argument of true, to prevent the scripts from blocking the page load
• These scripts look to be mainly jQuery plugins
• So far, the 'Like' button seems to work fine with these in the footer. But this could of course use more testing.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

• It modifies the performance of an existing feature, no functional change intended.

Testing instructions:

1. On a non-local site, ensure 'Add Like buttons...' is toggled on:
   
2. Go to the front-end of a post
3. Expected: The 'Like' button should appear, and should be clickable:

Proposed changelog entry for your changes:

• Performance enhancement for the Likes module

[jetpackbot]

	Warnings
⚠️	pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 65471ed

[kienstra]

Request For Review

Hi @dero,
Really good idea for the Map Block earlier.

Could you please review this when you have a chance?

Thanks, Dero!

[dero]

@kienstra The changes look good, I've verified the scripts get loaded before the https://widgets.wp.com/likes/master.html iframe (which depends of them being loaded) in the footer.

I'd suggest we make the same change in the sibling comment-likes.php module as well. These two seem the only ones to pull those scripts in.

[kienstra]

Nice, thanks for reviewing this.

I'd suggest we make the same change in the sibling comment-likes.php module as well. These two seem the only ones to pull those scripts in.

Oh, good idea.

[kienstra]

This PR now addresses #13929 (comment) for the Likes and Comment Likes modules.

It prevents the JS from being enqueued when Likes are disabled for a post or globally.

Steps To Test

1. In a non-local site, ensure 'Add like buttons' is toggled on:
   

2. Toggle on 'Enable comment likes':

3. On a post, uncheck 'Show likes':

4. Go to the front-end permalink, not just the preview URL
5. Search the source for queuehandler.js. That's used in the Likes and Comment Likes module.
6. Expected: it's not in the source:

7. In the block editor, check 'Show likes' (reversing what you did in step 3)

8. Toggle off 'Add like buttons' from step 1 above, and toggle off 'Enable Like Comments' from step 2

9. Repeat steps 4-6, queuehandler.js should still not be in the source

[kienstra]

Request For Review

Hi @dero,
Thanks your suggestion:

I'd suggest we make the same change in the sibling comment-likes.php module as well. These two seem the only ones to pull those scripts in.

Could you please review this again, now that it addresses your suggestion?

It also addresses issue #13929

Thanks, Dero!

[josephscott]

Passes to wp_enqueue_script() the 5th $in_footer argument of true, to prevent the scripts from blocking the page load

Has it been confirmed that this change of moving the script to the footer impacts how quickly the page renders?

I ask because in general moving a script tag to the bottom of the page will not change the impact in terms of render blocking. Browsers like Chrome will quickly scan the HTML looking for resources it thinks will be important to rendering the page and loads them early on, mainly CSS and JS. Even if they are listed at the very bottom of the page.

You can see that happening with this test - https://www.webpagetest.org/result/200304_1E_35a21961f2be5996d4a1613a687f7c3b/1/details/#waterfall_view_step1 - the third request in the page load was from a script tag at the bottom of the page ( bundle.js ). Specifically line 1333, out of 1436 lines.

[kienstra]

Hi @josephscott,
Thanks for testing that, and sharing the results of the webpagetest.

No, I haven't tested the performance difference between passing the 5th $in_footer argument as true or false.

That's a general best practice, but you're probably right that it's not a big impact.

I'll look at it in more detail.

[kienstra]

Performance impact of moving scripts to footer

Hi @josephscott,
Sorry for the delay.

Like you mentioned, the performance benefit of only moving the scripts to the footer looks small.

In 10 trials, the average time to First Contentful Paint was 3.15 with the scripts moved to the footer, as opposed to 3.4 with them above the footer.

But most of that difference is from outliers. The most common case is that there is no difference.

Steps to reproduce

1. checkout this PR's branch

2. Ensure 'Add like buttons' is toggled on:
   

3. Activate Twenty Twenty

4. Create a simple post, and publish it

5. In Chrome, go to the front-end of it on an Incognito window

6. Run 10 Lighthouse audits for Mobile

7. Revert this PR's diff for the 5th argument of wp_enqueue_script():

diff --git a/modules/comment-likes.php b/modules/comment-likes.php
index 8868f2688..4332e8eb5 100644
--- a/modules/comment-likes.php
+++ b/modules/comment-likes.php
@@ -142,7 +142,7 @@ class Jetpack_Comment_Likes {
                        Assets::get_file_url_for_environment( '_inc/build/postmessage.min.js', '_inc/postmessage.js' ),
                        array( 'jquery' ),
                        JETPACK__VERSION,
-                       true
+                       false
                );
                wp_enqueue_script(
                        'jetpack_resize',
@@ -152,7 +152,7 @@ class Jetpack_Comment_Likes {
                        ),
                        array( 'jquery' ),
                        JETPACK__VERSION,
-                       true
+                       false
                );
                wp_enqueue_script( 'jetpack_likes_queuehandler', plugins_url( 'likes/queuehandler.js' , __FILE__ ), array( 'jquery', 'postmessage', 'jetpack_resize' ), JETPACK__VERSION, true );
        }
diff --git a/modules/likes.php b/modules/likes.php
index 4a0b0273a..f4d3f683d 100644
--- a/modules/likes.php
+++ b/modules/likes.php
@@ -296,7 +296,7 @@ class Jetpack_Likes {
                        Assets::get_file_url_for_environment( '_inc/build/postmessage.min.js', '_inc/postmessage.js' ),
                        array( 'jquery' ),
                        JETPACK__VERSION,
-                       true
+                       false
                );
                wp_register_script(
                        'jetpack_resize',
@@ -306,7 +306,7 @@ class Jetpack_Likes {
                        ),
                        array( 'jquery' ),
                        JETPACK__VERSION,
-                       true
+                       false
                );
                wp_register_script(
                        'jetpack_likes_queuehandler',

8. Run another 10 Lighthouse audits for Mobile

Results

First Contentful Paint in seconds
Without/with this PR's moving of scripts to footer

Without this PR	With this PR
3.8	3.3
3.3	3.3
3.3	3.3
3.3	3.3
3.8	3.3
3.3	3.3
3.3	2.2
3.3	2.9
3.3	3.3
3.3	3.3

Average

Without this PR	With this PR
3.4	3.15

Difference

0.25 seconds, on average
Still, the most common case is that there's no difference

[dero]

I ask because in general moving a script tag to the bottom of the page will not change the impact in terms of render blocking.

@josephscott In my understanding speculative parsing is only a heuristic that is supposed to "guess" which assets will be needed for page construction and thus it only affects the timing of network requests. The synchronous script execution still blocks the HTML parser and thus introduces delays in page render.

To verify this, I've inspected the main thread tasks sequence on master and this branch and checked which segments of the incoming HTML markup are parsed when.

On master:

• 0 ms mark: first 260 lines of markup are parsed.
• Then style.css is parsed synchronously.
• Then all synchronous scripts in <head> are executed up until jquery.jetpack-resize.js.
• 725 ms mark: lines 264..679 of markup are parsed.
• Then all synchronous scripts from the footer of the page are executed.
• 1025 ms mark: the rest of the markup is parsed.

On this branch:

• 0 ms mark: first 260 lines of markup are parsed.
• Then style.css is parsed synchronously.
• Then all synchronous scripts in <head> are executed up until jquery.migrate.js.
• 590 ms mark: lines 262..677 of markup are parsed.
  • Notice on screenshot: This happens while postmessage.js and jquery.jetpack-resize.js are still being loaded.
• Then all synchronous scripts from the footer of the page are executed including the ones being moved by this PR.
• 880 ms mark: the rest of the markup is parsed.

Based on this experiment, I'd say we can conclude that:

• Speculative parsing allows assets to start loading even before the parser processes their <script> element.
• The engine still halts the renderer when it encounters a synchronous style / script, which means that sequencing matters.
• On a simple page with simple and small blocking assets, the gains may be imperceptible, but the principle still holds.

cc @kienstra

[josephscott]

@dero now this is what I like! When a claim is made about performance, it needs to be confirmed.

[kienstra]

Yeah, thanks a lot for your really detailed look at this! 😄

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello kienstra! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D39961-code before merging this PR. Thank you!

[kraftbj]

r203942-wpcom