DEV: Allow stylesheet entrypoints to use @use (#31905)

Previously we would prepend extra content to developer-authored files,
which means adding `@use` in some files would throw an error because
`@use` must be at the top of any compiled file.

Instead, we can ensure any developer-authored files are on the load
path, and then `@import` them into the synthetic entrypoint.

Plugin color_definitions stylesheets are an edge case here, and will
need to be handled separately (or... wait until we move to native css
relative-color syntax, then we can drop color-definition stylesheets
altogether)
This commit is contained in:
David Taylor
2025-03-26 09:15:32 +00:00
committed by GitHub
parent c8ccd4da31
commit b1924c3524
11 changed files with 92 additions and 76 deletions
+19 -3
View File
@@ -836,11 +836,27 @@ class Theme < ActiveRecord::Base
end
def with_scss_load_paths
return yield([]) if self.extra_scss_fields.empty?
ThemeStore::ZipExporter
.new(self)
.with_export_dir(extra_scss_only: true) { |dir| yield ["#{dir}/stylesheets"] }
.with_export_dir(scss_only: true) do |dir|
FileUtils.mkdir_p("#{dir}/_entry_loadpath/theme-entrypoint")
entrypoints = {
"common/common.scss" => "common.scss",
"common/embedded.scss" => "embedded.scss",
"common/color_definitions.scss" => "color_definitions.scss",
"desktop/desktop.scss" => "desktop.scss",
"mobile/mobile.scss" => "mobile.scss",
}
entrypoints.each do |source, destination|
source_path = "#{dir}/#{source}"
destination_path = "#{dir}/_entry_loadpath/theme-entrypoint/#{destination}"
FileUtils.mv(source_path, destination_path) if File.exist?(source_path)
end
yield ["#{dir}/_entry_loadpath", "#{dir}/stylesheets"]
end
end
def scss_variables
+18 -2
View File
@@ -480,12 +480,28 @@ class ThemeField < ActiveRecord::Base
# requests.
end
def scss_entrypoint_name
if name == "scss"
self.target_name
elsif target_name == "common" && name == "color_definitions"
"color_definitions"
elsif target_name == "common" && name == "embedded_scss"
"embedded"
else
raise "Unknown entrypoint for #{target_name}/#{name}"
end
end
def compile_scss(prepended_scss = nil)
prepended_scss ||= Stylesheet::Importer.new({}).prepended_scss
self.theme.with_scss_load_paths do |load_paths|
Stylesheet::Compiler.compile(
"#{prepended_scss} #{self.theme.scss_variables} #{self.value}",
<<~SCSS,
#{prepended_scss}
#{self.theme.scss_variables}
@import \"theme-entrypoint/#{scss_entrypoint_name}\";
SCSS
"#{Theme.targets[self.target_id]}.scss",
theme: self.theme,
load_paths: load_paths,
@@ -501,7 +517,7 @@ class ThemeField < ActiveRecord::Base
# 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]
["/* SCSS compilation error: #{e.message} */", nil]
end
css
end
+1 -1
View File
@@ -785,7 +785,7 @@ class Plugin::Instance
js = "(function(){#{js}})();" if js.present?
result = []
result << [css, "css"] if css.present?
result << [css, "scss"] if css.present?
result << [js, "js"] if js.present?
result.map do |asset, extension|
+10 -7
View File
@@ -9,22 +9,25 @@ module Stylesheet
def self.compile_asset(asset, options = {})
importer = Importer.new(options)
file = importer.prepended_scss
filename = "_#{asset}_entrypoint.scss"
if Importer::THEME_TARGETS.include?(asset.to_s)
filename = "theme_#{options[:theme_id]}.scss"
file += options[:theme_variables].to_s
file += importer.theme_import(asset)
elsif plugin_assets = Importer.plugin_assets[asset.to_s]
filename = "#{asset}.scss"
elsif plugin_asset_info = Importer.plugin_assets[asset.to_s]
options[:load_paths] = [] if options[:load_paths].nil?
plugin_assets = plugin_asset_info[:stylesheets]
plugin_path = plugin_asset_info[:plugin_path]
options[:load_paths] << plugin_path
plugin_assets.each do |src|
file += File.read src
options[:load_paths] << File.expand_path(File.dirname(src))
file += "@import \"#{src}\";\n"
end
else
filename = "#{asset}.scss"
path = "#{ASSET_ROOT}/#{filename}"
file += File.read path
else # Core asset
file += "@import \"#{asset}\";\n"
case asset.to_s
when "embed", "publish"
+8 -24
View File
@@ -27,9 +27,12 @@ module Stylesheet
end
)
plugin_assets[asset_name] = stylesheets[
plugin_directory_name
] if plugin_directory_name.present?
if plugin_directory_name.present?
plugin_assets[asset_name] = {
plugin_path: plugin.path,
stylesheets: stylesheets[plugin_directory_name],
}
end
end
end
end
@@ -112,21 +115,11 @@ module Stylesheet
resolved_ids = Theme.transform_ids(theme_id)
if resolved_ids
theme = Theme.find_by_id(theme_id)
contents << "\n\n// Theme SCSS variables\n\n"
contents << theme&.scss_variables.to_s.split(";").join(";\n") + ";\n\n"
contents << "\n\n"
Theme
.list_baked_fields(resolved_ids, :common, :color_definitions)
.each do |field|
contents << "\n\n// Color definitions from #{field.theme.name}\n\n"
if field.theme_id == theme.id
contents << field.value
else
contents << field.compiled_css(prepended_scss)
end
contents << field.compiled_css(prepended_scss)
contents << "\n\n"
end
end
@@ -198,16 +191,7 @@ module Stylesheet
fields = theme.list_baked_fields(target, attr)
fields.map do |field|
value = field.value
if value.present?
contents << <<~SCSS
// Theme: #{field.theme.name}
// Target: #{field.target_name} #{field.name}
// Last Edited: #{field.updated_at}
SCSS
contents << value
end
contents << "@import \"theme-entrypoint/#{field.scss_entrypoint_name}\";\n"
end
contents
end
+1 -1
View File
@@ -52,7 +52,7 @@ class Stylesheet::Manager::Builder
rescue SassC::SyntaxError, SassC::NotRenderedError, DiscourseJsProcessor::TranspileError => e
if Stylesheet::Importer::THEME_TARGETS.include?(@target.to_s)
# no special errors for theme, handled in theme editor
["", nil]
["/* SCSS compilation error: #{e.message} */", nil]
elsif @target.to_s == Stylesheet::Manager::COLOR_SCHEME_STYLESHEET && Rails.env.production?
# log error but do not crash for errors in color definitions SCSS
Rails.logger.error "SCSS compilation error: #{e.message}"
+3 -3
View File
@@ -35,12 +35,12 @@ class ThemeStore::ZipExporter
private
def export_to_folder(extra_scss_only: false)
def export_to_folder(scss_only: false)
destination_folder = File.join(@temp_folder, @export_name)
FileUtils.mkdir_p(destination_folder)
@theme.theme_fields.each do |field|
next if extra_scss_only && !field.extra_scss_field?
next if scss_only && !(field.extra_scss_field? || field.basic_scss_field?)
next unless path = field.file_path
# Belt and braces approach here. All the user input should already be
@@ -64,7 +64,7 @@ class ThemeStore::ZipExporter
File.write(path, content)
end
if !extra_scss_only
if !scss_only
File.write(
File.join(destination_folder, "about.json"),
JSON.pretty_generate(@theme.generate_metadata_hash),
+10 -8
View File
@@ -42,16 +42,18 @@ RSpec.describe Stylesheet::Compiler do
type_id: ThemeField.types[:scss],
)
end
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,
theme_variables: theme.scss_variables,
)
expect(css).to include(upload.url)
theme.reload.with_scss_load_paths do |load_paths|
css, _map =
Stylesheet::Compiler.compile_asset(
"desktop_theme",
theme_id: theme.id,
theme_variables: theme.scss_variables,
load_paths: load_paths,
)
expect(css).to include(upload.url)
end
end
context "with a plugin" do
+5 -4
View File
@@ -48,14 +48,15 @@ RSpec.describe Stylesheet::Importer do
end
describe "#import_color_definitions" do
let(:scss) { ":root{--custom-color: green}" }
let(:input_scss) { ':root{--custom-color: green;--core-color: #{$primary}}' }
let(:output_scss) { ":root{--custom-color: green;--core-color: #222}" }
let(:scss_child) do
"$navy: #000080; :root{--custom-color: red; --custom-color-rgb: \#{hexToRGB($navy)}}"
end
let(:theme) do
Fabricate(:theme).tap do |t|
t.set_field(target: :common, name: "color_definitions", value: scss)
t.set_field(target: :common, name: "color_definitions", value: input_scss)
t.save!
end
end
@@ -69,7 +70,7 @@ RSpec.describe Stylesheet::Importer do
it "should include color definitions in the theme" do
styles = Stylesheet::Importer.new({ theme_id: theme.id }).import_color_definitions
expect(styles).to include(scss)
expect(styles).to include(output_scss)
end
it "should include color definitions from components" do
@@ -85,7 +86,7 @@ RSpec.describe Stylesheet::Importer do
it "should include default theme color definitions" do
SiteSetting.default_theme_id = theme.id
styles = Stylesheet::Importer.new({}).import_color_definitions
expect(styles).to include(scss)
expect(styles).to include(output_scss)
end
end
+2 -6
View File
@@ -869,12 +869,8 @@ RSpec.describe Stylesheet::Manager do
manager: manager,
)
expect { stylesheet.compile }.to raise_error(Discourse::ScssError)
Rails.env.stubs(:production?).returns(true)
expect(stylesheet.compile).to include(
"/* SCSS compilation error: Error: Undefined variable",
)
expect(stylesheet.compile).to include("--primary:") # core vals preserved
expect(File.read(stylesheet.source_map_fullpath)).to include("/* SCSS compilation error:")
end
it "child theme SCSS includes the default theme's color scheme variables" do
+15 -17
View File
@@ -36,19 +36,19 @@ RSpec.describe ThemeField do
end
it "adds an error when optimized image links are included" do
theme_field = ThemeField.create!(theme_id: 1, target_id: 0, name: "body_tag", value: <<~HTML)
theme_field = theme.set_field(target: :common, name: :body_tag, value: <<~HTML)
<img src="http://mysite.invalid/uploads/default/optimized/1X/6d749a141f513f88f167e750e528515002043da1_2_1282x1000.png"/>
HTML
theme_field.ensure_baked!
expect(theme_field.error).to include(I18n.t("themes.errors.optimized_link"))
theme.save!
expect(theme_field.reload.error).to include(I18n.t("themes.errors.optimized_link"))
theme_field = ThemeField.create!(theme_id: 1, target_id: 0, name: "scss", value: <<~SCSS)
theme_field = theme.set_field(target: :common, name: :scss, value: <<~SCSS)
body {
background: url(http://mysite.invalid/uploads/default/optimized/1X/6d749a141f513f88f167e750e528515002043da1_2_1282x1000.png);
}
SCSS
theme_field.ensure_baked!
expect(theme_field.error).to include(I18n.t("themes.errors.optimized_link"))
theme.save!
expect(theme_field.reload.error).to include(I18n.t("themes.errors.optimized_link"))
theme_field.update(value: <<~SCSS)
body {
@@ -158,19 +158,17 @@ HTML
it "correctly generates errors for transpiled css" do
css = "body {"
field = ThemeField.create!(theme_id: 1, target_id: 0, name: "scss", value: css)
field.ensure_baked!
expect(field.error).not_to eq(nil)
field = theme.set_field(target: :common, name: :scss, value: css)
theme.save!
expect(field.reload.error).to include('Error: expected "}"')
field.value = "@import 'missingfile';"
field.save!
field.ensure_baked!
expect(field.error).to include("Error: Can't find stylesheet to import.")
theme.set_field(target: :common, name: :scss, value: "@import 'missingfile';")
theme.save!
expect(field.reload.error).to include("Error: Can't find stylesheet to import.")
field.value = "body {color: blue};"
field.save!
field.ensure_baked!
expect(field.error).to eq(nil)
theme.set_field(target: :common, name: :scss, value: "body {color: blue};")
theme.save!
expect(field.reload.error).to eq(nil)
end
it "allows importing scss files" do