SDC stylesheets should be added in the "theme" aggregate group (as opposed to "default" group) to correct CSS source order

I have the same problem. For now, I have solved it with @layer, but it not has support in some browsers.

This is interesting. I see your point, but SDC's are meant to be self contained and any dependencies should be explicit.

My thought is if you had a theme level library called reset, you should explicitly declare the dependency within the SDC by doing something like

libraryOverrides:
  dependencies:
    - my_theme/reset

Thoughts on this? This might also be a question for a frontend framework manager.

Funny enough, running into this right now. The method in #5 doesn't work.

What I expect

If I load mytheme/my-library as a dependency to a component, I expect to see the CSS files in my-library loaded before the CSS files from my component

What I'm seeing:

Currently it gets added as the last file within the source order. If I change the group to base, it still loads after the the component in the source order.

Updating IS

I may not be able to fully reproduce this.

I have tried to reproduce the conditions in the open MR. I cannot have the dependant load after the component stylesheet.

I do see the reset.css file loaded after the component. However I can also see resize.module.css loaded before reset.css. That CSS is part of system.libraries.yml, and also declared with a component level.

Do you think that creating the component inside the sdc_test_theme theme instead of the sdc_test component will make a difference?

Test case patch with a simple component within Olivero. Note that within the SDC, I don't declare any library dependencies.

This is the source order of the CSS.

Here's a test case where I create a dependency on the olivero/feed library to try to force the SDC to load CSS at the end.

Fixed failures in #14.

It looks like this is only triggered if the library being depended on lives inside of a theme.

This patch should turn tests red.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

CC are unhappy.

Ouf, the branch creation really thew me off. It created my branch against 10.1.x.

Let's see if the patch is fixed now and we can get a red.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

Fixed failures in #19.

Added to cspell instead.

As it turns out, this is not an SDC thing. Libraries autogenerated for components are owned by the SDC module.

Below there is a failing test that shows how a library owned by a module, that depends on a library owned by a theme, will not produce the expected load order of the link tags.

Moving this issue to the render system and re-titling it. Issue summary needs an update with new findings.

Related but probably unrelated issue #3073053: Theme library override can fail in when you have multiple parent themes.

A workaround could be to move the libraries your components depend on to a module. That should work.

I am also interested in knowing if adding a dependency in your theme to the SDC module helps at all. If anyone wants to test and report, add the following to your theme's info file:

dependencies:
  - drupal/sdc

That might, or might not work. I wasn't able to add it to the test because the functional tests installer fails to create the ephemeral site.

I am not so sure adding a dependency to the theme works. There is a fairly fundamental mechanism in the sorting of the assets that puts more weight on themes than on modules. In the LibraryDiscoveryParser, the group gets set to either CSS_AGGREGATE_THEME or CSS_AGGREGATE_DEFAULT, depending on whether the extension is a theme or not. When it comes to sorting assets, that happens in the AssetResolver. This comes with its own sort method, which will prioritise the group as set earlier in the LibraryDiscovery parser.

I think I will try if I can alter the library info from SDC to make my theme the owner of the libraries. That should get them below the base styling I have defined. Not sure if this could serve as a fix for this issue inside SDC, or whether there is a more fundamental problem at hand here.

Post op Drupal.org m.b.t. component css die eerder wordt ingeladen dan basetheme css
https://www.drupal.org/project/drupal/issues/3374901

I've run into this issue as well. I've created a subtheme of Bootstrap5 and my problem is that my styling is loaded prior to the base styling, rendering my styling useless.

The problem resides in LibraryDiscoveryParser as eelkeblok already suggested. The stylesheets are loaded by the SDC module, which will always result it in them being placed in the CSS_AGGREGATE_DEFAULT group, higher up the chain then CSS_AGGREGATE_THEME.

I don't have a solution for this but I've came with this workaround for anyone who encountered this problem:

/**
 * Implements hook_library_info_alter().
 */
function theme_virtuel_library_info_alter(&$libraries, $extension) {
  if ($extension == 'sdc') {
    foreach ($libraries as $key => $library) {
      $libraryName = explode('--', $key);

      if (empty($libraryName[1])) {
        continue;
      }
      $library['dependencies'][] = $libraryName[0] . '/' . $libraryName[1];
      $libraries[$key] = $library;
    }
  }
}

/**
 * Implements hook_library_info_build().
 */
function theme_library_info_build() {
  /** @var \Drupal\Core\Asset\LibraryDiscoveryCollector $libraryDiscoveryCollector */
  $libraryDiscoveryCollector = \Drupal::service('library.discovery.collector');
  $theme = \Drupal::theme()->getActiveTheme();

  $libraries = [];

  // Duplicate SDC libraries as dependencies.
  $sdc = $libraryDiscoveryCollector->get('sdc');

  foreach ($sdc as $key => $library) {
    $libraryName = explode('--', $key);

    // Only add items that have a component.
    if (empty($libraryName[1])) {
      continue;
    }

    $cssFiles = [];
    foreach ($library['css'] as $cssKey => $cssDefinition) {
      $start = strpos($cssDefinition['data'], $theme->getName()) + strlen($theme->getName());
      $cssFiles[] = base_path() . $theme->getPath() . substr($cssDefinition['data'], $start);
    }

    // Remove asset from SDC, otherwhise it will be loaded twice.
    unset($library['css']);

    $sdc[$key] = $library;

    // Create library based on SDC definition.
    foreach ($cssFiles as $cssFile) {
      $libraries[$libraryName[1]]['css']['component'][$cssFile] = [];
    }
  }

  $libraryDiscoveryCollector->set('sdc', $sdc);

  return $libraries;
}

