Remove the aggregate stale file threshold and state entry

Added reroll of patch #3 on Drupal 10.1.x.

Please Ignore this patch. By mistake blank patch has been uploaded. I will again upload in the same thread

Added reroll of patch #8 on Drupal 10.1.x.

Looks good, update hook as well. Looks like it wasn't tested so better to not have the code around :)

Needs a bit of re-roll due to changes to system.post_update.php. I'd handle that on commit but I've got a question. Given we've only deprecated \Drupal\Core\Asset\CssCollectionOptimizer and \Drupal\Core\Asset\JsCollectionOptimizer and they both still use these state entries shouldn't we wait until Drupal 11 for this?

Added reroll of patch #14 on Drupal 10.1.x, and removed needs reroll tag.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Rerolled the patch

Running PHPStan on *all* files.
------ ---------------------------------------------------------------------------
Line core/tests/Drupal/Tests/Core/Asset/CssCollectionOptimizerLazyUnitTest.php
------ ---------------------------------------------------------------------------
55 Class Drupal\Core\Asset\CssCollectionOptimizerLazy constructor
invoked with 11 parameters, 10 required.
115 Class Drupal\Core\Asset\CssCollectionOptimizerLazy constructor
invoked with 11 parameters, 10 required.

Updated patch #23 by addressing #24, please review it.

Thanks!

To me #32 sounds reasonable. Even if @alexpott disagrees, it seems fine to run the post update now and we could have another post update later to clean it up again if he feels strongly about this. I don't think there are any usages in contrib.

+++ b/core/lib/Drupal/Core/Asset/CssCollectionOptimizerLazy.php
@@ -135,27 +131,15 @@ public function optimize(array $css_assets, array $libraries) {
   public function getAll() {

Went through contrib uses of this class and didn't see any calls for this. Seems fine to make this change and deprecate the method.

+1 for backporting the changes to \Drupal\Core\Asset\CssCollectionOptimizerLazy::deleteAll to 10.1.x.

+++ b/core/lib/Drupal/Core/Asset/AssetCollectionOptimizerInterface.php
@@ -25,6 +25,11 @@ public function optimize(array $assets, array $libraries);
+   * @deprecated in drupal:10.2.0 and is removed from drupal:11.1.0. There is

+++ b/core/lib/Drupal/Core/Asset/CssCollectionOptimizerLazy.php
@@ -135,27 +131,15 @@ public function optimize(array $css_assets, array $libraries) {
+    @trigger_error(__METHOD__ . ' is deprecated in drupal:10.2.0 and will be removed in drupal:11.0.0, there is no replacement. See https:// www.drupal.org/node/3301744', E_USER_DEPRECATED);

+++ b/core/lib/Drupal/Core/Asset/JsCollectionOptimizerLazy.php
@@ -150,26 +146,15 @@ public function optimize(array $js_assets, array $libraries) {
+    @trigger_error(__METHOD__ . ' is deprecated in drupal:10.2.0 and will be removed in drupal:11.0.0, there is no replacement. See https:// www.drupal.org/node/3301744', E_USER_DEPRECATED);

Could be fixed on commit - s/11.1/11.0

Will this fix the issue in 10.1 where if a user clears cache, it doesn't refresh the aggregate CSS/JS that's pulled down?

@mherchel - I just applied this to my 10.1 and other than a warning "] rmdir(/files/css): Directory not empty FileSystem.php:267" it did appear to fix the aggregate CSS/JS on my Pantheon deploy.

@mherchel yes it should.

Just tested this out, and it doesn't appear to be working.

Not sure if this blocks this issue, or if this should be reported in a followup.

Testing steps

1. Ensure aggregated CSS/JS, caching enabled etc.
2. View source. Copy filename including querystring and save it (It looks something like

   /sites/default/files/css/css_hegpbZvWLCegz1eocO6sYJ0X_AGboa15OU4bOdIZPaI.css?delta=0&language=en&theme=olivero&include=eJx9UFuSgyAQvBDqGfYkqQF6lV1kqBmI6-0XEzWVVCo_MPQDmnYsGLzUTLHPwqNA1bgN5ORwnyKT_9nRXephud4FqeCvVIoH9UC6GNKvGl21YB4sKZ7uIFfCFTeR4dhG4SHzAoHv7NrZyO5BfAPeXAMWHW5rP7OvESdfaNTzkOgaRiqBU6doeTzJepIKEjd1S_B4Z8gS5jfyRCK8PHcQNEdqHRTmaEmGfTdVIYdom_vQMuz9hVaOpEbohDxBfHPXzVql07K2MsYH8vRcIWsbO1OiEfK5-COJTizF1XJIjrMhP4d0eQneFwE-UP3ErZLX7_ZQRxlfm83YMF5yyBiO4R-XNOx7

3. Clear cache
4. View source again and compare the aggregate filename/querystring against what I checked before.
5. I expect the filename or querystring to change but it doesn't. I also manually modified Olivero's CSS (to intentionally make it out of date), but that had no effect.

Yep. Verified its working 🙌

Testing steps

1. Verified path of CSS aggregate file
2. Ran drush cr
3. noted that entire CSS directory is now removed from sites/files
4. modified a CSS file in Olivero
5. Refreshed the path to the CSS file
6. Noted that the new file is created, and it has the updated content.

Thank you!

+++ b/core/lib/Drupal/Core/Asset/CssCollectionOptimizerLazy.php
@@ -142,20 +142,7 @@ public function getAll() {
-    $this->state->delete('drupal_css_cache_files');

Shouldn't we keep this?

Committed 405f8b33a7 and pushed to 11.0.x. Also committed the 10.1.x patch. Thanks!

#45, It works when I clear the cache via the admin toolbar, but not when I do it via Drush.
When I run drush cr I get the following messages:

[notice] The file assets://css was not deleted because it does not exist.
[notice] The file assets://js was not deleted because it does not exist.

CSS and JS are not removed and new files are not created.

#45, works when I run cache clear via admin toolbar, but doesn't work via Drush.
When I run drush cr command I get the following messages:

[notice] The file assets://css was not deleted because it does not exist.
[notice] The file assets://js was not deleted because it does not exist.

As a result, old CSS and JS files are not deleted, and new ones are not created.

Sorry for the duplicate comment, i accidentally

I did get the notices @BadWebCoder mentioned when I cleared cache with Drush,

$ vendor/bin/drush cr
 [notice] The file assets://css was not deleted because it does not exist.
 [notice] The file assets://js was not deleted because it does not exist.

But when I refreshed the page, my CSS changes did apply to the page.

Off-topic: I can also see that some of the forum topics I added on the site which I saw on the PC (it is a fresh install), viewing the site on a phone I can't see the most recent forum topics. Content this time and not CSS or JS.

Kindly ignore the off-topic above. It was my fault. I had wrong forum access permissions set.

If CSS aggregation is turned on, CSS changes don't show up on the site and the site remains in the past never changing, whether caches are cleared in Drush or UI.

The existing code was already deleting files in that directory, just not the directory itself

@catch, seems it is deleting the directory as well.

https://git.drupalcode.org/project/drupal/-/blob/10.1.1/core/lib/Drupal/...

Currently it's also deleted the JS directory and both are never recreated (tested on both 10.1.1 and 10.1.x-dev).

We are having the same issue as @FiNeX in #56.

published the change record

AFAICT this incorrectly removed system.performance:stale_file_threshold from config schema. It should have deprecated that instead? 🤔 (That is possible since #2997100: Introduce a way to deprecate config schemas.)