DEV: Compile core and plugin stylesheets independently of themes (#13638)

DEV: Compile core and plugin stylesheets independently of themes (#13638)

Take 2 of DEV: Compile core and plugin stylesheets without themes by pmusaraj · Pull Request #13466 · discourse/discourse · GitHub.

Fixes a few issues with the original PR:

  • color definition stylesheet target now includes the theme id, to avoid themes set to use the default color scheme loading the same stylesheet
  • changes the internal cache key for color definition stylesheet to reset the pre-existing cache
diff --git a/app/assets/javascripts/discourse/app/initializers/live-development.js b/app/assets/javascripts/discourse/app/initializers/live-development.js
index e657240..bf92d0e 100644
--- a/app/assets/javascripts/discourse/app/initializers/live-development.js
+++ b/app/assets/javascripts/discourse/app/initializers/live-development.js
@@ -1,7 +1,6 @@
 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 {
@@ -51,7 +50,7 @@ export default {
     // Observe file changes
     messageBus.subscribe(
       "/file-change",
-      function (data) {
+      (data) => {
         if (Handlebars.compile && !Ember.TEMPLATES.empty) {
           // hbs notifications only happen in dev
           Ember.TEMPLATES.empty = Handlebars.compile("<div></div>");
@@ -60,20 +59,12 @@ export default {
           if (me === "refresh") {
             // Refresh if necessary
             document.location.reload(true);
-          } else {
-            $("link").each(function () {
-              if (me.hasOwnProperty("theme_id") && me.new_href) {
-                const target = $(this).data("target");
-                const themeId = $(this).data("theme-id");
-                if (
-                  target === me.target &&
-                  (!themeId || themeId === me.theme_id)
-                ) {
-                  refreshCSS(this, null, me.new_href);
-                }
-              } else if (this.href.match(me.name) && (me.hash || me.new_href)) {
-                refreshCSS(this, me.hash, me.new_href);
-              }
+          } else if (me.new_href && me.target) {
+            const link_target = me.theme_id
+              ? `[data-target="${me.target}"][data-theme-id="${me.theme_id}"]`
+              : `[data-target="${me.target}"]`;
+            document.querySelectorAll(`link${link_target}`).forEach((link) => {
+              this.refreshCSS(link, me.new_href);
             });
           }
         });
@@ -81,4 +72,23 @@ export default {
       session.mbLastFileChangeId
     );
   },
+
+  refreshCSS(node, newHref) {
+    if (node.dataset.reloading) {
+      clearTimeout(node.dataset.timeout);
+    }
+
+    node.dataset.reloading = true;
+
+    let reloaded = node.cloneNode(true);
+    reloaded.href = newHref;
+    node.insertAdjacentElement("afterend", reloaded);
+
+    let timeout = setTimeout(() => {
+      node.parentNode.removeChild(node);
+      reloaded.dataset.reloading = false;
+    }, 2000);
+
+    node.dataset.timeout = timeout;
+  },
 };
diff --git a/app/assets/javascripts/discourse/app/lib/theme-selector.js b/app/assets/javascripts/discourse/app/lib/theme-selector.js
index fdc4cb9..4d7c9a9 100644
--- a/app/assets/javascripts/discourse/app/lib/theme-selector.js
+++ b/app/assets/javascripts/discourse/app/lib/theme-selector.js
@@ -44,41 +44,6 @@ export function setLocalTheme(ids, themeSeq) {
   }
 }
 
-export function refreshCSS(node, hash, newHref) {
-  let $orig = $(node);
-
-  if ($orig.data("reloading")) {
-    clearTimeout($orig.data("timeout"));
-    $orig.data("copy").remove();
-  }
-
-  if (!$orig.data("orig")) {
-    $orig.data("orig", node.href);
-  }
-
-  $orig.data("reloading", true);
-
-  const orig = $(node).data("orig");
-
-  let reloaded = $orig.clone(true);
-  if (hash) {
-    reloaded[0].href =
-      orig + (orig.indexOf("?") >= 0 ? "&hash=" : "?hash=") + hash;
-  } else {
-    reloaded[0].href = newHref;
-  }
-
-  $orig.after(reloaded);
-
-  let timeout = setTimeout(() => {
-    $orig.remove();
-    reloaded.data("reloading", false);
-  }, 2000);
-
-  $orig.data("timeout", timeout);
-  $orig.data("copy", reloaded);
-}
-
 export function listThemes(site) {
   let themes = site.get("user_themes");
 
diff --git a/config/initializers/014-track-setting-changes.rb b/config/initializers/014-track-setting-changes.rb
index 1340d1d..ba916ab 100644
--- a/config/initializers/014-track-setting-changes.rb
+++ b/config/initializers/014-track-setting-changes.rb
@@ -27,7 +27,7 @@ DiscourseEvent.on(:site_setting_changed) do |name, old_value, new_value|
     end
   end
 
-  Stylesheet::Manager.clear_core_cache!(["desktop", "mobile"]) if [:base_font, :heading_font].include?(name)
+  Stylesheet::Manager.clear_color_scheme_cache! if [:base_font, :heading_font].include?(name)
 
   Report.clear_cache(:storage_stats) if [:backup_location, :s3_backup_bucket].include?(name)
 
diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb
index efc4718..72231f4 100644
--- a/lib/stylesheet/compiler.rb
+++ b/lib/stylesheet/compiler.rb
@@ -29,9 +29,6 @@ module Stylesheet
         file += File.read path
 
         case asset.to_s
-        when "desktop", "mobile"
-          file += importer.category_backgrounds
-          file += importer.font
         when "embed", "publish"
           file += importer.font
         when "wizard"
@@ -39,6 +36,8 @@ module Stylesheet
         when Stylesheet::Manager::COLOR_SCHEME_STYLESHEET
           file += importer.import_color_definitions
           file += importer.import_wcag_overrides
+          file += importer.category_backgrounds
+          file += importer.font
         end
       end
 
diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb
index e5c3f4c..ea7fe6b 100644
--- a/lib/stylesheet/manager.rb
+++ b/lib/stylesheet/manager.rb
@@ -38,58 +38,58 @@ class Stylesheet::Manager
   def self.color_scheme_cache_key(color_scheme, theme_id = nil)
     color_scheme_name = Slug.for(color_scheme.name) + color_scheme&.id.to_s
     theme_string = theme_id ? "_theme#{theme_id}" : ""
-    "#{COLOR_SCHEME_STYLESHEET}_#{color_scheme_name}#{theme_string}_#{Discourse.current_hostname}"
+    "#{COLOR_SCHEME_STYLESHEET}_#{color_scheme_name}_#{theme_string}_#{Discourse.current_hostname}"
   end
 
   def self.precompile_css
+    targets = [:desktop, :mobile, :desktop_rtl, :mobile_rtl, :admin, :wizard]
+    targets += Discourse.find_plugin_css_assets(include_disabled: true, mobile_view: true, desktop_view: true)
+
+    targets.each do |target|
+      $stderr.puts "precompile target: #{target}"
+
+      Stylesheet::Manager::Builder.new(target: target, manager: nil).compile(force: true)
+    end
+  end
+
+  def self.precompile_theme_css
     themes = Theme.where('user_selectable OR id = ?', SiteSetting.default_theme_id).pluck(:id, :color_scheme_id)
-    themes << nil
 
     color_schemes = ColorScheme.where(user_selectable: true).to_a
     color_schemes << ColorScheme.find_by(id: SiteSetting.default_dark_mode_color_scheme_id)
+    color_schemes << ColorScheme.base
     color_schemes = color_schemes.compact.uniq
 
-    targets = [:desktop, :mobile, :desktop_rtl, :mobile_rtl, :desktop_theme, :mobile_theme, :admin, :wizard]
-    targets += Discourse.find_plugin_css_assets(include_disabled: true, mobile_view: true, desktop_view: true)
+    targets = [:desktop_theme, :mobile_theme]
     compiled = Set.new
 
-    themes.each do |id, color_scheme_id|
-      theme_id = id || SiteSetting.default_theme_id
+    themes.each do |theme_id, color_scheme_id|
       manager = self.new(theme_id: theme_id)
 
       targets.each do |target|
-        if target =~ THEME_REGEX
-          next if theme_id == -1
-
-          scss_checker = ScssChecker.new(target, manager.theme_ids)
-
-          manager.load_themes(manager.theme_ids).each do |theme|
-            next if compiled.include?("#{target}_#{theme.id}")
+        next if theme_id == -1
 
-            builder = Stylesheet::Manager::Builder.new(
-              target: target, theme: theme, manager: manager
-            )
+        scss_checker = ScssChecker.new(target, manager.theme_ids)
 
-            next if theme.component && !scss_checker.has_scss(theme.id)
-            $stderr.puts "precompile target: #{target} #{theme.name}"

[... diff too long, it was truncated ...]

GitHub sha: 95b57943310d1ea8ef1808195f2250e1adbcf003

This commit appears in #13638 which was approved by markvanlan. It was merged by pmusaraj.