Move system/base component CSS to respective libraries where they exist

The MR will fail because librariesy-overrides and possibly test coverage will need updating, but did a before/after with devtools moving just progress.module.css for now.

Before: 2.9kb transferred over network, resource size 9.3kb
After: 2.7kb transferred over network, resource size 8.7kb

Pushed a commit that should make tests pass. Moving to needs review - we may or may not decide to do the other two (or more) CSS files in this issue, but views on the general approach would be great.

+1 for better split!

With tabledrag and autocomplete done too:

1.8kb transferred over network, resource size 5.5kb

This is nearly half the filesize for the first CSS aggregate (standard profile) already.

Pretty sure we can also do:

AJAX progress - can be moved to drupal.ajax - this could probably be done in this issue.

      css/components/system-status-counter.css: { weight: -10 }
      css/components/system-status-report-counters.css: { weight: -10 }
      css/components/system-status-report-general-info.css: { weight: -10 }

- can make a new library for the status report and then attach from there, but no existing library - needs its own issue probably.

tablesort - we could make a new library for tablesort and move the CSS there, not sure exactly where's best to attach it from yet (TableSort.php somewhere?), so could also use its own issue.

Opened 📌 Split system status report CSS out to its own library and attach it via tablesort Active and 📌 Split tablesort.module.css out to its own library and attach it via tablesort Active

It also may affect JS tests time as less data transfered

Fixed that last Claro issue and added a change record: https://www.drupal.org/node/3432346 →

Tagging with sustainability because although it's a tiny change, it's likely to reduce bandwidth requirements on 99.9% of Drupal sites including for anonymous users - the only sites not affected would be those actively removing these files from the library definitions.

Now the first CSS aggregate in the standard profile/olivero for anonymous users on the front page is less than half the size both compressed and uncompressed:

Before:
Before: 2.9kb transferred over network, resource size 9.3kb

After:
1.3kb transferred over network, resource size 3.5kb

I find it ready to go, thank you

Please don't rush to commit this one, need to look at it more closely

I think we should update https://www.drupal.org/about/core/policies/core-change-policies/frontend... → to explicitly say that the location of JavaScript and CSS files is @internal and that libraries-override implementations may need to be updated in minor releases. I realise this is the opposite of what I said about bc circa 8.6 in #2484623: Move all JS in modules to a js/ folder → but we've done a lot more minor releases since then. It might be worth grepping contrib and pro-actively opening issues for any themes affected though. Supporting two minor branches should be OK too, because libraries-override will silently ignore an override of a file that doesn't actually exist.

If we did want to implement bc for all of these, we could probably add it to LibraryDiscoveryParser directly, we'd need a mapping with something like:

$mapping = [
  'new_library' => [
     'old_library' => [
         'new_filename' => 'old_filename',
     ],
   ],
];     

Then in ::setOverrideValue() when we're applying overrides to the library, we'd look up whether there are any entries for that library, then look for any overrides of the old library, then whether any of the files match, then add those to the replacements and trigger a deprecation error. If we want to do that, should be its own issue and postpone this and similar issues on it.

The JS changes were kinda expected to module/theme maintainers. The CSS ones will come from nowhere to them so BC layer definitely needed if we don't want to make a lot of people frustrated with updating

📌 Refactor system/base library Needs work also attempted something like this.

I think we should perhaps have a meta to go through each of the libraries in system/base and decide whether they are still needed, as well as the ones listed here already and in #10, position-container doesn't seem used at all, reset-appearance is used exactly once in Claro, resize only is needed for textareas, etc.

I hadn't seen 📌 Refactor system/base library Needs work , it's taking a completely different approach, which I'm not sure if it's better or worse yet - creates new libraries for each file, and adds them in the templates with the markup they correspond to. The problem here would be if a theme overrides those templates and doesn't update, then the libraries won't be loaded any more. But also if code is using those templates generically without the libraries that we're moving files to here, then it would work better. Would be good to get an opinion from someone who actually does theme things about that.

While that issue doesn't move the files, it does change the library definitions which will also break library-overrides, so I've gone ahead and opened 📌 Add a deprecation support for library-overrides Active to add deprecation support - if we've got that, then we don't need to discuss whether to add it or not :)

Opened a meta at 📌 Audit the system/base libraries Active too, although I think we can go ahead with these couple of issues parallel to that, then audit whatever's left.

