Subscriptions block: domReady missing

[simison]

Upstream issue: https://wordpress.org/support/?post_type=topic&p=18021829

---

There's a site which skips loading wp-dom-ready core package, breaking Subscribe block and throwing JS error in console:

Subscription form falls back to just submitting the form to subscription management, which as an experience feels broken.

Site: franzvitulli.com Theme: TwentyTwenty Nineteen

Problem

For some reason, the domReady is missing from the site, resulting in a fatal error and a broken Subscribe block. The site also lacks Jetpack_Block_Assets_Base_Url.

Broken site includes only these scripts:

<script src="https://franzvitulli.com/wp-includes/js/dist/vendor/wp-polyfill.min.js" id="wp-polyfill-js"></script>
<script src="https://franzvitulli.com/wp-content/plugins/jetpack/_inc/blocks/subscriptions/view.js?minify=false" id="jetpack-block-subscriptions-js"></script>

A similar (functioning) test site with the same theme has everything it should:

<script type="text/javascript" src="https://usually-managerial.jurassic.ninja/wp-includes/js/dist/vendor/wp-polyfill.min.js?ver=3.15.0" id="wp-polyfill-js"></script>
<script type="text/javascript" id="jetpack-blocks-assets-base-url-js-before">
/* <![CDATA[ */
var Jetpack_Block_Assets_Base_Url="https://usually-managerial.jurassic.ninja/wp-content/plugins/jetpack/_inc/blocks/";
/* ]]> */
</script>
<script type="text/javascript" src="https://usually-managerial.jurassic.ninja/wp-includes/js/dist/dom-ready.min.js?ver=f77871ff7694fffea381" id="wp-dom-ready-js"></script>
<script type="text/javascript" src="https://usually-managerial.jurassic.ninja/wp-content/plugins/jetpack/_inc/blocks/subscriptions/view.js?minify=false&amp;ver=13.9-a.3" id="jetpack-block-subscriptions-js"></script>

How the package normally is loaded

Subscription block uses domReady function from WordPress core:

https://github.com/Automattic/jetpack/blob/70d1104596b18f7e16a5b705fa26c27d7581b35f/projects/plugins/jetpack/extensions/blocks/subscriptions/view.js#L39

The function/package has been there for years already.

When blocks are built, we detect there's an import from @wordpress/dom-ready core package, and "externalize" the import as WP dependency in a generated file projects/plugins/jetpack/_inc/blocks/subscriptions/view.asset.php:

<?php return array('dependencies' => array('wp-dom-ready', 'wp-polyfill'), 'version' => '12d2169125a2760ec964');

This file is then included here:

https://github.com/Automattic/jetpack/blob/70d1104596b18f7e16a5b705fa26c27d7581b35f/projects/plugins/jetpack/class.jetpack-gutenberg.php#L556-L564

[simison]

I could replicate locally with "Speed Optimizer" plugin's "combine JavaScript files" feature. With it toggled, wp-dom-ready-js script disappears; without it everything works:

I would normally recommend excluding the file in plugin's settings, but the dropdown doesn't list any files:

[anomiex]

Sounds like this is a bug with that "Speed Optimizer" plugin, it's breaking script load order.

I see the wp-dom-ready-js code is included in https://franzvitulli.com/wp-content/uploads/siteground-optimizer-assets/siteground-optimizer-combined-js-c82353fe4394b8cf956d4a703915aafc.js, but the extension has it marked as "defer" and is loading it after the jetpack-block-subscriptions-js that depends on it so it won't be loaded in time.

[simison]

