From 99d259d39bdaade6d20951c84def890dc91246a5 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 21 Jun 2021 20:27:11 -0400 Subject: [PATCH] Precompile core stylesheets independently of themes --- lib/stylesheet/manager.rb | 63 +++++++++++++++------- lib/tasks/assets.rake | 3 +- spec/components/stylesheet/manager_spec.rb | 34 ++++++------ 3 files changed, 63 insertions(+), 37 deletions(-) diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index e8713943c52..b3749e48939 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -42,6 +42,17 @@ class Stylesheet::Manager 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, :name, :color_scheme_id) themes << nil @@ -49,8 +60,7 @@ class Stylesheet::Manager color_schemes << ColorScheme.find_by(id: SiteSetting.default_dark_mode_color_scheme_id) 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] themes.each do |id, name, color_scheme_id| theme_id = id || SiteSetting.default_theme_id @@ -193,15 +203,19 @@ class Stylesheet::Manager def stylesheet_details(target = :desktop, media = 'all') target = target.to_sym current_hostname = Discourse.current_hostname + is_theme_target = !!(target.to_s =~ THEME_REGEX) + + array_cache_key = is_theme_target ? + "array_themes_#{@theme_ids.join(",")}_#{target}_#{current_hostname}" : + "array_#{target}_#{current_hostname}" - array_cache_key = "array_themes_#{@theme_ids.join(",")}_#{target}_#{current_hostname}" stylesheets = cache[array_cache_key] return stylesheets if stylesheets.present? @@lock.synchronize do stylesheets = [] stale_theme_ids = [] - theme_ids = target.to_s =~ THEME_REGEX ? @theme_ids : [@theme_id] + theme_ids = is_theme_target ? @theme_ids : [nil] theme_ids.each do |theme_id| cache_key = "path_#{target}_#{theme_id}_#{current_hostname}" @@ -219,25 +233,36 @@ class Stylesheet::Manager scss_checker = ScssChecker.new(target, stale_theme_ids) - themes = @theme_id.blank? ? [nil] : load_themes(stale_theme_ids) + if is_theme_target + themes = load_themes(stale_theme_ids) - themes.each do |theme| - theme_id = theme&.id - data = { target: target, theme_id: theme_id } - builder = Builder.new(target: target, theme: theme, manager: self) - is_theme = builder.is_theme? - has_theme = builder.theme.present? + themes.each do |theme| + theme_id = theme&.id + data = { target: target, theme_id: theme_id } + builder = Builder.new(target: target, theme: theme, manager: self) + is_theme = builder.is_theme? + has_theme = builder.theme.present? - if is_theme && !has_theme - next - else - next if is_theme && builder.theme&.component && !scss_checker.has_scss(theme_id) - builder.compile unless File.exists?(builder.stylesheet_fullpath) - href = builder.stylesheet_path(current_hostname) - cache.defer_set("path_#{target}_#{theme_id}_#{current_hostname}", href) + if is_theme && !has_theme + next + else + next if is_theme && builder.theme&.component && !scss_checker.has_scss(theme_id) + builder.compile unless File.exists?(builder.stylesheet_fullpath) + href = builder.stylesheet_path(current_hostname) + cache.defer_set("path_#{target}_#{theme_id}_#{current_hostname}", href) + end + + data[:new_href] = href + stylesheets << data end + else + builder = Builder.new(target: target, manager: self) + builder.compile unless File.exists?(builder.stylesheet_fullpath) + href = builder.stylesheet_path(current_hostname) - data[:new_href] = href + cache.defer_set("path_#{target}__#{current_hostname}", href) + + data = { target: target, new_href: href } stylesheets << data end diff --git a/lib/tasks/assets.rake b/lib/tasks/assets.rake index 85debe82334..f82c362a716 100644 --- a/lib/tasks/assets.rake +++ b/lib/tasks/assets.rake @@ -51,7 +51,8 @@ task 'assets:precompile:css' => 'environment' do if ActiveRecord::Base.connection.table_exists?(Theme.table_name) STDERR.puts "Compiling css for #{db} #{Time.zone.now}" begin - Stylesheet::Manager.precompile_css + Stylesheet::Manager.precompile_css if db == "default" + Stylesheet::Manager.precompile_theme_css rescue PG::UndefinedColumn, ActiveModel::MissingAttributeError, NoMethodError => e STDERR.puts "#{e.class} #{e.message}: #{e.backtrace.join("\n")}" STDERR.puts "Skipping precompilation of CSS cause schema is old, you are precompiling prior to running migrations." diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb index 1c7673ac933..98fda863aed 100644 --- a/spec/components/stylesheet/manager_spec.rb +++ b/spec/components/stylesheet/manager_spec.rb @@ -39,7 +39,7 @@ describe Stylesheet::Manager do }} it "generates the right links for non-theme targets" do - manager = manager(theme.id) + manager = manager(nil) hrefs = manager.stylesheet_details(:desktop, 'all') @@ -607,8 +607,7 @@ describe Stylesheet::Manager do end end - # this test takes too long, we don't run it by default - describe ".precompile_css", if: ENV["RUN_LONG_TESTS"] == "1" do + describe "precompile css" do before do class << STDERR alias_method :orig_write, :write @@ -661,14 +660,22 @@ describe Stylesheet::Manager do StylesheetCache.destroy_all + # only core Stylesheet::Manager.precompile_css results = StylesheetCache.pluck(:target) + expect(results.size).to eq(core_targets.size) - expect(results.size).to eq(24) # (2 themes x 8 targets) + (1 child Theme x 2 targets) + 6 color schemes (2 custom theme schemes, 4 base schemes) + StylesheetCache.destroy_all - core_targets.each do |tar| - expect(results.count { |target| target =~ /^#{tar}_(#{scheme1.id}|#{scheme2.id})$/ }).to eq(2) - end + # only themes + Stylesheet::Manager.precompile_theme_css + results = StylesheetCache.pluck(:target) + expect(results.size).to eq(12) # (2 themes * 2 targets) + (one child theme * 2 targets) + 6 color schemes (2 custom, 4 base schemes) + + # themes + core + Stylesheet::Manager.precompile_css + results = StylesheetCache.pluck(:target) + expect(results.size).to eq(18) # core targets + 6 theme + 6 color schemes theme_targets.each do |tar| expect(results.count { |target| target =~ /^#{tar}_(#{user_theme.id}|#{default_theme.id})$/ }).to eq(2) @@ -677,18 +684,11 @@ describe Stylesheet::Manager do Theme.clear_default! StylesheetCache.destroy_all + # themes + core with no theme set as default Stylesheet::Manager.precompile_css + Stylesheet::Manager.precompile_theme_css results = StylesheetCache.pluck(:target) - - expect(results.size).to eq(30) # (2 themes x 8 targets) + (1 child Theme x 2 targets) + (1 no/default/core theme x 6 core targets) + 6 color schemes (2 custom theme schemes, 4 base schemes) - - core_targets.each do |tar| - expect(results.count { |target| target =~ /^(#{tar}_(#{scheme1.id}|#{scheme2.id})|#{tar})$/ }).to eq(3) - end - - theme_targets.each do |tar| - expect(results.count { |target| target =~ /^#{tar}_(#{user_theme.id}|#{default_theme.id})$/ }).to eq(2) - end + expect(results.size).to eq(18) # core targets + 6 theme + 6 color schemes expect(results).to include(color_scheme_targets[0]) expect(results).to include(color_scheme_targets[1])