DEV: More refactoring of SCSS importers (#12143)

app/assets/stylesheets/common/foundation/variables.scss

@@ -77,7 +77,6 @@ $line-height-large: 1.4; // Normal or small text
 // These files don't actually exist. They're injected by Stylesheet::Compiler.
 // --------------------------------------------------
 
-@import "theme_colors";
 @import "plugins_variables";
 @import "common/foundation/math";
 

app/models/theme_field.rb

@@ -346,21 +346,19 @@ class ThemeField < ActiveRecord::Base
     end
   end
 
-  def compile_scss
-    scss = <<~SCSS
-      @import "common/foundation/variables"; @import "common/foundation/mixins"; #{self.theme.scss_variables.to_s} #{self.value}
-    SCSS
+  def compile_scss(prepended_scss = nil)
+    prepended_scss ||= Stylesheet::Importer.new({}).prepended_scss
 
-    Stylesheet::Compiler.compile(scss,
+    Stylesheet::Compiler.compile("#{prepended_scss} #{self.theme.scss_variables.to_s} #{self.value}",
       "#{Theme.targets[self.target_id]}.scss",
       theme: self.theme,
       load_paths: self.theme.scss_load_paths
     )
   end
 
-  def compiled_css
+  def compiled_css(prepended_scss)
     css, _source_map = begin
-      compile_scss
+      compile_scss(prepended_scss)
     rescue SassC::SyntaxError => e
       # We don't want to raise a blocking error here
       # admin theme editor or discourse_theme CLI will show it nonetheless

lib/stylesheet/compiler.rb

@@ -9,12 +9,13 @@ module Stylesheet
   class Compiler
 
     def self.compile_asset(asset, options = {})
-      file = "@import \"common/foundation/variables\"; @import \"common/foundation/mixins\";"
+      importer = Importer.new(options)
+      file = importer.prepended_scss
 
       if Importer::THEME_TARGETS.include?(asset.to_s)
         filename = "theme_#{options[:theme_id]}.scss"
         file += options[:theme_variables].to_s
-        file += Importer.new({ theme_id: options[:theme_id] }).theme_import(asset)
+        file += importer.theme_import(asset)
       elsif Importer.special_imports[asset.to_s]
         filename = "theme_#{options[:theme_id]}.scss"
         file += " @import \"#{asset}\";"
@@ -24,13 +25,12 @@ module Stylesheet
         file += File.read path
 
         if asset.to_s == Stylesheet::Manager::COLOR_SCHEME_STYLESHEET
-          file += Stylesheet::Importer.import_color_definitions(options[:theme_id])
-          file += Stylesheet::Importer.import_wcag_overrides(options[:color_scheme_id])
+          file += importer.import_color_definitions
+          file += importer.import_wcag_overrides
         end
       end
 
       compile(file, filename, options)
-
     end
 
     def self.compile(stylesheet, filename, options = {})

lib/stylesheet/importer.rb

@@ -95,25 +95,6 @@ module Stylesheet
         import_files(DiscoursePluginRegistry.sass_variables)
       end
 
-      register_import "theme_colors" do
-        contents = +""
-        if @color_scheme_id
-          colors = begin
-            ColorScheme.find(@color_scheme_id).resolved_colors
-          rescue
-            ColorScheme.base_colors
-          end
-        else
-          colors = (@theme_id && theme.color_scheme) ? theme.color_scheme.resolved_colors : ColorScheme.base_colors
-        end
-
-        colors.each do |n, hex|
-          contents << "$#{n}: ##{hex} !default;\n"
-        end
-
-        Import.new("theme_colors.scss", source: contents)
-      end
-
       register_import "category_backgrounds" do
         contents = +""
         Category.where('uploaded_background_id IS NOT NULL').each do |c|
@@ -127,7 +108,7 @@ module Stylesheet
 
     register_imports!
 
-    def self.import_color_definitions(theme_id)
+    def import_color_definitions
       contents = +""
       DiscoursePluginRegistry.color_definition_stylesheets.each do |name, path|
         contents << "// Color definitions from #{name}\n\n"
@@ -135,7 +116,7 @@ module Stylesheet
         contents << "\n\n"
       end
 
-      theme_id ||= SiteSetting.default_theme_id
+      theme_id = @theme_id || SiteSetting.default_theme_id
       resolved_ids = Theme.transform_ids([theme_id])
 
       if resolved_ids
@@ -147,7 +128,7 @@ module Stylesheet
           if field.theme_id == theme.id
             contents << field.value
           else
-            contents << field.compiled_css
+            contents << field.compiled_css(prepended_scss)
           end
           contents << "\n\n"
         end
@@ -155,13 +136,36 @@ module Stylesheet
       contents
     end
 
-    def self.import_wcag_overrides(color_scheme_id)
-      if color_scheme_id && ColorScheme.find_by_id(color_scheme_id)&.is_wcag?
+    def import_wcag_overrides
+      if @color_scheme_id && ColorScheme.find_by_id(@color_scheme_id)&.is_wcag?
         return "@import \"wcag\";"
       end
       ""
     end
 
+    def color_variables
+      contents = +""
+      if @color_scheme_id
+        colors = begin
+          ColorScheme.find(@color_scheme_id).resolved_colors
+        rescue
+          ColorScheme.base_colors
+        end
+      else
+        colors = (@theme_id && theme.color_scheme) ? theme.color_scheme.resolved_colors : ColorScheme.base_colors
+      end
+
+      colors.each do |n, hex|
+        contents << "$#{n}: ##{hex} !default; "
+      end
+
+      contents
+    end
+
+    def prepended_scss
+      "#{color_variables} @import \"common/foundation/variables\"; @import \"common/foundation/mixins\"; "
+    end
+
     def initialize(options)
       @theme = options[:theme]
       @theme_id = options[:theme_id]
@@ -204,7 +208,7 @@ module Stylesheet
           if field.theme_id == theme.id
             contents << value
           else
-            contents << field.compiled_css
+            contents << field.compiled_css(prepended_scss)
           end
         end
 

spec/components/stylesheet/importer_spec.rb

@@ -80,7 +80,7 @@ describe Stylesheet::Importer do
     }}
 
     it "should include color definitions in the theme" do