I'm continuously running into this. Any progress? I'd like to help but not sure where to start.

@mherchel it might be worth double checking in 10.4/11.1 because #3467860: Ensure consistent ordering when calculating library asset order changed library ordering to more strictly follow dependencies.

#1945262: Introduce "before" and "after" for conditional ordering in library definitions is open for getting rid of custom weights more generally. #1924522: Remove separate CSS_AGGREGATE_THEME aggregate file is open for getting rid of CSS_AGGREGATE_THEME.

Probably getting rid of CSS_AGGREGATE_THEME is the real blocker here, and that in turn is blocked on #2880237: [meta] Refactor system/base library and friends which could use some help but was getting close.

Thanks for the response.

I re-checked 11.1.x and the problem is still occuring :(

I will look at those other issues.

@mherchel

Have you tried the workaround mentioned in #29. I know it's not the long-term fix, but want to know more out of curiosity.

I made a small module called SDC CSS Relocator to help with a similar issue that I was seeing. If there's anything that would make it more helpful while this issue awaits resolution, feel free to comment on that project page.

@sethhill

I think this issue is closely related to this one LibraryDiscoveryParser overwrites theme CSS group ordering.

It feels like something we'd have to fix in \Drupal\Core\Asset\LibraryDependencyResolver::doGetDependencies. That if a library from a theme is resolved as a dependency. The CSS files should be iterated over and the group changed to CSS_AGGREGATE_DEFAULT.

So my dream in doGetDependencies doesn't work. But any adjustments there do not matter because the code path ends up at \Drupal\Core\Asset\AssetResolver::getCssAssets.

    foreach ($libraries_to_load as $library) {
      [$extension, $name] = explode('/', $library, 2);
      $definition = $this->libraryDiscovery->getLibraryByName($extension, $name);
      foreach ($definition['css'] as $options) {

The library definition is fetched here with the group CSS_AGGREGATE_THEME.

And to make sure I'm understanding this correctly, because I've flipflopped in my head a few times: if a theme's library is a dependency for a component, that theme's library should not be in CSS_AGGREGATE_THEME to ensure proper ordering.

I opened https://git.drupalcode.org/project/drupal/-/merge_requests/11633 to prove where a fix _could_ go but I have no idea how to solve it. Moving to the asset library system, as I think it belongs there. The problem is how theme libraries are treated special and break the idea of being a dependency

The other solution and proposal: all SDCs should have their libraries in the theme aggregation group since they're theme level anyway

Chatted w/ laurii about #45 and got a thumbs up with having SDCs grouped into the theme aggregate versus default

Chatted w/ laurii about #45 and got a thumbs up with having SDCs grouped into the theme aggregate versus default

Per #36 we can't make SDCs have a specific group without #3046636: LibraryDiscoveryParser overwrites theme CSS group ordering

Okay. This will need a change record if it goes through along with the issue summary update and a title update. It also brings in #3046636: LibraryDiscoveryParser overwrites theme CSS group ordering so we may need to fix that first or decide to roll it in here.

SDCs libraries are now aggregated at the theme group

Updating title.

Updating issue summary.

Assigning credit (everyone gets credit)

Draft CR at https://www.drupal.org/node/3515838. Still needs screenshots, which I will add as I test this out.

So a bunch of performance tests bumped in their stylesheet counts, which I think is a consequence of the different group. It is documented as a reason in #1924522: Remove separate CSS_AGGREGATE_THEME aggregate file. Some of the jumps seem high, I'm not sure if this is expected or can be mitigated without #1924522

Screenshots showing that the issue is fixed. I'm going to double check this works as expected when aggregation is enabled.

Before

After

Confirmed that this is fixed when aggregation is enabled. I tested this by modifying Olivero's CSS in two places:

1. In the teaser SDC component.
2. In the theme's base CSS.

The SDC styles take get injected after, which takes precedent, which is the correct behavior! YAY!!!

Here's a picture of Matt doing the work!! :D

Image showing issue is fixed with aggregation

We made a change to avoid doing #3046636: LibraryDiscoveryParser overwrites theme CSS group ordering and exposing group as an API.

Reviewing now!

Re-tested. Confirmed new method outputs the same. Not including screenshots because they're exactly the same as in #59 and #62

Committed 73613e5 and pushed to 11.x. Thanks!

Whoohoo!! Thanks Matt and Lauri!!!

I'm hoping that this can be backported to 10.5.x. As it stands now if I want to make a theme that supports both D10 and D11, I still have to work around this bug by adding additional specificity onto my SDC CSS selectors. :D

I think this is reasonable to backport to 10.5.x so that behaviour is consistent between the two major branches, although it will need a backport MR at least for the performance test changes due to merge conflicts. Moving to 'to be ported' for that.

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

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

Committed/pushed the backport MR to 10.6.x and 10.5.x, thanks!