FIX: Add order to outputted stylesheet link tags (PR #13735)

Theme and component stylesheets have so far been loaded without a specified order. This causes issues, especially on site with multiple themes that have multiple components each, styles from components would sometimes take precedence over the base theme and other times would override the base theme.

This PR adds an order when outputting theme stylesheets: component themes are displayed first, ordered alphabetically, and then the base theme is output last. This means that styles in the base theme will override styles with the same selector in a component. (In cases where you need a component’s style to override the base theme’s style, make sure the component uses a more specific selector.)

This PR also removes an extra layer of caching. We cache the full array of stylesheets for a specific target, and we also cached each stylesheet path. The PR removes the latter, the full array should be sufficient.


This looks good to me but could it possibly break existing themes where they depended on the previous order? I agree this order is better, for the record. I just want to figure out if any forums will be affected negatively.

Yes, it can potentially break existing sites. We currently don’t have a load order, though, so this would have caused issues sooner or later anyway…

I will test this on a few sites with many themes and components before merging. Also, the main risk here are sites that have styles in a component that are supposed to override styles in the parent theme.

Thanks I think it’s better to have an order than not, but we should be aware of the risk and have a plan of action when merging.

      let(:z_child_theme) do 
        Fabricate(:theme, component: true, name: "ze component").tap do |z|
          z.set_field(target: :desktop, name: "scss", value: ".child_desktop{.scss{color: red;}}")

We usually prefer do..end for multi-line blocks

      let(:child_remote) do 
        Fabricate(:theme, remote_theme: remote, component: true).tap do |t|
          t.set_field(target: :desktop, name: "scss", value: ".child_desktop{.scss{color: red;}}")
          stylesheets = stylesheets.sort_by do |s|
              s[:remote] ? 0 : 1, 
              s[:theme_id] == @theme_id ? 1 : 0, 

Minor but I find this easier to read as compared to all the code being in a single line.

We’re missing camelCase and snake_case for the variables here. Usually we default to snake_case for variable names in Ruby.

        remote_main_theme = Fabricate(:theme, remote_theme: remote2, name: "remote main").tap do |t|
          t.set_field(target: :desktop, name: "scss", value: ".el{color: red;}")

Some minor code style comments but the PR looks good to me otherwise. Regarding how we would deploy this, I recommend just putting the sorting behind a hidden site setting first and then roll it out slowly across our hosting.