Boost: Add notice if legacy concatenation files are used #41604

donnchawp · 2025-02-06T17:00:29Z

If the new static cache files introduced in #41056 are not being used by a server then notify the user so they can fix the problem.

Proposed changes:

Add a notice in the CSS and JS concatenation modules. (duplication? Maybe it should be a single notice?)
Add a useStaticMinification hook to fetch the static cache files status
Register a new readonly status to read the static_minification site option as a bool.

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?
Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

pc9hqz-3kc-p2

Does this pull request change what data or activity we track or use?

Testing instructions:

Apply PR and jetpack rebuild plugins/boost to build everything.
Make sure one or both Concatenation modules are enabled.
If it's using the new static files you won't see a notice.
To enforce it using the legacy delivery method:
- Create wp-content/.htaccess with RewriteEngine Off in it.
- Toggle off both Concatenation modules, and toggle them back on.
A notice should appear in the JS module if they are both enabled.
Disable the Concatenate JS module, the notice should now appear under Concatenate CSS.
Enable the Concatenate JS module, the notice should now appear under Concatenate JS.
Define a constant JETPACK_BOOST_DISABLE_404_TESTER within your wp-config.php
Refresh the page. The notice should no longer be visible under any notice.
Comment out or rename .htaccess to htaccess added previously.
Disable both Concatenate modules, and re-enable. The notice should now be gone.
Dismiss the notice and refresh the page, the notice should now be dismissed permanently.

github-actions · 2025-02-06T17:01:27Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Follow this PR Review Process:

Ensure all required checks appearing at the bottom of this PR are passing.
Choose a review path based on your changes:
- A. Team Review: add the "[Status] Needs Team Review" label
  - For most changes, including minor cross-team impacts.
  - Example: Updating a team-specific component or a small change to a shared library.
- B. Crew Review: add the "[Status] Needs Review" label
  - For significant changes to core functionality.
  - Example: Major updates to a shared library or complex features.
- C. Both: Start with Team, then request Crew
  - For complex changes or when you need extra confidence.
  - Example: Refactor affecting multiple systems.
Get at least one approval before merging.

Still unsure? Reach out in #jetpack-developers for guidance!

Boost plugin:

Next scheduled release: March 3, 2025.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

github-actions · 2025-02-06T17:04:46Z

Code Coverage Summary

Coverage changed in 3 files.

File	Coverage	Δ%	Δ Uncovered
projects/plugins/boost/app/assets/src/js/pages/index/index.tsx	0/23 (0.00%)	0.00%	8 💔
projects/plugins/boost/wp-js-data-sync.php	0/282 (0.00%)	0.00%	5 💔
projects/plugins/boost/app/lib/minify/functions-helpers.php	0/135 (0.00%)	0.00%	4 💔

2 files are newly checked for coverage.

File	Coverage
projects/plugins/boost/app/assets/src/js/lib/stores/static-minification.ts	0/3 (0.00%) 💔
projects/plugins/boost/app/data-sync/Minify_Notice_Entry.php	0/3 (0.00%) 💔

Full summary · PHP report · JS report

_{Add label

I don't care about code coverage for this PR
Use this label to ignore the check for insufficient code coveage.
to override the failing coverage check.}

…catenation-notice

- Refactor `useStaticMinification` hook to return more detailed query data - Update Minification modules to use new hook and add immediate 404 tester on activation - Modify module UI to conditionally show notices based on loading and enabled states - Add option to run 404 tester immediately during module activation

- Remove Minify_Notice_Entry and replace with a readonly option for static minification - Add new components for Minify CSS and JS modules - Introduce a LegacyMinifyNotice component for displaying static minification warning - Update performance history hooks to support static minification notice - Simplify index page by extracting minification modules into separate components

jp-launch-control · 2025-02-14T11:18:11Z

Code Coverage Summary

Coverage changed in 3 files.

File	Coverage	Δ%	Δ Uncovered
projects/plugins/boost/wp-js-data-sync.php	0/290 (0.00%)	0.00%	13 💔
projects/plugins/boost/app/assets/src/js/lib/stores/minify.ts	0/10 (0.00%)	0.00%	3 ❤️‍🩹
projects/plugins/boost/app/lib/minify/functions-helpers.php	0/132 (0.00%)	0.00%	1 ❤️‍🩹

3 files are newly checked for coverage.

File	Coverage
projects/plugins/boost/app/assets/src/js/features/minify-css/minify-css.tsx	0/4 (0.00%) 💔
projects/plugins/boost/app/assets/src/js/features/minify-js/minify-js.tsx	0/3 (0.00%) 💔
projects/plugins/boost/app/assets/src/js/features/minify-legacy-notice/minify-legacy-notice.tsx	0/6 (0.00%) 💔

Full summary · PHP report · JS report

_{Coverage check overridden by

I don't care about code coverage for this PR
Use this label to ignore the check for insufficient code coveage.
.}

