-
Notifications
You must be signed in to change notification settings - Fork 797
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Subscriptions block: domReady missing #39418
Comments
I could replicate locally with "Speed Optimizer" plugin's "combine JavaScript files" feature. With it toggled, I would normally recommend excluding the file in plugin's settings, but the dropdown doesn't list any files: |
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. |
In Jetpack, the files are minified at build time. Hence, we include
With
Often in WP world, files instead have 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' ); |
Using 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 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. |
@anomiex Yeah, the bundle size issue is a bummer; otherwise, the i18n pipeline treats But you're right, the bug in this case is elsewhere and |
I've reported the bugs upstream (pending moderation): https://wordpress.org/support/?post_type=topic&p=18021829 |
At a quick check with some of our larger JS files in Jetpack, the |
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 lacksJetpack_Block_Assets_Base_Url
.Broken site includes only these scripts:
A similar (functioning) test site with the same theme has everything it should:
How the package normally is loaded
Subscription block uses
domReady
function from WordPress core:jetpack/projects/plugins/jetpack/extensions/blocks/subscriptions/view.js
Line 39 in 70d1104
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 fileprojects/plugins/jetpack/_inc/blocks/subscriptions/view.asset.php
:This file is then included here:
jetpack/projects/plugins/jetpack/class.jetpack-gutenberg.php
Lines 556 to 564 in 70d1104
The text was updated successfully, but these errors were encountered: