From c54d58e28f9d026a1d9076e1740983d406c1bf79 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan <gxtan1990@gmail.com> Date: Thu, 1 Jul 2021 10:09:39 +0800 Subject: [PATCH] FIX: Child themes being precompiled multiple times take 2. (#13599) This fix was reverted in 128fdf9d9ce92275ecb0a2bdb12bdf038a811a45 but fix is still relevant. --- lib/stylesheet/manager.rb | 18 ++++++++++++------ spec/components/stylesheet/manager_spec.rb | 8 +++++++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index e8713943c52..3df9a0b0723 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -42,7 +42,7 @@ class Stylesheet::Manager end def self.precompile_css - themes = Theme.where('user_selectable OR id = ?', SiteSetting.default_theme_id).pluck(:id, :name, :color_scheme_id) + 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 @@ -51,8 +51,9 @@ class Stylesheet::Manager 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) + compiled = Set.new - themes.each do |id, name, color_scheme_id| + themes.each do |id, color_scheme_id| theme_id = id || SiteSetting.default_theme_id manager = self.new(theme_id: theme_id) @@ -63,16 +64,19 @@ class Stylesheet::Manager scss_checker = ScssChecker.new(target, manager.theme_ids) manager.load_themes(manager.theme_ids).each do |theme| + next if compiled.include?("#{target}_#{theme.id}") + builder = Stylesheet::Manager::Builder.new( target: target, theme: theme, manager: manager ) - STDERR.puts "precompile target: #{target} #{builder.theme.name}" next if theme.component && !scss_checker.has_scss(theme.id) + $stderr.puts "precompile target: #{target} #{theme.name}" builder.compile(force: true) + compiled << "#{target}_#{theme.id}" end else - STDERR.puts "precompile target: #{target} #{name}" + $stderr.puts "precompile target: #{target} #{name}" Stylesheet::Manager::Builder.new( target: target, theme: manager.get_theme(theme_id), manager: manager @@ -81,17 +85,19 @@ class Stylesheet::Manager end theme_color_scheme = ColorScheme.find_by_id(color_scheme_id) || ColorScheme.base + theme = manager.get_theme(theme_id) [theme_color_scheme, *color_schemes].uniq.each do |scheme| - STDERR.puts "precompile target: #{COLOR_SCHEME_STYLESHEET} #{name} (#{scheme.name})" + $stderr.puts "precompile target: #{COLOR_SCHEME_STYLESHEET} (#{scheme.name}) #{theme&.name}" Stylesheet::Manager::Builder.new( target: COLOR_SCHEME_STYLESHEET, - theme: manager.get_theme(theme_id), + theme: theme, color_scheme: scheme, manager: manager ).compile(force: true) end + clear_color_scheme_cache! end diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb index da6768d07a8..7acbe325121 100644 --- a/spec/components/stylesheet/manager_spec.rb +++ b/spec/components/stylesheet/manager_spec.rb @@ -650,13 +650,19 @@ describe Stylesheet::Manager do t.save! user_theme.add_relative_theme!(:child, t) + default_theme.add_relative_theme!(:child, t) end default_theme.set_default! StylesheetCache.destroy_all - Stylesheet::Manager.precompile_css + output = capture_output(:stderr) do + Stylesheet::Manager.precompile_css + end + + # Ensure we force compile each theme only once + expect(output.scan(/#{child_theme_with_css.name}/).length).to eq(2) results = StylesheetCache.pluck(:target) 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)