+1 on not sending unneeded CSS. I agree that the changes could potentially cause issues with libraries-override... but I personally haven't ever overridden these particular CSS files on a frontend theme. The use cases where that would happen are pretty slim. The only places where I've done this before is when doing core work on Claro.

Nice. Any sense of how many bytes or even kb's might be saved by doing this? Given the number of Drupal installs in the wild, this seems like it could reduce a lot of CO2 distributed across our community.

Re #20/21 yeah I'm not sure most themers specifically override these CSS files, they likely just add their own CSS rules that come after the core ones if they want to adjust the styling of these elements?

@mgifford see #13, it's already saving 1.6kb compressed, 6kb uncompressed and there are at least four more files we can stop loading on every request.

I've got working deprecation support up at 📌 Add a deprecation support for library-overrides Active now.

Went ahead and added the moved_files entities here now that 📌 Add a deprecation support for library-overrides Active is RTBC.

Discussed this with @nod_ in slack and he preferred merging the files into the existing libraries rather than creating new ones, so keeping going with the approach here. We can repurpose 📌 Refactor system/base library Needs work once this lands for what's left.

I've added asset file size support to performance tests over in 📌 Record total css and js file size in performance tests Fixed .

I also added a draft MR on that issue, combining the test coverage with the changes here, so we can see the difference in the test coverage, the test changes as a result of that are here:
https://git.drupalcode.org/project/drupal/-/merge_requests/7231/diffs?co...

diff --git a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryAuthenticatedPerformanceTest.php b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryAuthenticatedPerformanceTest.php
index d623c59a90..af01bf3fe0 100644
--- a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryAuthenticatedPerformanceTest.php
+++ b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryAuthenticatedPerformanceTest.php
@@ -36,7 +36,7 @@ public function testFrontPageAuthenticatedWarmCache(): void {
       $this->drupalGet('<front>');
     }, 'authenticatedFrontPage');
     $this->assertSame(2, $performance_data->getStylesheetCount());
-    $this->assertSame(47552, $performance_data->getStylesheetBytes());
+    $this->assertSame(44966, $performance_data->getStylesheetBytes());
     $this->assertSame(1, $performance_data->getScriptCount());
     $this->assertSame(132038, $performance_data->getScriptBytes());
 