-      styles = Stylesheet::Importer.import_color_definitions(theme.id)
+      styles = Stylesheet::Importer.new({ theme_id: theme.id }).import_color_definitions
       expect(styles).to include(scss)
     end
 
@@ -88,14 +88,14 @@ describe Stylesheet::Importer do
       theme.add_relative_theme!(:child, child)
       theme.save!
 
-      styles = Stylesheet::Importer.import_color_definitions(theme.id)
+      styles = Stylesheet::Importer.new({ theme_id: theme.id }).import_color_definitions
       expect(styles).to include(scss_child)
       expect(styles).to include("Color definitions from Child Theme")
     end
 
     it "should include default theme color definitions" do
       SiteSetting.default_theme_id = theme.id
-      styles = Stylesheet::Importer.import_color_definitions(nil)
+      styles = Stylesheet::Importer.new({}).import_color_definitions
       expect(styles).to include(scss)
     end
   end
@@ -103,12 +103,12 @@ describe Stylesheet::Importer do
   context "#import_wcag_overrides" do
     it "should do nothing on a regular scheme" do
       scheme = ColorScheme.create_from_base(name: 'Regular')
-      expect(Stylesheet::Importer.import_wcag_overrides(scheme.id)).to eq("")
+      expect(Stylesheet::Importer.new({ color_scheme_id: scheme.id }).import_wcag_overrides).to eq("")
     end
 
     it "should include WCAG overrides for WCAG based scheme" do
       scheme = ColorScheme.create_from_base(name: 'WCAG New', base_scheme_id: "WCAG Dark")
-      expect(Stylesheet::Importer.import_wcag_overrides(scheme.id)).to eq("@import \"wcag\";")
+      expect(Stylesheet::Importer.new({ color_scheme_id: scheme.id }).import_wcag_overrides).to eq("@import \"wcag\";")
     end
   end
 end

spec/components/stylesheet/manager_spec.rb

@@ -288,14 +288,38 @@ describe Stylesheet::Manager do
         t.set_field(target: :common, name: "color_definitions", value: ':root {--special: rebeccapurple;}')
         t.save!
       }}
+      let(:scss_child) { ':root {--child-definition: #{dark-light-choose(#c00, #fff)};}' }
+      let(:child) { Fabricate(:theme, component: true, name: "Child Theme").tap { |t|
+        t.set_field(target: :common, name: "color_definitions", value: scss_child)
+        t.save!
+      }}
 
       let(:scheme) { ColorScheme.base }
+      let(:dark_scheme) { ColorScheme.create_from_base(name: 'Dark', base_scheme_id: 'Dark') }
 
       it "includes theme color definitions in color scheme" do
         stylesheet = Stylesheet::Manager.new(:color_definitions, theme.id, scheme).compile(force: true)
         expect(stylesheet).to include("--special: rebeccapurple")
       end
 
+      it "includes child color definitions in color schemes" do
+        theme.add_relative_theme!(:child, child)
+        theme.save!
+        stylesheet = Stylesheet::Manager.new(:color_definitions, theme.id, scheme).compile(force: true)
+
+        expect(stylesheet).to include("--special: rebeccapurple")
+        expect(stylesheet).to include("--child-definition: #c00")
+      end
+
+      it "respects selected color scheme in child color definitions" do
+        theme.add_relative_theme!(:child, child)
+        theme.save!
+
+        stylesheet = Stylesheet::Manager.new(:color_definitions, theme.id, dark_scheme).compile(force: true)
+        expect(stylesheet).to include("--special: rebeccapurple")
+        expect(stylesheet).to include("--child-definition: #fff")
+      end
+
       it "fails gracefully for broken SCSS" do
         scss = "$test: $missing-var;"
         theme.set_field(target: :common, name: "color_definitions", value: scss)