In Jetpack, the files are minified at build time. Hence, we include ?minify=false to the file URL when enqueuing the file (added in PR https://github.com/Automattic/jetpack/pull/21748) to instruct the minifier at WP.com servers to skip minifying these assets:

https://github.com/Automattic/jetpack/blob/05b6a0338cdfeed754e04c92f7e9ddb4aaf5c28b/projects/plugins/jetpack/class.jetpack-gutenberg.php#L569

With ?minify=false, Site Ground's JS combinator doesn't include the Subscription block's view.js in the combined JS file loaded as the very last script on the site. They do include the wp-dom-ready script. So the subscription block's view.js and wp-dom-ready end up in a different load order than they should:

Polyfill doesn't get combined and loads in right order, because the file is excluded from being combined in Site ground's plugin:

<script src="https://example.com/wp-includes/js/dist/vendor/wp-polyfill.js?ver=3.15.0" id="wp-polyfill-js"></script>

Here we should have wp-polyfill and Jetpack_Block_Assets_Base_Url constant.

Jetpack Subscription JS doesn't get combined due to ?minify=false in the URL.

<script src="https://example.com/wp-content/plugins/jetpack/_inc/blocks/subscriptions/view.js?minify=false&amp;ver=13.9-a.5" id="jetpack-block-subscriptions-js"></script>

Combined scripts, including Jetpack_Block_Assets_Base_Url constant and wp-dom-ready:

<script  src="https://example.com/wp-content/uploads/siteground-optimizer-assets/siteground-optimizer-combined-js-37d52d08d6f7f3b05647602cb89bfa4a.js"></script>

Often in WP world, files instead have .min.js file extensions, but ours don't, but it shouldn't make a difference. Noting this because I saw Site Ground's JS minifier skip files based on .min.js extensions. Our issues are more with Combinator, not Minifier, but there could be some interplay between the two.

Meanwhile, excluding Jetpack Subscription block from combined JS using Site Ground plugin's filters will solve the issue:

function jetpack_sgo_javascript_combine_exclude( $exclude_list ) {
    $exclude_list[] = 'wp-dom-ready';
    $exclude_list[] = 'jetpack-block-subscriptions';
    return $exclude_list;
}
add_filter( 'sgo_javascript_combine_exclude', 'jetpack_sgo_javascript_combine_exclude' );

function jetpack_sgo_javascript_combine_excluded_inline_content( $excluded_inline_content ) {
    $excluded_inline_content[] = 'Jetpack_Block_Assets_Base_Url';
    return $excluded_inline_content;
}
add_filter( 'sgo_javascript_combine_excluded_inline_content', 'jetpack_sgo_javascript_combine_excluded_inline_content' );

[simison]

@jeherve @anomiex (since you worked on https://github.com/Automattic/jetpack/pull/21748) while not directly related to this issue, would it make sense to return to use .min.js appendix for our minified block assets, since that's a common way to communicate other plugins the file is already minified? Gutenberg does the same.

[anomiex]

Using .min.js would reopen #21343, breaking i18n, unless we started including both non-min and min copies of the JS or started including .js.map files. Last I looked at the latter, it would increase Jetpack unzipped by 31.5–36.7M, and the zip by 8–9M, which is not a change I'd want to make lightly. The former would increase sizes similarly for even less benefit.

Also, I don't see anything in their Combinator class that cares whether the file gets minified by their Minifier class. The exclusion from combination looks to be because the JS contains the strings post_id and setAttribute("id". Why those strings are in their huge array of random strings that disable combination, I have no idea.

This is still a bug in that "Speed Optimizer" plugin. It needs to track dependencies so if a handle is excluded from its bundling, then everything it depends on is still loaded (bundled or unbundled) before that handle, and anything that depends on it is still loaded (bundled or unbundled) after that handle.

They might try switching to Boost, which handles this sort of thing correctly.

[simison]

@anomiex Yeah, the bundle size issue is a bummer; otherwise, the i18n pipeline treats .js and .min.js files identically when enqueueing translations. Non-minified files are also helpful when running WP_DEBUG = true mode; links to source code at the translate site are more meaningful, we wouldn't need to worry about weird and hard to notice minification-bugs when using translation functions, etc. Overall .js+.min.js feels more like the WP way and ours needed some hacks to get i18n+minification work well together.

But you're right, the bug in this case is elsewhere and .min.js convo is more tangential. :-)

[simison]

I've reported the bugs upstream (pending moderation): https://wordpress.org/support/?post_type=topic&p=18021829

[anomiex]

Non-minified files are also helpful when running WP_DEBUG = true mode; links to source code at the translate site are more meaningful, we wouldn't need to worry about weird and hard to notice minification-bugs when using translation functions, etc.

.js.map files give all these benefits, plus debugging in modern-ish browsers works sensibly even if SCRIPT_DEBUG is not set on the server, and it doesn't need changes to the PHP code to decide whether to serve a minified or non-minified version.

At a quick check with some of our larger JS files in Jetpack, the .js.map also winds up a bit smaller than the unminified .js. Webpack's unminified boilerplate seems to outweigh the extra metadata in the map file.