Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DEV: Refactor theme SCSS compilation (#11919)

This commit is contained in:
Penar Musaraj 2021-02-02 13:09:41 -05:00 committed by GitHub
parent f88def5f5b
commit e8b82724fd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 173 additions and 209 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
app/models/theme.rb
View File

@ -1,6 +1,9 @@
# frozen_string_literal: true
require_dependency 'global_path'
class Theme < ActiveRecord::Base
include GlobalPath
attr_accessor :child_components
@ -23,6 +26,7 @@ class Theme < ActiveRecord::Base
has_one :javascript_cache, dependent: :destroy
has_many :locale_fields, -> { filter_locale_fields(I18n.fallbacks[I18n.locale]) }, class_name: 'ThemeField'
has_many :upload_fields, -> { where(type_id: ThemeField.types[:theme_upload_var]).preload(:upload) }, class_name: 'ThemeField'
has_many :extra_scss_fields, -> { where(target_id: Theme.targets[:extra_scss]) }, class_name: 'ThemeField'
validate :component_validations
@ -581,8 +585,44 @@ class Theme < ActiveRecord::Base
find_disable_action_log&.created_at
end
def scss_load_paths
return if self.extra_scss_fields.empty?
@exporter ||= ThemeStore::ZipExporter.new(self)
["#{@exporter.export_dir}/scss", "#{@exporter.export_dir}/stylesheets"]
end
def scss_variables
return if all_theme_variables.empty? && included_settings.empty?
contents = +""
all_theme_variables&.each do |field|
if field.type_id == ThemeField.types[:theme_upload_var]
if upload = field.upload
url = upload_cdn_path(upload.url)
contents << "$#{field.name}: unquote(\"#{url}\");"
end
else
contents << to_scss_variable(field.name, field.value)
end
end
included_settings&.each do |name, value|
next if name == "theme_uploads"
contents << to_scss_variable(name, value)
end
contents
end
private
def to_scss_variable(name, value)
escaped = SassC::Script::Value::String.quote(value, sass: true)
"$#{name}: unquote(#{escaped});"
end
def find_disable_action_log
if component? && !enabled?
@disable_log ||= UserHistory.where(context: id.to_s, action: UserHistory.actions[:disable_theme_component]).order("created_at DESC").first

12
app/models/theme_field.rb
View File

@ -343,10 +343,14 @@ class ThemeField < ActiveRecord::Base
end
def compile_scss
Stylesheet::Compiler.compile("@import \"common/foundation/variables\"; @import \"common/foundation/mixins\"; @import \"theme_variables\"; @import \"theme_field\";",
"theme.scss",
theme_field: self.value.dup,
theme: self.theme
scss = <<~SCSS
@import "common/foundation/variables"; @import "common/foundation/mixins"; #{self.theme.scss_variables.to_s} #{self.value}
SCSS
Stylesheet::Compiler.compile(scss,
"#{Theme.targets[self.target_id]}.scss",
theme: self.theme,
load_paths: self.theme.scss_load_paths
)
end

12
lib/stylesheet/compiler.rb
View File

@ -11,9 +11,12 @@ module Stylesheet
def self.compile_asset(asset, options = {})
file = "@import \"common/foundation/variables\"; @import \"common/foundation/mixins\";"
if Importer.special_imports[asset.to_s]
if Importer::THEME_TARGETS.include?(asset.to_s)
filename = "theme_#{options[:theme_id]}.scss"
file += options[:theme_variables].to_s
file += Importer.new({ theme_id: options[:theme_id] }).theme_import(asset)
elsif Importer.special_imports[asset.to_s]
filename = "theme_#{options[:theme_id]}.scss"
file += " @import \"theme_variables\";" if Importer::THEME_TARGETS.include?(asset.to_s)
file += " @import \"#{asset}\";"
else
filename = "#{asset}.scss"
@ -33,6 +36,9 @@ module Stylesheet
def self.compile(stylesheet, filename, options = {})
source_map_file = options[:source_map_file] || "#{filename.sub(".scss", "")}.css.map"
load_paths = [Stylesheet::Common::ASSET_ROOT]
load_paths += options[:load_paths] if options[:load_paths]
engine = SassC::Engine.new(stylesheet,
importer: Importer,
filename: filename,
@ -43,7 +49,7 @@ module Stylesheet
theme: options[:theme],
theme_field: options[:theme_field],
color_scheme_id: options[:color_scheme_id],
load_paths: [Stylesheet::Common::ASSET_ROOT])
load_paths: load_paths)
result = engine.render

111
lib/stylesheet/importer.rb
View File

@ -21,10 +21,6 @@ module Stylesheet
def self.register_imports!
@special_imports = {}
register_import "theme_field" do
Import.new("#{theme_dir(@theme_id)}/theme_field.scss", source: @theme_field)
end
Discourse.plugins.each do |plugin|
plugin_directory_name = plugin.directory_name
@ -118,28 +114,6 @@ module Stylesheet
Import.new("theme_colors.scss", source: contents)
end
register_import "theme_variables" do
contents = +""
theme&.all_theme_variables&.each do |field|
if field.type_id == ThemeField.types[:theme_upload_var]
if upload = field.upload
url = upload_cdn_path(upload.url)
contents << "$#{field.name}: unquote(\"#{url}\");\n"
end
else
contents << to_scss_variable(field.name, field.value)
end
end
theme&.included_settings&.each do |name, value|
next if name == "theme_uploads"
contents << to_scss_variable(name, value)
end
Import.new("theme_variable.scss", source: contents)
end
register_import "category_backgrounds" do
contents = +""
Category.where('uploaded_background_id IS NOT NULL').each do |c|
@ -149,23 +123,6 @@ module Stylesheet
Import.new("category_background.scss", source: contents)
end
register_import "embedded_theme" do
next unless @theme_id
theme_import(:common, :embedded_scss)
end
register_import "mobile_theme" do
next unless @theme_id
theme_import(:mobile, :scss)
end
register_import "desktop_theme" do
next unless @theme_id
theme_import(:desktop, :scss)
end
end
register_imports!
@ -182,9 +139,10 @@ module Stylesheet
resolved_ids = Theme.transform_ids([theme_id])
if resolved_ids
contents << " @import \"theme_variables\";"
theme = Theme.find_by_id(theme_id)
contents << theme&.scss_variables.to_s
Theme.list_baked_fields(resolved_ids, :common, :color_definitions).each do |row|
contents << "// Color definitions from #{Theme.find_by_id(theme_id)&.name}\n\n"
contents << "// Color definitions from #{theme.name}\n\n"
contents << row.value
end
end
@ -201,14 +159,12 @@ module Stylesheet
def initialize(options)
@theme = options[:theme]
@theme_id = options[:theme_id]
@theme_field = options[:theme_field]
@color_scheme_id = options[:color_scheme_id]
if @theme && !@theme_id
# make up an id so other stuff does not bail out
@theme_id = @theme.id || -1
end
@importable_theme_fields = {}
end
def import_files(files)
@ -222,23 +178,27 @@ module Stylesheet
end
end
def theme_import(target, attr)
fields = theme.list_baked_fields(target, attr)
def theme_import(target)
attr = target == :embedded_theme ? :embedded_scss : :scss
target = target.to_s.gsub("_theme", "").to_sym
contents = +""
fields = theme.list_baked_fields(target, attr)
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"
with_comment = <<~COMMENT
contents << <<~COMMENT
// Theme: #{field.theme.name}
// Target: #{field.target_name} #{field.name}
// Last Edited: #{field.updated_at}
#{value}
COMMENT
Import.new(filename, source: with_comment)
end
end.compact
end
contents
end
def theme
@ -248,43 +208,6 @@ module Stylesheet
@theme == :nil ? nil : @theme
end
def theme_dir(import_theme_id)
"theme_#{import_theme_id}"
end
def extract_theme_id(path)
path[/^theme_([0-9]+)\//, 1]
end
def importable_theme_fields(import_theme_id)
return {} unless theme && import_theme = Theme.find(import_theme_id)
@importable_theme_fields[import_theme_id] ||= begin
hash = {}
import_theme.theme_fields.where(target_id: Theme.targets[:extra_scss]).each do |field|
hash[field.name] = field.value
end
hash
end
end
def match_theme_import(path, parent_path)
# Only allow importing theme stylesheets from within stylesheets in the same theme
return false unless theme && import_theme_id = extract_theme_id(parent_path) # Could be a child theme
parent_dir, _ = File.split(parent_path)
# Could be relative to the importing file, or relative to the root of the theme directory
search_paths = [parent_dir, theme_dir(import_theme_id)].uniq
search_paths.each do |search_path|
resolved = Pathname.new("#{search_path}/#{path}").cleanpath.to_s # Remove unnecessary ./ and ../
next unless resolved.start_with?("#{theme_dir(import_theme_id)}/")
resolved_within_theme = resolved.sub(/^theme_[0-9]+\//, "")
if importable_theme_fields(import_theme_id).keys.include?(resolved_within_theme)
return resolved, importable_theme_fields(import_theme_id)[resolved_within_theme]
end
end
false
end
def category_css(category)
full_slug = category.full_slug.split("-")[0..-2].join("-")
"body.category-#{full_slug} { background-image: url(#{upload_cdn_path(category.uploaded_background.url)}) }\n"
@ -309,22 +232,12 @@ module Stylesheet
contents
end
def to_scss_variable(name, value)
escaped = SassC::Script::Value::String.quote(value, sass: true)
"$#{name}: unquote(#{escaped});\n"
end
def imports(asset, parent_path)
if callback = Importer.special_imports[asset]
instance_eval(&callback)
else
path, source = match_theme_import(asset, parent_path)
if path && source
Import.new(path, source: source)
else
Import.new(asset + ".scss")
end
end
end
end
end

4
lib/stylesheet/manager.rb
View File

@ -248,8 +248,10 @@ class Stylesheet::Manager
@target,
rtl: rtl,
theme_id: theme&.id,
theme_variables: theme&.scss_variables.to_s,
source_map_file: source_map_filename,
color_scheme_id: @color_scheme&.id
color_scheme_id: @color_scheme&.id,
load_paths: theme&.scss_load_paths
)
rescue SassC::SyntaxError => e
if Stylesheet::Importer::THEME_TARGETS.include?(@target.to_s)

6
lib/theme_store/zip_exporter.rb
View File

@ -55,6 +55,12 @@ class ThemeStore::ZipExporter
@temp_folder
end
def export_dir
export_to_folder
File.join(@temp_folder, @export_name)
end
private
def export_package

4
spec/components/stylesheet/compiler_spec.rb
View File

@ -17,13 +17,13 @@ describe Stylesheet::Compiler do
context "with a theme" do
let!(:theme) { Fabricate(:theme) }
let!(:upload) { Fabricate(:upload) }
let!(:upload) { UploadCreator.new(file_from_fixtures("logo.png"), "logo.png").create_for(Discourse.system_user.id) }
let!(:upload_theme_field) { ThemeField.create!(theme: theme, target_id: 0, name: "primary", upload: upload, value: "", type_id: ThemeField.types[:theme_upload_var]) }
let!(:stylesheet_theme_field) { ThemeField.create!(theme: theme, target_id: 0, name: "scss", value: "body { background: $primary }", type_id: ThemeField.types[:scss]) }
before { stylesheet_theme_field.save! }
it "theme stylesheet should be able to access theme asset variables" do
css, _map = Stylesheet::Compiler.compile_asset("desktop_theme", theme_id: theme.id)
css, _map = Stylesheet::Compiler.compile_asset("desktop_theme", theme_id: theme.id, theme_variables: theme.scss_variables)
expect(css).to include(upload.url)
end

90
spec/components/stylesheet/importer_spec.rb
View File

@ -63,96 +63,6 @@ describe Stylesheet::Importer do
.to eq(DiscourseFonts.fonts.map { |f| f[:variants]&.count || 0 }.sum)
end
context "#theme_variables" do
let!(:theme) { Fabricate(:theme) }
let(:importer) { described_class.new(theme: theme) }
fab!(:upload) { Fabricate(:upload) }
fab!(:upload_s3) { Fabricate(:upload_s3) }
let!(:theme_field) { ThemeField.create!(theme: theme, target_id: 0, name: "var", upload: upload, value: "", type_id: ThemeField.types[:theme_upload_var]) }
let!(:theme_field_s3) { ThemeField.create!(theme: theme, target_id: 1, name: "var_s3", upload: upload_s3, value: "", type_id: ThemeField.types[:theme_upload_var]) }
it "should contain the URL" do
theme_field.save!
import = importer.imports("theme_variables", nil)
expect(import.source).to include(upload.url)
end
it "should contain the S3 URL" do
theme_field_s3.save!
import = importer.imports("theme_variables", nil)
expect(import.source).to include(upload_s3.url)
end
end
context "extra_scss" do
let(:scss) { "body { background: red}" }
let(:child_scss) { "body { background: green}" }
let(:theme) { Fabricate(:theme).tap { |t|
t.set_field(target: :extra_scss, name: "my_files/magic", value: scss)
t.save!
}}
let(:child_theme) { Fabricate(:theme).tap { |t|
t.component = true
t.set_field(target: :extra_scss, name: "my_files/moremagic", value: child_scss)
t.save!
theme.add_relative_theme!(:child, t)
}}
let(:importer) { described_class.new(theme: theme) }
it "should be able to import correctly" do
# Import from regular theme file
expect(
importer.imports(
"my_files/magic",
"theme_#{theme.id}/desktop-scss-mytheme.scss"
).source).to eq(scss)
# Import from some deep file
expect(
importer.imports(
"my_files/magic",
"theme_#{theme.id}/some/deep/folder/structure/myfile.scss"
).source).to eq(scss)
# Import from parent dir
expect(
importer.imports(
"../../my_files/magic",
"theme_#{theme.id}/my_files/folder1/myfile.scss"
).source).to eq(scss)
# Import from same dir without ./
expect(
importer.imports(
"magic",
"theme_#{theme.id}/my_files/myfile.scss"
).source).to eq(scss)
# Import from same dir with ./
expect(
importer.imports(
"./magic",
"theme_#{theme.id}/my_files/myfile.scss"
).source).to eq(scss)
# Import within a child theme
expect(
importer.imports(
"my_files/moremagic",
"theme_#{child_theme.id}/theme_field.scss"
).source).to eq(child_scss)
end
end
context "#import_color_definitions" do
let(:scss) { ":root { --custom-color: green}" }
let(:scss_child) { ":root { --custom-color: red}" }

9
spec/components/stylesheet/manager_spec.rb
View File

@ -64,6 +64,13 @@ describe Stylesheet::Manager do
# our theme better have a name with the theme_id as part of it
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).to match(/\.child_embedded/)
end
describe 'digest' do
@ -285,7 +292,7 @@ describe Stylesheet::Manager do
let(:scheme) { ColorScheme.base }
it "includes theme color definitions in color scheme" do
stylesheet = Stylesheet::Manager.new(:color_definitions, theme.id, scheme).compile
stylesheet = Stylesheet::Manager.new(:color_definitions, theme.id, scheme).compile(force: true)
expect(stylesheet).to include("--special: rebeccapurple")
end

11
spec/models/theme_field_spec.rb
View File

@ -150,20 +150,26 @@ HTML
field = ThemeField.create!(theme_id: 1, target_id: 0, name: "scss", value: css)
field.ensure_baked!
expect(field.error).not_to eq(nil)
field.value = "@import 'missingfile';"
field.save!
field.ensure_baked!
expect(field.error).to include("File to import not found or unreadable: missingfile.scss.")
field.value = "body {color: blue};"
field.save!
field.ensure_baked!
expect(field.error).to eq(nil)
end
it "allows importing scss files" do
theme = Fabricate(:theme)
main_field = theme.set_field(target: :common, name: :scss, value: ".class1{color: red}\n@import 'rootfile1';")
main_field = theme.set_field(target: :common, name: :scss, value: ".class1{color: red}\n@import 'rootfile1';\n@import 'rootfile3';")
theme.set_field(target: :extra_scss, name: "rootfile1", value: ".class2{color:green}\n@import 'foldername/subfile1';")
theme.set_field(target: :extra_scss, name: "rootfile2", value: ".class3{color:green} ")
theme.set_field(target: :extra_scss, name: "foldername/subfile1", value: ".class4{color:yellow}\n@import 'subfile2';")
theme.set_field(target: :extra_scss, name: "foldername/subfile2", value: ".class5{color:yellow}\n@import '../rootfile2';")
theme.set_field(target: :extra_scss, name: "rootfile3", value: ".class6{color:green} ")
theme.save!
result = main_field.compile_scss[0]
@ -173,6 +179,7 @@ HTML
expect(result).to include(".class3")
expect(result).to include(".class4")
expect(result).to include(".class5")
expect(result).to include(".class6")
end
it "correctly handles extra JS fields" do

79
spec/models/theme_spec.rb
View File

@ -328,7 +328,7 @@ HTML
theme.reload
expect(theme.theme_fields.find_by(name: :scss).error).to eq(nil)
scss, _map = Stylesheet::Compiler.compile('@import "common/foundation/variables"; @import "theme_variables"; @import "desktop_theme"; ', "theme.scss", theme_id: theme.id)
scss, _map = Stylesheet::Manager.new(:desktop_theme, theme.id).compile(force: true)
expect(scss).to include(upload.url)
end
end
@ -339,7 +339,7 @@ HTML
theme.set_field(target: :common, name: :scss, value: 'body {background-color: $background_color; font-size: $font-size}')
theme.save!
scss, _map = Stylesheet::Compiler.compile('@import "theme_variables"; @import "desktop_theme"; ', "theme.scss", theme_id: theme.id)
scss, _map = Stylesheet::Manager.new(:desktop_theme, theme.id).compile(force: true)
expect(scss).to include("background-color:red")
expect(scss).to include("font-size:25px")
@ -347,7 +347,7 @@ HTML
setting.value = '30px'
theme.save!
scss, _map = Stylesheet::Compiler.compile('@import "theme_variables"; @import "desktop_theme"; ', "theme.scss", theme_id: theme.id)
scss, _map = Stylesheet::Manager.new(:desktop_theme, theme.id).compile(force: true)
expect(scss).to include("font-size:30px")
# Escapes correctly. If not, compiling this would throw an exception
@ -359,7 +359,7 @@ HTML
theme.set_field(target: :common, name: :scss, value: 'body {font-size: quote($font-size)}')
theme.save!
scss, _map = Stylesheet::Compiler.compile('@import "theme_variables"; @import "desktop_theme"; ', "theme.scss", theme_id: theme.id)
scss, _map = Stylesheet::Manager.new(:desktop_theme, theme.id).compile(force: true)
expect(scss).to include('font-size:"#{$fakeinterpolatedvariable}\a andanothervalue \'withquotes\'; margin: 0;\a"')
end
@ -569,7 +569,7 @@ HTML
it 'includes theme_uploads in settings' do
Theme.destroy_all
upload = Fabricate(:upload)
upload = UploadCreator.new(file_from_fixtures("logo.png"), "logo.png").create_for(-1)
theme.set_field(type: :theme_upload_var, target: :common, name: "bob", upload_id: upload.id)
theme.save!
@ -754,4 +754,73 @@ HTML
expect(Theme.lookup_field(theme.id, :common, :after_header)).to include("_ws=someotherhostname.com")
end
end
describe "extra_scss" do
let(:scss) { "body { background: red}" }
let(:second_file_scss) { "p { color: blue};" }
let(:child_scss) { "body { background: green}" }
let(:theme) { Fabricate(:theme).tap { |t|
t.set_field(target: :extra_scss, name: "my_files/magic", value: scss)
t.set_field(target: :extra_scss, name: "my_files/magic2", value: second_file_scss)
t.save!
}}
let(:child_theme) { Fabricate(:theme).tap { |t|
t.component = true
t.set_field(target: :extra_scss, name: "my_files/moremagic", value: child_scss)
t.save!
theme.add_relative_theme!(:child, t)
}}
let(:compiler) {
manager = Stylesheet::Manager.new(:desktop_theme, theme.id)
manager.compile(force: true)
}
it "works when importing file by path" do
theme.set_field(target: :common, name: :scss, value: '@import "my_files/magic";')
theme.save!
css, _map = compiler
expect(css).to include("body{background:red}")
end
it "works when importing multiple files" do
theme.set_field(target: :common, name: :scss, value: '@import "my_files/magic"; @import "my_files/magic2"')
theme.save!
css, _map = compiler
expect(css).to include("body{background:red}")
expect(css).to include("p{color:blue}")
end
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}")
end
end
describe "scss_variables" do
it "is empty by default" do
expect(theme.scss_variables).to eq(nil)
end
it "includes settings and uploads when set" do
theme.set_field(target: :settings, name: :yaml, value: "background_color: red\nfont_size: 25px")
upload = UploadCreator.new(file_from_fixtures("logo.png"), "logo.png").create_for(-1)
theme.set_field(type: :theme_upload_var, target: :common, name: "bobby", upload_id: upload.id)
theme.save!
expect(theme.scss_variables).to include("$background_color: unquote(\"red\")")
expect(theme.scss_variables).to include("$font_size: unquote(\"25px\")")
expect(theme.scss_variables).to include("$bobby: ")
end
end
end

2
spec/requests/admin/themes_controller_spec.rb
View File

@ -342,7 +342,7 @@ describe Admin::ThemesController do
child_theme = Fabricate(:theme, component: true)
upload = Fabricate(:upload)
upload = UploadCreator.new(file_from_fixtures("logo.png"), "logo.png").create_for(Discourse.system_user.id)
put "/admin/themes/#{theme.id}.json", params: {
theme: {