RUCSS and Autoptimize compatibility

[NataliaDrause]

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

• Made sure you’re on the latest version - 3.9.3
• Used the search feature to ensure that the bug hasn’t been reported before - yes

Describe the bug When Autoptimize CSS Options -> Optimize CSS Code? and/or Aggregate CSS-files? options are enabled, and Remove Unused CSS option in WP Rocket is turned on, the Autoptimize stylesheets are not processed and unused CSS not removed.

To Reproduce Steps to reproduce the behavior:

1. Activate CSS Options -> Optimize CSS Code? and/or Aggregate CSS-files? options in Autoptimize plugin.

2. Activate Remove Unused CSS option in WP Rocket and wait for the process to complete

3. Check the site and the source code. The inline CSS will be processed and Used CSS will be added, but the Autoptimize stylesheets (that have .php extension) are not removed:

Expected behavior All stylesheets have to be processed and the Unused CSS has to be removed.

Additional context

Currently, only the Minify/Combine CSS options are being grayed out in WP Rocket when Autoptimize CSS optimization options are enabled:

Since the Autoptimize CSS optimization options are becoming redundant when Remove Unused CSS in WP Rocket is enabled, as a possible solution, the RUCSS option can be grayed out and a notice to be updated to something like this: " CSS Optimization is currently activated in Autoptimize. If you want to use WP Rocket’s CSS optimization features, disable those options in Autoptimize."

Related ticket: https://secure.helpscout.net/conversation/1610683804/289711/

Backlog Grooming (for WP Media dev team use only)

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

[Mai-Saad]

Using Autoptimize 2.9.2, it was reproducible on the test site when both Optimize CSS Code? and Aggregate CSS-files? are enabled.

[vmanthos]

Related tickets: https://secure.helpscout.net/conversation/1646634460/298171/ https://secure.helpscout.net/conversation/1644474396/297674

[remyperona]

Does it happen when the option to save optimized/aggregated files as static is enabled in Autoptimize configuration?

On our side, in the RUCSS code, we only remove from the HTML CSS that have the .css extension, so if the CSS file is using .php, it is ignored.

[NataliaDrause]

Related: https://secure.helpscout.net/conversation/1680132305/304660?folderId=3864740

[remyperona]

After testing, the files are correctly processed if they are saved as static file in the Autoptimize configuration.

@GeekPress This is happening only if Save aggregated script/css as static files? is disabled in Autoptimize settings, because in that case the extension is .php instead of .css.

Should we recommend to users to enable this option in Autoptimize? It shouldn't really be ever disabled for performance.

[piotrbak]

@Tabrisrp I think we should easily go with the notice approach, similar to this one: https://github.com/wp-media/wp-rocket/issues/4472#issuecomment-963149723

We should rather encourage people to disable combination at all, since it will just add more, unnecessary processing.

[iCaspar]

Grooming notes:

Reproduced: ✅ (see https://github.com/wp-media/wp-rocket/issues/4327#issuecomment-962171183) Root cause: ✅ (see https://github.com/wp-media/wp-rocket/issues/4327#issuecomment-948026456) Solution: (simple notice solution, per https://github.com/wp-media/wp-rocket/issues/4327#issuecomment-963171805)

1. Create a new function rocket_maybe_disable_rucss() in inc/3rd-party/plugins/autoptimize.php that checks for (a) is_plugin_active( 'autoptimize/autoptimize.php' ) AND
   (b) ('on' === get_option( 'autoptimize_css' ) || 'on' === get_option( 'autoptimize_css_aggregate' )

2. In inc/Engine/Admin/Settings/Page.php add an item to the add_settings_fields array under optimize_css_delivery:

'helper' => rocket_maybe_disable_rucss() ? __( 'We have detected that Autoptimize's Optimize CSS and/or Aggregate CSS feature is enabled. Remove Unused CSS will not be applied to the Autoptmized files created by those settings. We suggest disabling them to take full advantage of Remove Unused CSS.' ) : '',

Estimate Effort: [XS]

[iCaspar]

In the combined rucss/cpcss UI of 3.10, and in conjunction with a similar helper text that should be added via #4013, it may be necessary to also use a bit of JS to toggle the helper text above with the helper text pertenant to the Aggregate inline/CPCSS conflict, depending on which of the Optimize selections is active.

[piotrbak]

We'll need to perform some performance tests with the new RUCSS approach and then decide whether to implement it or not

[piotrbak]

In the new approach we need to check for both JavaScript and CSS combination. That's because we're looking for compatibility conditions within the CSS/JS files.

Another solution would be possible from AO side. @futtta 👋 Would it be possible not to apply AO features on the pages ending with query string ?nowprocket?

If so, then we could make sure that features are compatible with RUCSS and we could avoid adding another notices to the dashboard.

[futtta]

:wave: Maybe try with ?ao_noptirocket=1 @piotrbak, that should already work? ;-)

[piotrbak]

Didn't know about this query string! Unfortunately, it might not work for us, as we need to make sure that we cover all products that have combination features :/

Moreover, during my tests I realised that AO combination feature is happening after we inject the Used CSS 😢 It ends up with AO's combined CSS file above our automatically preloaded fonts...

It would be great if it would be possible to have no AO features under the ?nowprocket, then we could:

1. Disable automatically AO's CSS combination when we detect that our RUCSS is enabled
2. Preserve all AO's JS features including combination, as our SaaS will visit the not cached version ?nowprocket

What do you think? 🤔

[futtta]

Hmm, I'm not principally against adding ?nowprocket, but given you'll probably have to ask other optimization plugin devs (as per "all products that have combination features") the same question, it would make sense to standardize on a non-branded "do not optimize" parameter (much like we standardized the skip-lazy class to disable lazyloading of an image back in 2020, @GeekPress or @Tabrisrp might remember).

What if AO & WPR and anyone else who would want to jump on board (I have contact data of must plugin devs) would stop optimizing when e.g. ?no_optimize or ?no_cache (the latter I think I have even already seen used) is found in the querystring?

[futtta]

Just added support for both ?no_cache and ?no_optimize in what will be AO 3.0.3 (minor release out tomorrow), see https://github.com/futtta/autoptimize/commit/dcbcfb5c0514ca05612fd999318fa3d048aace56

[GeekPress]

@futtta Thanks for adding ?no_cache and ?no_optimize.

Unfortunately, we won't be able to use them.

Why?

Our SaaS used to generate Remove Unused simulates a visit on the website with the ?nowprocket query string to be sure to get the uncached version with no optimization.

If we change the query string to ?no_cache or no_optimize on our SaaS, it means we won't correctly generate the Used CSS for all users who will not update to the version which will integrate the ?no_cache or no_optimize.

If you can do a favor for this exceptional case, everybody will appreciate it =)

[futtta]

As you asked so kind, see https://github.com/futtta/autoptimize/commit/090bef03e74795624274a445db1a91a82182d987

That should give users some more time to update to versions that integrate no_cache/ no_optimize, but do consider this is a temporary workaround ;-)

