DEV: Use separate files for theme component stylesheets (#12214)
This switches to outputting a separate file for each theme component CSS asset. We have separate CSS plugin files, separate JS files (for plugins/themes/components), it makes sense to do the same for component CSS assets. Benefits: - easier debugging - fixes a regression with theme component sourcemaps - changes to theme components are updated individually With HTTP/2, there is also no performance downside to having additional files in the initial request.
This commit is contained in:
parent
bb3d5e9758
commit
f57a49c2f9
|
@ -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,13 +63,11 @@ 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)
|
||||
) {
|
||||
|
|
|
@ -315,8 +315,7 @@ class Theme < ActiveRecord::Base
|
|||
if all_themes
|
||||
message = theme_ids.map { |id| refresh_message_for_targets(targets, id) }.flatten
|
||||
else
|
||||
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
|
||||
message = refresh_message_for_targets(targets, theme_ids).flatten
|
||||
end
|
||||
|
||||
MessageBus.publish('/file-change', message)
|
||||
|
@ -372,7 +371,7 @@ class Theme < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def list_baked_fields(target, name)
|
||||
theme_ids = Theme.transform_ids([id])
|
||||
theme_ids = Theme.transform_ids([id], extend: false)
|
||||
self.class.list_baked_fields(theme_ids, target, name)
|
||||
end
|
||||
|
||||
|
|
|
@ -194,18 +194,13 @@ 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
|
||||
|
||||
if field.theme_id == theme.id
|
||||
contents << value
|
||||
else
|
||||
contents << field.compiled_css(prepended_scss)
|
||||
end
|
||||
contents << value
|
||||
end
|
||||
|
||||
end
|
||||
|
|
|
@ -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, extend: false)
|
||||
theme_ids = Theme.transform_ids(theme_ids)
|
||||
|
||||
current_hostname = Discourse.current_hostname
|
||||
|
||||
|
|
|
@ -42,7 +42,7 @@ describe Stylesheet::Manager do
|
|||
|
||||
theme.add_relative_theme!(:child, child_theme)
|
||||
|
||||
old_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id)
|
||||
old_links = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id)
|
||||
|
||||
manager = Stylesheet::Manager.new(:desktop_theme, theme.id)
|
||||
manager.compile(force: true)
|
||||
|
@ -50,27 +50,56 @@ 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_link = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id)
|
||||
new_links = Stylesheet::Manager.stylesheet_link_tag(:desktop_theme, 'all', theme.id)
|
||||
|
||||
expect(new_link).not_to eq(old_link)
|
||||
expect(new_links).not_to eq(old_links)
|
||||
|
||||
# our theme better have a name with the theme_id as part of it
|
||||
expect(new_link).to include("/stylesheets/desktop_theme_#{theme.id}_")
|
||||
expect(new_links).to include("/stylesheets/desktop_theme_#{theme.id}_")
|
||||
expect(new_links).to include("/stylesheets/desktop_theme_#{child_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
|
||||
|
|
|
@ -795,24 +795,13 @@ HTML
|
|||
expect(css).to include("p{color:blue}")
|
||||
end
|
||||
|
||||
it "works for child themes and includes child theme SCSS in parent theme" do
|
||||
it "works for child themes" 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
|
||||
|
||||
|
|
Loading…
Reference in New Issue