From e74bdfdf8e47c8fb15ba5e1cd3870d9bba1c78c8 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Fri, 26 Feb 2021 08:20:39 -0500 Subject: [PATCH] Revert "DEV: Use separate files for theme component stylesheets (#12214)" (#12224) This reverts commit f57a49c2f97c78865a4ad806339a2f847d6bc98c. This had some unexpected side effects, needs some more work. --- .../app/initializers/live-development.js | 4 +- app/models/theme.rb | 5 ++- lib/stylesheet/importer.rb | 7 +++- lib/stylesheet/manager.rb | 2 +- spec/components/stylesheet/manager_spec.rb | 41 +++---------------- spec/models/theme_spec.rb | 13 +++++- 6 files changed, 31 insertions(+), 41 deletions(-) diff --git a/app/assets/javascripts/discourse/app/initializers/live-development.js b/app/assets/javascripts/discourse/app/initializers/live-development.js index 058865af3c4..00f1f89d0ba 100644 --- a/app/assets/javascripts/discourse/app/initializers/live-development.js +++ b/app/assets/javascripts/discourse/app/initializers/live-development.js @@ -1,7 +1,7 @@ +import { currentThemeIds, refreshCSS } from "discourse/lib/theme-selector"; 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 { @@ -63,11 +63,13 @@ export default { // Refresh if necessary document.location.reload(true); } else { + const themeIds = currentThemeIds(); $("link").each(function () { if (me.hasOwnProperty("theme_id") && me.new_href) { const target = $(this).data("target"); const themeId = $(this).data("theme-id"); if ( + themeIds.indexOf(me.theme_id) !== -1 && target === me.target && (!themeId || themeId === me.theme_id) ) { diff --git a/app/models/theme.rb b/app/models/theme.rb index e6120778ff1..418b3595349 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -315,7 +315,8 @@ class Theme < ActiveRecord::Base if all_themes message = theme_ids.map { |id| refresh_message_for_targets(targets, id) }.flatten else - message = refresh_message_for_targets(targets, theme_ids).flatten + parent_ids = Theme.where(id: theme_ids).joins(:parent_themes).pluck(:parent_theme_id).uniq + message = refresh_message_for_targets(targets, theme_ids | parent_ids).flatten end MessageBus.publish('/file-change', message) @@ -371,7 +372,7 @@ class Theme < ActiveRecord::Base end def list_baked_fields(target, name) - theme_ids = Theme.transform_ids([id], extend: false) + theme_ids = Theme.transform_ids([id]) self.class.list_baked_fields(theme_ids, target, name) end diff --git a/lib/stylesheet/importer.rb b/lib/stylesheet/importer.rb index d339fe8e654..b4d65498f4b 100644 --- a/lib/stylesheet/importer.rb +++ b/lib/stylesheet/importer.rb @@ -194,13 +194,18 @@ module Stylesheet fields.map do |field| value = field.value if value.present? + filename = "theme_#{field.theme.id}/#{field.target_name}-#{field.name}-#{field.theme.name.parameterize}.scss" contents << <<~COMMENT // Theme: #{field.theme.name} // Target: #{field.target_name} #{field.name} // Last Edited: #{field.updated_at} COMMENT - contents << value + if field.theme_id == theme.id + contents << value + else + contents << field.compiled_css(prepended_scss) + end end end diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb index c23a7eaa11c..1689a1acef5 100644 --- a/lib/stylesheet/manager.rb +++ b/lib/stylesheet/manager.rb @@ -58,7 +58,7 @@ class Stylesheet::Manager theme_ids = [theme_ids] unless Array === theme_ids theme_ids = [theme_ids.first] unless target =~ THEME_REGEX - theme_ids = Theme.transform_ids(theme_ids) + theme_ids = Theme.transform_ids(theme_ids, extend: false) current_hostname = Discourse.current_hostname diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb index 4ffff1744bc..22b756de94d 100644 --- a/spec/components/stylesheet/manager_spec.rb +++ b/spec/components/stylesheet/manager_spec.rb @@ -42,7 +42,7 @@ describe Stylesheet::Manager do theme.add_relative_theme!(:child, child_theme) - old_links = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id) + old_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id) manager = Stylesheet::Manager.new(:desktop_theme, theme.id) manager.compile(force: true) @@ -50,56 +50,27 @@ describe Stylesheet::Manager do css = File.read(manager.stylesheet_fullpath) _source_map = File.read(manager.source_map_fullpath) + expect(css).to match(/child_common/) + expect(css).to match(/child_desktop/) expect(css).to match(/\.common/) expect(css).to match(/\.desktop/) - # child theme CSS is no longer bundled with main theme - expect(css).not_to match(/child_common/) - expect(css).not_to match(/child_desktop/) - - child_theme_manager = Stylesheet::Manager.new(:desktop_theme, child_theme.id) - child_theme_manager.compile(force: true) - - child_css = File.read(child_theme_manager.stylesheet_fullpath) - _child_source_map = File.read(child_theme_manager.source_map_fullpath) - - expect(child_css).to match(/child_common/) - expect(child_css).to match(/child_desktop/) - child_theme.set_field(target: :desktop, name: :scss, value: ".nothing{color: green;}") child_theme.save! - new_links = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id) + new_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id) - expect(new_links).not_to eq(old_links) + expect(new_link).not_to eq(old_link) # our theme better have a name with the theme_id as part of it - expect(new_links).to include("/stylesheets/desktop_theme_#{theme.id}_") - expect(new_links).to include("/stylesheets/desktop_theme_#{child_theme.id}_") + expect(new_link).to include("/stylesheets/desktop_theme_#{theme.id}_") manager = Stylesheet::Manager.new(:embedded_theme, theme.id) manager.compile(force: true) css = File.read(manager.stylesheet_fullpath) expect(css).to match(/\.embedded/) - expect(css).not_to match(/\.child_embedded/) - - child_theme_manager = Stylesheet::Manager.new(:embedded_theme, child_theme.id) - child_theme_manager.compile(force: true) - - css = File.read(child_theme_manager.stylesheet_fullpath) expect(css).to match(/\.child_embedded/) - - # ensure theme + component stylesheets are included - hrefs = Stylesheet::Manager.stylesheet_details(:desktop, 'all', [theme.id]) - expect(hrefs.count).to eq(2) - expect(hrefs[0][:theme_id]).to eq(theme.id) - expect(hrefs[1][:theme_id]).to eq(child_theme.id) - - hrefs = Stylesheet::Manager.stylesheet_details(:embedded_theme, 'all', [theme.id]) - expect(hrefs.count).to eq(2) - expect(hrefs[0][:theme_id]).to eq(theme.id) - expect(hrefs[1][:theme_id]).to eq(child_theme.id) end describe 'digest' do diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index f19cdb38191..3a65d8ab265 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -795,13 +795,24 @@ HTML expect(css).to include("p{color:blue}") end - it "works for child themes" do + it "works for child themes and includes child theme SCSS in parent theme" do child_theme.set_field(target: :common, name: :scss, value: '@import "my_files/moremagic"') child_theme.save! manager = Stylesheet::Manager.new(:desktop_theme, child_theme.id) css, _map = manager.compile(force: true) expect(css).to include("body{background:green}") + + parent_css, _parent_map = compiler + expect(parent_css).to include("body{background:green}") + end + + it "does not fail if child theme has SCSS errors" do + child_theme.set_field(target: :common, name: :scss, value: 'p {color: $missing_var;}') + child_theme.save! + + parent_css, _parent_map = compiler + expect(parent_css).to include("sourceMappingURL") end end