DEV: Use separate files for theme component stylesheets (take 2) (#12225)

This switches to outputting a separate file for each theme component CSS
asset. We have separate CSS plugin files, separate JS files
(for plugins/themes/components), it makes sense to do the same for
component CSS assets.

Benefits:
- easier debugging
- fixes a regression with theme component sourcemaps
- changes to theme components are updated individually

With HTTP/2, there is also no performance downside to having additional
files in the initial request.

app/assets/javascripts/discourse/app/initializers/live-development.js

@@ -1,7 +1,7 @@
-import { currentThemeIds, refreshCSS } from "discourse/lib/theme-selector";
 import DiscourseURL from "discourse/lib/url";
 import Handlebars from "handlebars";
 import { isDevelopment } from "discourse-common/config/environment";
+import { refreshCSS } from "discourse/lib/theme-selector";
 
 //  Use the message bus for live reloading of components for faster development.
 export default {
@@ -63,13 +63,11 @@ export default {
           // Refresh if necessary
           document.location.reload(true);
         } else {
-          const themeIds = currentThemeIds();
           $("link").each(function () {
             if (me.hasOwnProperty("theme_id") && me.new_href) {
               const target = $(this).data("target");
               const themeId = $(this).data("theme-id");
               if (
-                themeIds.indexOf(me.theme_id) !== -1 &&
                 target === me.target &&
                 (!themeId || themeId === me.theme_id)
               ) {

app/models/theme.rb

@@ -315,8 +315,7 @@ class Theme < ActiveRecord::Base
     if all_themes
       message = theme_ids.map { |id| refresh_message_for_targets(targets, id) }.flatten
     else
-      parent_ids = Theme.where(id: theme_ids).joins(:parent_themes).pluck(:parent_theme_id).uniq
-      message = refresh_message_for_targets(targets, theme_ids | parent_ids).flatten
+      message = refresh_message_for_targets(targets, theme_ids).flatten
     end
 
     MessageBus.publish('/file-change', message)
@@ -372,7 +371,7 @@ class Theme < ActiveRecord::Base
   end
 
   def list_baked_fields(target, name)
-    theme_ids = Theme.transform_ids([id])
+    theme_ids = Theme.transform_ids([id], extend: false)
     self.class.list_baked_fields(theme_ids, target, name)
   end
 

lib/stylesheet/importer.rb

@@ -194,18 +194,13 @@ module Stylesheet
       fields.map do |field|
         value = field.value
         if value.present?
-          filename = "theme_#{field.theme.id}/#{field.target_name}-#{field.name}-#{field.theme.name.parameterize}.scss"
           contents << <<~COMMENT
           // Theme: #{field.theme.name}
           // Target: #{field.target_name} #{field.name}
           // Last Edited: #{field.updated_at}
           COMMENT
 
-          if field.theme_id == theme.id
-            contents << value
-          else
-            contents << field.compiled_css(prepended_scss)
-          end
+          contents << value
         end
 
       end

lib/stylesheet/manager.rb

@@ -58,7 +58,9 @@ class Stylesheet::Manager
 
     theme_ids = [theme_ids] unless Array === theme_ids
     theme_ids = [theme_ids.first] unless target =~ THEME_REGEX
-    theme_ids = Theme.transform_ids(theme_ids, extend: false)
+    include_components = !!(target =~ THEME_REGEX)
+
+    theme_ids = Theme.transform_ids(theme_ids, extend: include_components)
 
     current_hostname = Discourse.current_hostname
 

spec/components/stylesheet/manager_spec.rb

@@ -22,55 +22,96 @@ describe Stylesheet::Manager do
     expect(link).not_to eq("")
   end
 
-  it 'can correctly compile theme css' do
-    theme = Fabricate(:theme)
+  context "themes with components" do
+    let(:child_theme) { Fabricate(:theme, component: true).tap { |c|
+      c.set_field(target: :common, name: "scss", value: ".child_common{.scss{color: red;}}")
+      c.set_field(target: :desktop, name: "scss", value: ".child_desktop{.scss{color: red;}}")
+      c.set_field(target: :mobile, name: "scss", value: ".child_mobile{.scss{color: red;}}")
+      c.set_field(target: :common, name: "embedded_scss", value: ".child_embedded{.scss{color: red;}}")
+      c.save!
+    }}
 
-    theme.set_field(target: :common, name: "scss", value: ".common{.scss{color: red;}}")
-    theme.set_field(target: :desktop, name: "scss", value: ".desktop{.scss{color: red;}}")
-    theme.set_field(target: :mobile, name: "scss", value: ".mobile{.scss{color: red;}}")
-    theme.set_field(target: :common, name: "embedded_scss", value: ".embedded{.scss{color: red;}}")
+    let(:theme) { Fabricate(:theme).tap { |t|
+      t.set_field(target: :common, name: "scss", value: ".common{.scss{color: red;}}")
+      t.set_field(target: :desktop, name: "scss", value: ".desktop{.scss{color: red;}}")
+      t.set_field(target: :mobile, name: "scss", value: ".mobile{.scss{color: red;}}")
+      t.set_field(target: :common, name: "embedded_scss", value: ".embedded{.scss{color: red;}}")
+      t.save!
 
-    theme.save!
+      t.add_relative_theme!(:child, child_theme)
+    }}
 
-    child_theme = Fabricate(:theme, component: true)
+    it 'can correctly compile theme css' do
+      old_links = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id)
 
-    child_theme.set_field(target: :common, name: "scss", value: ".child_common{.scss{color: red;}}")
-    child_theme.set_field(target: :desktop, name: "scss", value: ".child_desktop{.scss{color: red;}}")
-    child_theme.set_field(target: :mobile, name: "scss", value: ".child_mobile{.scss{color: red;}}")
-    child_theme.set_field(target: :common, name: "embedded_scss", value: ".child_embedded{.scss{color: red;}}")
-    child_theme.save!
+      manager = Stylesheet::Manager.new(:desktop_theme, theme.id)
+      manager.compile(force: true)
 
-    theme.add_relative_theme!(:child, child_theme)
+      css = File.read(manager.stylesheet_fullpath)
+      _source_map = File.read(manager.source_map_fullpath)
 
-    old_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id)
+      expect(css).to match(/\.common/)
+      expect(css).to match(/\.desktop/)
 
-    manager = Stylesheet::Manager.new(:desktop_theme, theme.id)
-    manager.compile(force: true)
+      # child theme CSS is no longer bundled with main theme
+      expect(css).not_to match(/child_common/)
+      expect(css).not_to match(/child_desktop/)
 
-    css = File.read(manager.stylesheet_fullpath)
-    _source_map = File.read(manager.source_map_fullpath)
+      child_theme_manager = Stylesheet::Manager.new(:desktop_theme, child_theme.id)
+      child_theme_manager.compile(force: true)
 
-    expect(css).to match(/child_common/)
-    expect(css).to match(/child_desktop/)
-    expect(css).to match(/\.common/)
-    expect(css).to match(/\.desktop/)
+      child_css = File.read(child_theme_manager.stylesheet_fullpath)
+      _child_source_map = File.read(child_theme_manager.source_map_fullpath)
 
-    child_theme.set_field(target: :desktop, name: :scss, value: ".nothing{color: green;}")
-    child_theme.save!
+      expect(child_css).to match(/child_common/)
+      expect(child_css).to match(/child_desktop/)
 
-    new_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id)
+      child_theme.set_field(target: :desktop, name: :scss, value: ".nothing{color: green;}")
+      child_theme.save!
 
-    expect(new_link).not_to eq(old_link)
+      new_links = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id)
 
-    # our theme better have a name with the theme_id as part of it
-    expect(new_link).to include("/stylesheets/desktop_theme_#{theme.id}_")
+      expect(new_links).not_to eq(old_links)
 
-    manager = Stylesheet::Manager.new(:embedded_theme, theme.id)
-    manager.compile(force: true)
+      # our theme better have a name with the theme_id as part of it
+      expect(new_links).to include("/stylesheets/desktop_theme_#{theme.id}_")
+      expect(new_links).to include("/stylesheets/desktop_theme_#{child_theme.id}_")
+    end
 
-    css = File.read(manager.stylesheet_fullpath)
-    expect(css).to match(/\.embedded/)
-    expect(css).to match(/\.child_embedded/)
+    it 'can correctly compile embedded theme css' do
+      manager = Stylesheet::Manager.new(:embedded_theme, theme.id)
+      manager.compile(force: true)
+
+      css = File.read(manager.stylesheet_fullpath)
+      expect(css).to match(/\.embedded/)
+      expect(css).not_to match(/\.child_embedded/)
+
+      child_theme_manager = Stylesheet::Manager.new(:embedded_theme, child_theme.id)
+      child_theme_manager.compile(force: true)
+
+      css = File.read(child_theme_manager.stylesheet_fullpath)
+      expect(css).to match(/\.child_embedded/)
+    end
+
+    it 'includes both parent and child theme assets' do
+      hrefs = Stylesheet::Manager.stylesheet_details(:desktop_theme, 'all', [theme.id])
+      expect(hrefs.count).to eq(2)
+      expect(hrefs[0][:theme_id]).to eq(theme.id)
+      expect(hrefs[1][:theme_id]).to eq(child_theme.id)
+
+      hrefs = Stylesheet::Manager.stylesheet_details(:embedded_theme, 'all', [theme.id])
+      expect(hrefs.count).to eq(2)
+      expect(hrefs[0][:theme_id]).to eq(theme.id)
+      expect(hrefs[1][:theme_id]).to eq(child_theme.id)
+    end
+
+    it 'does not output multiple assets for non-themes' do
+      hrefs = Stylesheet::Manager.stylesheet_details(:admin, 'all', [theme.id])
+      expect(hrefs.count).to eq(1)
+
+      hrefs = Stylesheet::Manager.stylesheet_details(:mobile, 'all', [theme.id])
+      expect(hrefs.count).to eq(1)
+    end
   end
 
   describe 'digest' do

spec/models/theme_spec.rb

@@ -795,24 +795,13 @@ HTML
       expect(css).to include("p{color:blue}")
     end
 
-    it "works for child themes and includes child theme SCSS in parent theme" do
+    it "works for child themes" do
       child_theme.set_field(target: :common, name: :scss, value: '@import "my_files/moremagic"')
       child_theme.save!
 
       manager = Stylesheet::Manager.new(:desktop_theme, child_theme.id)
       css, _map = manager.compile(force: true)
       expect(css).to include("body{background:green}")
-
-      parent_css, _parent_map = compiler
-      expect(parent_css).to include("body{background:green}")
-    end
-
-    it "does not fail if child theme has SCSS errors" do
-      child_theme.set_field(target: :common, name: :scss, value: 'p {color: $missing_var;}')
-      child_theme.save!
-
-      parent_css, _parent_map = compiler
-      expect(parent_css).to include("sourceMappingURL")
     end
   end