diff --git a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryFrontPagePerformanceTest.php b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryFrontPagePerformanceTest.php
index 36590df28c..5f3fbff2c4 100644
--- a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryFrontPagePerformanceTest.php
+++ b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryFrontPagePerformanceTest.php
@@ -66,7 +66,7 @@ public function testFrontPageHotCache() {
     $this->assertSame(1, $performance_data->getScriptCount());
     $this->assertSame(7075, $performance_data->getScriptBytes());
     $this->assertSame(2, $performance_data->getStylesheetCount());
-    $this->assertSame(45495, $performance_data->getStylesheetBytes());
+    $this->assertSame(41556, $performance_data->getStylesheetBytes());
   }
 
   /**
diff --git a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
index bdfe1ff949..4c1964afca 100644
--- a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
+++ b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
@@ -55,7 +55,7 @@ public function testAnonymous() {
     }, 'standardFrontPage');
     $this->assertNoJavaScript($performance_data);
     $this->assertSame(1, $performance_data->getStylesheetCount());
-    $this->assertSame(7434, $performance_data->getStylesheetBytes());
+    $this->assertSame(3495, $performance_data->getStylesheetBytes());
 
     $expected_queries = [
       'SELECT "base_table"."id" AS "id", "base_table"."path" AS "path", "base_table"."alias" AS "alias", "base_table"."langcode" AS "langcode" FROM "path_alias" "base_table" WHERE ("base_table"."status" = 1) AND ("base_table"."alias" LIKE "/node" ESCAPE ' . "'\\\\'" . ') AND ("base_table"."langcode" IN ("en", "und")) ORDER BY "base_table"."langcode" ASC, "base_table"."id" DESC',
@@ -111,7 +111,7 @@ public function testAnonymous() {
     }, 'standardNodePage');
     $this->assertNoJavaScript($performance_data);
     $this->assertSame(1, $performance_data->getStylesheetCount());
-    $this->assertSame(7159, $performance_data->getStylesheetBytes());
+    $this->assertSame(3220, $performance_data->getStylesheetBytes());
 
     $expected_queries = [
       'SELECT "base_table"."id" AS "id", "base_table"."path" AS "path", "base_table"."alias" AS "alias", "base_table"."langcode" AS "langcode" FROM "path_alias" "base_table" WHERE ("base_table"."status" = 1) AND ("base_table"."alias" LIKE "/node/1" ESCAPE ' . "'\\\\'" . ') AND ("base_table"."langcode" IN ("en", "und")) ORDER BY "base_table"."langcode" ASC, "base_table"."id" DESC',
@@ -146,7 +146,7 @@ public function testAnonymous() {
     }, 'standardUserPage');
     $this->assertNoJavaScript($performance_data);
     $this->assertSame(1, $performance_data->getStylesheetCount());
-    $this->assertSame(7159, $performance_data->getStylesheetBytes());
+    $this->assertSame(3220, $performance_data->getStylesheetBytes());
 
     $expected_queries = [
       'SELECT "base_table"."id" AS "id", "base_table"."path" AS "path", "base_table"."alias" AS "alias", "base_table"."langcode" AS "langcode" FROM "path_alias" "base_table" WHERE ("base_table"."status" = 1) AND ("base_table"."alias" LIKE "/user/2" ESCAPE ' . "'\\\\'" . ') AND ("base_table"."langcode" IN ("en", "und")) ORDER BY "base_table"."langcode" ASC, "base_table"."id" DESC',

As you can see with the standard profile it's a reduction of about 50% the filesize for the CSS aggregates, with Umami it's a reduction of more like 10%.

However this is issue is only the very easiest changes, if we finish 📌 Refactor system/base library Needs work I think we could be looking at more like a 20% reduction in CSS payload on Umami, which should be comparable with at least some real themes and sites.

Bumping this to major given the numbers.

Poking around in here I also found/opened 📌 Remove the views-align-* CSS classes Needs review .

Being optimistic and tagging this for release highlights, with some other issues we've got enough for a short performance improvements section.

Tried a lighthouse report on olivero to see how we score for unused CSS and how much this issue improves it. It's not much unfortunately, but then discovered that Olivero's own global styling library includes tabledrag.css rather than using libraries-extend. I've added a commit to change that too.

This change makes total sense and the CR reads very well.

As one of the few that have already hijacked/turned off some of the library assets in scope here, the organization seems much more closely aligned with a component driven system.

Is Rector capable of working with theme info files to update the paths automatically for upgrading users? A buttery smooth automated upgrade path is the only not-strictly-positive feedback here.

Is Rector capable of working with theme info files to update the paths automatically for upgrading users?

That's a good question and the answer is no because moved_files only exists since last week, but I opened a feature request against rector ✨ Support the new 'moved_files' key for updating libraries_overrides Active .

Test coverage in 📌 Record total css and js file size in performance tests Fixed landed so we can now update that test coverage to show the improvement here.

Slightly reviewed this one already when looking at 3436527.

Installed another theme besides claro or olivero and confirmed I'm still getting the css files removed from system.

Performance measure shows the benefit

I want to check that moving the ajax css doesn't cause problems on some ajax pages.

• Seems like we could move the details css file to the drupal.collapse library too no? it's a pretty big file in claro.
• Same for sticky-header.module.css that could go into drupal.tableheader maybe?

Too bad that we don't have a library definition for the status page already :)

@nod_ agreed with should move more things out, but 📌 Refactor system/base library Needs work , 📌 Audit the system/base libraries Active and 📌 Split tablesort.module.css out to its own library and attach it via tablesort Active are already open. I already worked on 📌 Record total css and js file size in performance tests Fixed and 📌 Add a deprecation support for library-overrides Active since this was last RTBC, and would prefer not to increase the scope here any more. Definitely planning to continue once this lands though.

makes sense +1 I'll get to it today

Added a note on 📌 Refactor system/base library Needs work to make sure those two get included.

Committed 86b8931 and pushed to 11.x. Thanks!

I think we need a dedicated 10.3.x MR. cherry pick is not clean and last time that happened, I broke tests on 10.2.x.

I was wrong, it worked :p

nod_ → closed merge request !7372

nod_ → closed merge request !7094

Automatically closed - issue fixed for 2 weeks with no activity.