On 18/07/2022 15:26, Jonathan Buttigieg wrote:

@futtta https://github.com/futtta Thanks for adding |?no_cache| and |?no_optimize|.

Unfortunately, we won't be able to use them.

Why?

Our SaaS used to generate Remove Unused simulates a visit on the website with the |?nowprocket| query string to be sure to get the uncached version with no optimization.

If we change the query string to |?no_cache| or |no_optimize| on our SaaS, it means we won't correctly generate the Used CSS for all users who will not update to the version which will integrate the |?no_cache| or |no_optimize|.

If you can do a favor for this exceptional case, everybody will appreciate it =)

— Reply to this email directly, view it on GitHub https://github.com/wp-media/wp-rocket/issues/4327#issuecomment-1187424658, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABMIMIBEUSIGGC5N3IMADLVUVLQFANCNFSM5DOOB3NA. You are receiving this because you were mentioned.Message ID: @.***>

[GeekPress]

Thanks @futtta

I'm afraid this will need to be a permanent enhancement to implement.

Whenever we decide to implement it into our SaaS, changing the URL will cause a ton of users to have an inaccurate RUCSS.

I can see that you already have specific conditions for Elementor, Visual Composer, etc...

[futtta]

I would make sure my SAAS adds both nowpocket AND no_cache (or no_optimize) in the querystring and have newer versions of my premium plugin honor no_cache instead of nowprocket. That way over time (almost) all sites will honor no_cache and nowprocket can be gently laid to rest. But it's not my SAAS or premium plugin, obviously ;-)

On 18/07/2022 16:10, Jonathan Buttigieg @.***> wrote:

Thanks @futtta https://github.com/futtta

I'm afraid this will need to be a permanent enhancement to implement.

Whenever we decide to implement it into our SaaS, changing the URL will cause a ton of users to have an inaccurate RUCSS.

I can see that you already have specific conditions for Elementor, Visual Composer, etc...

— Reply to this email directly, view it on GitHub https://github.com/wp-media/wp-rocket/issues/4327#issuecomment-1187532377, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABMIMLWM7MO52TFWRSG3HLVUVQVJANCNFSM5DOOB3NA. You are receiving this because you were mentioned.Message ID: @.***>

[GeekPress]

Ah ok, I didn't think about that solution. Thanks for sharing. I will discuss it with our developers asap.

[GeekPress]

@mostafa-hisham Do you see any problem with SaaS cumulating both query strings (nowprocket & no_optimize ) when we visit a page?

Can we do the change directly in SaaS or does it require a change in the plugin?

[GeekPress]

I found we are adding the nowprocket query string in the plugin: https://github.com/wp-media/wp-rocket/blob/develop/inc/Engine/Optimization/RUCSS/Frontend/APIClient.php#L28

I suppose we can add the no_optimize at the same moment

[piotrbak]

As per discussion with @mostafa-hisham, this change won't affect RUCSS.

[remyperona]

@piotrbak Is the addition of the new query string the only thing needed now for this issue?

[piotrbak]

@Tabrisrp Yes, @mostafa-hisham mentioned that this is just enough, no other changes needed.