FIX: Improve SCSS handling in components (#11963)

- ignores errors when including component SCSS in parent theme
- adds support for SCSS `@import`s in components' `color_definitions.scss` files
This commit is contained in:
Penar Musaraj 2021-02-04 08:51:18 -05:00 committed by GitHub
parent b580e3e657
commit 12ffba771c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 33 additions and 13 deletions

View File

@ -354,6 +354,18 @@ class ThemeField < ActiveRecord::Base
)
end
def compiled_css
css, _source_map = begin
compile_scss
rescue SassC::SyntaxError => e
# We don't want to raise a blocking error here
# admin theme editor or discourse_theme CLI will show it nonetheless
Rails.logger.error "SCSS compilation error: #{e.message}"
["", nil]
end
css
end
def ensure_scss_compiles!
result = ["failed"]
begin

View File

@ -141,9 +141,15 @@ module Stylesheet
if resolved_ids
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.name}\n\n"
contents << row.value
Theme.list_baked_fields(resolved_ids, :common, :color_definitions).each do |field|
contents << "// Color definitions from #{field.theme.name}\n\n"
if field.theme_id == theme.id
contents << field.value
else
contents << field.compiled_css
end
contents << "\n\n"
end
end
contents
@ -198,13 +204,7 @@ module Stylesheet
if field.theme_id == theme.id
contents << value
else
css, source_map = begin
field.compile_scss
rescue SassC::SyntaxError => e
raise Discourse::ScssError, e.message
end
contents << css
contents << field.compiled_css
end
end

View File

@ -64,8 +64,8 @@ describe Stylesheet::Importer do
end
context "#import_color_definitions" do
let(:scss) { ":root { --custom-color: green}" }
let(:scss_child) { ":root { --custom-color: red}" }
let(:scss) { ":root{--custom-color: green}" }
let(:scss_child) { ":root{--custom-color: red}" }
let(:theme) do
Fabricate(:theme).tap do |t|
@ -74,7 +74,7 @@ describe Stylesheet::Importer do
end
end
let(:child) { Fabricate(:theme, component: true).tap { |t|
let(:child) { Fabricate(:theme, component: true, name: "Child Theme").tap { |t|
t.set_field(target: :common, name: "color_definitions", value: scss_child)
t.save!
}}
@ -90,6 +90,7 @@ describe Stylesheet::Importer do
styles = Stylesheet::Importer.import_color_definitions(theme.id)
expect(styles).to include(scss_child)
expect(styles).to include("Color definitions from Child Theme")
end
it "should include default theme color definitions" do

View File

@ -807,6 +807,13 @@ HTML
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
describe "scss_variables" do