From 6986b36985fa242cb3d346b05f84d3878573c354 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 30 Jun 2021 16:06:13 +0800 Subject: [PATCH] FIX: Child themes being precompiled multiple times. --- lib/stylesheet/manager.rb | 43 ++++++++++------------ spec/components/stylesheet/manager_spec.rb | 8 +++- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index b3749e48939..ec48eee3b7f 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -46,54 +46,49 @@ class Stylesheet::Manager targets += Discourse.find_plugin_css_assets(include_disabled: true, mobile_view: true, desktop_view: true) targets.each do |target| - STDERR.puts "precompile target: #{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 + themes = Theme.where('user_selectable OR id = ?', SiteSetting.default_theme_id).pluck(:id, :color_scheme_id) 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_theme, :mobile_theme] + compiled = Set.new - themes.each do |id, name, 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 + next if theme_id == -1 - scss_checker = ScssChecker.new(target, manager.theme_ids) + scss_checker = ScssChecker.new(target, manager.theme_ids) - manager.load_themes(manager.theme_ids).each do |theme| - builder = Stylesheet::Manager::Builder.new( - target: target, theme: theme, manager: manager - ) + manager.load_themes(manager.theme_ids).each do |theme| + next if compiled.include?("#{target}_#{theme.id}") - STDERR.puts "precompile target: #{target} #{builder.theme.name}" - next if theme.component && !scss_checker.has_scss(theme.id) - builder.compile(force: true) - end - else - STDERR.puts "precompile target: #{target} #{name}" + builder = Stylesheet::Manager::Builder.new( + target: target, theme: theme, manager: manager + ) - Stylesheet::Manager::Builder.new( - target: target, theme: manager.get_theme(theme_id), manager: manager - ).compile(force: true) + $stderr.puts "precompile target: #{target} #{theme.name}" + next if theme.component && !scss_checker.has_scss(theme.id) + builder.compile(force: true) + compiled << "#{target}_#{theme.id}" end end - theme_color_scheme = ColorScheme.find_by_id(color_scheme_id) || ColorScheme.base + theme_color_scheme = ColorScheme.find_by_id(color_scheme_id) - [theme_color_scheme, *color_schemes].uniq.each do |scheme| - STDERR.puts "precompile target: #{COLOR_SCHEME_STYLESHEET} #{name} (#{scheme.name})" + [theme_color_scheme, *color_schemes].compact.uniq.each do |scheme| + $stderr.puts "precompile target: #{COLOR_SCHEME_STYLESHEET} #{name} (#{scheme.name})" Stylesheet::Manager::Builder.new( target: COLOR_SCHEME_STYLESHEET, diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb index 0287c2bf635..51ffcc04608 100644 --- a/spec/components/stylesheet/manager_spec.rb +++ b/spec/components/stylesheet/manager_spec.rb @@ -654,6 +654,7 @@ 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! @@ -668,7 +669,12 @@ describe Stylesheet::Manager do StylesheetCache.destroy_all # only themes - Stylesheet::Manager.precompile_theme_css + output = capture_output(:stderr) do + Stylesheet::Manager.precompile_theme_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(12) # (2 themes * 2 targets) + (one child theme * 2 targets) + 6 color schemes (2 custom, 4 base schemes)