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.

GitHub

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;}}")
          z.save!
        end
      end

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;}}")
          t.save!
        end
      end
          stylesheets = stylesheets.sort_by do |s|
            [
              s[:remote] ? 0 : 1, 
              s[:theme_id] == @theme_id ? 1 : 0, 
              s[:theme_name]
            ]
          end

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;}")
          t.save!
        end

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.