- Rename and update MinifyLegacyNotice component with more detailed information - Update data sync keys and hooks for legacy minify notice - Modify minify CSS and JS modules to use new legacy notice logic - Remove static-minification store and replace with minify store - Add support for dismissing legacy minify notice with analytics tracking

- Add namespace definition for Jetpack Boost data sync - Rename parameter in jetpack_boost_404_setup for clarity

haqadn

Overall looks good. I have a couple of nitpicks and noticed an issue.

The notice flashes for a moment when you enable any of the concat modules for the first time.

haqadn · 2025-02-14T21:45:11Z

projects/plugins/boost/app/lib/minify/functions-helpers.php

@@ -350,12 +356,12 @@ function jetpack_boost_minify_serve_concatenated() {
 *
 * @return void
 */
-function jetpack_boost_minify_activation() {
+function jetpack_boost_minify_activation( $run_tester_immediately = false ) {


Are there situations where we do not need to run 404 tester immediately on minify activation? If not, I think we should pass a boolean directly to the jetpack_boost_404_setup function.

We do run the 404 tester activation if we detect the plugin has been updated, and they have either Minify Module enabled.

jetpack/projects/plugins/boost/app/class-jetpack-boost.php

Lines 152 to 167 in 55c6ab8

public function handle_version_change() {

$version = get_option( 'jetpack_boost_version' );

if ( $version === JETPACK_BOOST_VERSION ) {

return;

}

update_option( 'jetpack_boost_version', JETPACK_BOOST_VERSION );

if ( jetpack_boost_minify_is_enabled() ) {

// We need to clear Minify scheduled events to ensure the latest scheduled jobs are only scheduled irrespective of scheduled arguments.

jetpack_boost_minify_clear_scheduled_events();

jetpack_boost_minify_activation();

}

}

Though I don't think there would be an issue with running the 404 tester itself immediately in this scenario either. So I'm updating the code to apply your feedback.

haqadn · 2025-02-14T21:47:29Z

projects/plugins/boost/app/lib/minify/functions-helpers.php

@@ -188,9 +188,15 @@ function jetpack_boost_page_optimize_remove_concat_base_prefix( $original_fs_pat
 /**
 * Schedule a cronjob for the 404 tester, if one isn't already scheduled.
 */
-function jetpack_boost_page_optimize_schedule_404_tester() {
+function jetpack_boost_page_optimize_schedule_404_tester( $run_immediately = false ) {


Perhaps we can skip the chain of boolean and run immediately always if no cron is scheduled?

Good point, I've updated the PR to remove the chain of booleans, and in relation to this comment.

haqadn · 2025-02-14T21:59:51Z

projects/plugins/boost/wp-js-data-sync.php

+		// Otherwise show it if the 404 tester determined it can't be used.
+		return ! (bool) get_site_option( 'jetpack_boost_static_minification' );


The flashing notice issue mentioned above is probably because of this. Before the 404 tester has a chance to run, the default value is false as the option doesn't exist yet.

As a result, when you enable a module, initially the notice will show. After it has a chance to fetch, it hides the notice again. I think something like the following might work:

Suggested change

// Otherwise show it if the 404 tester determined it can't be used.

return ! (bool) get_site_option( 'jetpack_boost_static_minification' );

/*

* Until the 404 tester running, the option will not be created and it will return false.

* However, we don't want to show the notice before the tester has a chance to run.

* We only want to show the notice when the tester has already run and determined that

* it will use the fallback system.

*/

return get_site_option( 'jetpack_boost_static_minification' ) !== '';

Nice catch, I've updated the PR with something similar to your solution.

haqadn · 2025-02-14T22:03:59Z

projects/plugins/boost/app/assets/src/js/features/minify-legacy-notice/minify-legacy-notice.tsx

+			>
+				<p>
+					{ __(
+						'You are using the legacy cache delivery method for concatenated files.',


I am a bit unsure about this. But, maybe we don't want to call it legacy in a user-facing way? I think the notice should instead say something like: "You can improve the speed of concatenated files and reduce the load on WordPress." ... Learn More.

You're right legacy may not be the correct word here, we also don't mention it's legacy in the documentation. I've updated the PR with that text, and Oxford comma.

- Remove immediate 404 tester parameter from activation functions - Update legacy notice to conditionally display based on static minification state - Simplify 404 tester scheduling logic - Modify legacy notice text to focus on performance improvement

haqadn

I think it's ready to merge. But, I have one tiny suggestion.

haqadn · 2025-02-17T23:45:53Z

projects/plugins/boost/app/assets/src/js/lib/stores/minify.ts

+	const [ query ] = useDataSync( 'jetpack_boost_ds', 'minify_legacy_notice', z.boolean() );
+
+	return query;


It's a nitpick but since we aren't using other properties, can we narrow it down to data? This will make it cleaner on the end it is being used.

Suggested change

const [ query ] = useDataSync( 'jetpack_boost_ds', 'minify_legacy_notice', z.boolean() );

return query;

const [ query: { data } ] = useDataSync( 'jetpack_boost_ds', 'minify_legacy_notice', z.boolean() );

return data;

We're using the refetch function within the Minify components so the query object is needed unfortunately, e.g.

jetpack/projects/plugins/boost/app/assets/src/js/features/minify-js/minify-js.tsx

Line 22 in 866d7dd

onEnable={ showMinifyLegacy.refetch }

donnchawp added 2 commits February 6, 2025 16:51

Add notice if legacy concatenation files are used

950e65b

changelog

ecd3578

donnchawp added [Status] In Progress [Plugin] Boost A feature to speed up the site and improve performance. labels Feb 6, 2025

donnchawp added this to the boost/next milestone Feb 6, 2025

donnchawp self-assigned this Feb 6, 2025

donnchawp added the [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Feb 6, 2025

donnchawp added 2 commits February 10, 2025 15:29

Merge remote-tracking branch 'origin/trunk' into update/boost/add-con…

40eade9

…catenation-notice

Only show contactenation notice once, and show hide buttons

3a04b58

haqadn removed this from the boost/3.9.0 milestone Feb 11, 2025

donnchawp and others added 2 commits February 13, 2025 10:23

Merge remote-tracking branch 'origin/trunk' into update/boost/add-con…

c159654

…catenation-notice

github-actions bot added the [Boost Feature] Minify label Feb 13, 2025

Make it read write instead of readonly

c104990

donnchawp assigned LiamSarsfield Feb 13, 2025

LiamSarsfield added 2 commits February 14, 2025 14:23

Boost: Minor Improvements to Data Sync and 404 Tester

55c6ab8

- Add namespace definition for Jetpack Boost data sync - Rename parameter in jetpack_boost_404_setup for clarity

LiamSarsfield added the I don't care about code coverage for this PR Use this label to ignore the check for insufficient code coveage. label Feb 14, 2025

LiamSarsfield requested a review from a team February 14, 2025 14:59

LiamSarsfield added [Status] Needs Team Review Obsolete. Use Needs Review instead. and removed [Status] In Progress labels Feb 14, 2025

LiamSarsfield marked this pull request as ready for review February 14, 2025 14:59

github-actions bot added the [Status] In Progress label Feb 14, 2025

haqadn requested changes Feb 14, 2025

View reviewed changes

LiamSarsfield requested a review from haqadn February 17, 2025 12:18

Remove unnecessary argument

866d7dd

LiamSarsfield removed the [Status] In Progress label Feb 17, 2025

haqadn approved these changes Feb 17, 2025

View reviewed changes

Merge branch 'trunk' into update/boost/add-concatenation-notice

6d4265b

LiamSarsfield added this to the boost/3.10.0 milestone Feb 18, 2025

LiamSarsfield merged commit 18f04e8 into trunk Feb 18, 2025
67 checks passed

LiamSarsfield deleted the update/boost/add-concatenation-notice branch February 18, 2025 13:23

github-actions bot removed the [Status] Needs Team Review Obsolete. Use Needs Review instead. label Feb 18, 2025

	public function handle_version_change() {
	$version = get_option( 'jetpack_boost_version' );

	if ( $version === JETPACK_BOOST_VERSION ) {
	return;
	}

	update_option( 'jetpack_boost_version', JETPACK_BOOST_VERSION );

	if ( jetpack_boost_minify_is_enabled() ) {
	// We need to clear Minify scheduled events to ensure the latest scheduled jobs are only scheduled irrespective of scheduled arguments.
	jetpack_boost_minify_clear_scheduled_events();
	jetpack_boost_minify_activation();
	}
	}

		// Otherwise show it if the 404 tester determined it can't be used.
		return ! (bool) get_site_option( 'jetpack_boost_static_minification' );

-		// Otherwise show it if the 404 tester determined it can't be used.
-		return ! (bool) get_site_option( 'jetpack_boost_static_minification' );
+		/*
+		 * Until the 404 tester running, the option will not be created and it will return false.
+		 * However, we don't want to show the notice before the tester has a chance to run.
+		 * We only want to show the notice when the tester has already run and determined that
+		 * it will use the fallback system.
+		 */
+		return get_site_option( 'jetpack_boost_static_minification' ) !== '';

		const [ query ] = useDataSync( 'jetpack_boost_ds', 'minify_legacy_notice', z.boolean() );

		return query;

Boost: Add notice if legacy concatenation files are used #41604

Boost: Add notice if legacy concatenation files are used #41604

Uh oh!

Conversation

donnchawp commented Feb 6, 2025 • edited by LiamSarsfield Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes:

Other information:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Uh oh!

github-actions bot commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Uh oh!

jp-launch-control bot commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Uh oh!

haqadn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haqadn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

donnchawp commented Feb 6, 2025 •

edited by LiamSarsfield

Loading

github-actions bot commented Feb 6, 2025 •

edited

Loading

github-actions bot commented Feb 6, 2025 •

edited

Loading

jp-launch-control bot commented Feb 14, 2025 •

edited

Loading