Various CSS files in libraries split out from system/base, such as progress.module.css, resize.module.css, tablesort, fieldgroup, item-list and more have an explicit negative weight. That almost certainly isn't required anymore, especially now when they are their own library that others depend on, so library dependencie should take care of this.
With #3565258: Support library-specific aggregates, this causes an unfortunate issue which puts those files, if required on a page, into their own group ahead of system/base, making it a tiny and not-well-aggregated separate asset request.
Remove all weight definitions from CSS libraries in core.libraries.yml, mostly those moved from system.base, but also ajax-progress.module.css for example.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
berdirUnsure about the scope and impact here. This obviously overlaps with #1945262: Introduce "before" and "after" for conditional ordering in library definitions, so I'm wondering if we can really split off, doing a quick test.
I think it should work for a library like core/drupal.progress, that's loaded as a dependency of another. However, where it gets tricky is with core/normalize for example. claro/umami/.. load that as a global library, and not a dependency of their own global theming library. That means that normalize.css is not actually loaded first, so it's unclear how that is supposed to work, also with before/after dependencies, since there isn't anything depending on it? We might just need to keep that for edge cases like that?
Comment #4
catchThere's a different issue (will link later) about themes not being able to add their own reset styles because it gets loaded after module CSS. So I think anyway we might want a new CSS 'reset' group and put normalize in there.
Comment #5
catchFound the issue again:
#3046636: LibraryDiscoveryParser overwrites theme CSS group ordering
We should leave normalize.css alone here and tackle that in the before/after issue or another similar one.
Comment #6
berdirYes, leaving alone normalize.css sounds like a good plan. No test fails/performance changes with this yet. What I'll do with this I think is a diff of the non-aggregated CSS files on umami and claro or so, but I need to find a page that adds at least some of them, because I don't think umami does, at least not on the pages we tested.
Comment #7
berdirTesting the impact of this might be tricky, both positive and negative.
On it's own, with aggregation off, there's a shift in order of CSS files with this, but it's pretty contained. Diffing it is tricky, because the query strings change on every cache clear, so I put the HTML of node/add/article into a file, cut of the query string for all the claro CSS classes and then compared that before/after this change.
That gave me this diff:
so both progress and resize css files are pushed a bit down, not significantly.
Combined with #3565258: Support library-specific aggregates, there's no change yet in number of aggregate files. The first is normalize + jquery.ui + drupal.dialog + drupal.autocomplete, so essentially all those that have explicit negative weights. Then a separate one for system.base and then everything else, which interestingly also again has autocomplete and dialog library files, for those without an explicit weight.
Came up with this one-liner to summarize the libraries in a given css group:
implode(',', array_unique(array_column($css_groups[0]['items'], 'library'))), which gives me:I didn't look into the weights of jquery, dialog and autocomplete yet ,but as long normalize is up there, there's not much to gain with this for claro. It needs a theme that doesn't use normalize and a scenario where you can remove all libraries with explicit weights with this. Which we have.
Comment #8
dcam commentedI haven't done a complete review of this issue yet, but the MR needs a rebase.
Comment #10
berdirRebased and created an 11.x version off the previous state. main and 11.x are drifting apart pretty significantly, going to be quite a bit of work to do backports.
Comment #11
dcam commentedI verified @berdir's results for the unaggregated, ungrouped CSS files. The diff can be seen in the following screenshot.
The remaining weights for CSS files include:
The MR changes are simple, only removing weights from CSS files in libraries. I think this one is OK to go.
Comment #12
catchThis looks great and will help reduce the scope of #1945262: Introduce "before" and "after" for conditional ordering in library definitions. Committed/pushed to main, thanks!
Doesn't apply to 11.x, so moving there for backport.
Comment #15
berdir@catch: There's already a 11.x MR open, based on the previous state before the conflicts with the BC removals in main. it had fails, which I assume were random, can't see it on 11.x, rebased it to trigger a retest.
Comment #18
catchSorry for missing the existence of the 11.x MR. Committed/pushed to 11.x, thanks!
Comment #20
joelpittetThis is awesome! Thanks for doing this! I’ve always found that an itch I meant to scratch!
Thanks berdir
#ex-theme-system-maintainer