Skip to content

Commit

Permalink
DEV: Compile theme migrations javascript files when running theme qun…
Browse files Browse the repository at this point in the history
…it (discourse#25219)

Why this change?

Currently, is it hard to iteratively write a theme settings migrations
because our theme migrations system does not rollback. Therefore, we
want to allow theme developers to be able to write QUnit tests for their
theme migrations files enabling them to iteratively write their theme
migrations.

What does this change do?

1. Update `Theme#baked_js_tests_with_digest` to include all `ThemeField`
records of `ThemeField#target` equal to `migrations`. Note that we do
not include the `settings` and `themePrefix` variables for migration files.

2. Don't minify JavaScript test files becasue it makes debugging in
   development hard.
  • Loading branch information
tgxworld authored Jan 16, 2024
1 parent fd7c6f1 commit 22614ca
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 20 deletions.
27 changes: 20 additions & 7 deletions app/models/theme.rb
Original file line number Diff line number Diff line change
Expand Up @@ -913,21 +913,27 @@ def convert_list_to_json_schema(setting_row, setting)

def baked_js_tests_with_digest
tests_tree =
theme_fields
.where(target_id: Theme.targets[:tests_js])
.order(name: :asc)
.pluck(:name, :value)
.to_h
theme_fields_to_tree(
theme_fields.where(target_id: Theme.targets[:tests_js]).order(name: :asc),
)

return nil, nil if tests_tree.blank?

compiler = ThemeJavascriptCompiler.new(id, name)
compiler.append_tree(tests_tree, for_tests: true)
migrations_tree =
theme_fields_to_tree(
theme_fields.where(target_id: Theme.targets[:migrations]).order(name: :asc),
)

compiler = ThemeJavascriptCompiler.new(id, name, minify: false)
compiler.append_tree(migrations_tree, include_variables: false)
compiler.append_tree(tests_tree)

compiler.append_raw_script "test_setup.js", <<~JS
(function() {
require("discourse/lib/theme-settings-store").registerSettings(#{self.id}, #{cached_default_settings.to_json}, { force: true });
})();
JS

content = compiler.content

if compiler.source_map
Expand All @@ -942,6 +948,13 @@ def baked_js_tests_with_digest

attr_accessor :theme_setting_requests_refresh

def theme_fields_to_tree(theme_fields_scope)
theme_fields_scope.reduce({}) do |tree, theme_field|
tree[theme_field.file_path] = theme_field.value
tree
end
end

def to_scss_variable(name, value)
escaped = SassC::Script::Value::String.quote(value.to_s, sass: true)
"$#{name}: unquote(#{escaped});"
Expand Down
14 changes: 9 additions & 5 deletions lib/theme_javascript_compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,27 @@ def self.enable_terser!
@@terser_disabled = false
end

def initialize(theme_id, theme_name)
def initialize(theme_id, theme_name, minify: true)
@theme_id = theme_id
@output_tree = []
@theme_name = theme_name
@minify = minify
end

def compile!
if !@compiled
@compiled = true
@output_tree.freeze

output =
if !has_content?
{ "code" => "" }
elsif @@terser_disabled
elsif @@terser_disabled || !@minify
{ "code" => raw_content }
else
DiscourseJsProcessor::Transpiler.new.terser(@output_tree.to_h, terser_config)
end

@content = output["code"]
@source_map = output["map"]
end
Expand Down Expand Up @@ -92,7 +95,7 @@ def prepend_settings(settings_hash)
JS
end

def append_tree(tree, for_tests: false)
def append_tree(tree, include_variables: true)
# Replace legacy extensions
tree.transform_keys! do |filename|
if filename.ends_with? ".js.es6"
Expand Down Expand Up @@ -168,9 +171,9 @@ def append_tree(tree, for_tests: false)
# Transpile and write to output
tree.each_pair do |filename, content|
module_name, extension = filename.split(".", 2)
module_name = "test/#{module_name}" if for_tests

if extension == "js" || extension == "gjs"
append_module(content, module_name, extension)
append_module(content, module_name, extension, include_variables:)
elsif extension == "hbs"
append_ember_template(module_name, content)
elsif extension == "hbr"
Expand Down Expand Up @@ -238,6 +241,7 @@ def append_module(script, name, extension, include_variables: true)

script = "#{theme_settings}#{script}" if include_variables
transpiler = DiscourseJsProcessor::Transpiler.new

@output_tree << ["#{original_filename}.#{extension}", <<~JS]
if ('define' in window) {
#{transpiler.perform(script, "", name, theme_id: @theme_id, extension: extension).strip}
Expand Down
17 changes: 17 additions & 0 deletions spec/models/theme_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -916,12 +916,14 @@ def included_settings(id)
name: "yaml",
value: "some_number: 1",
)

theme.set_field(
target: :tests_js,
type: :js,
name: "acceptance/some-test.js",
value: "assert.ok(true);",
)

theme.save!
end

Expand All @@ -930,6 +932,21 @@ def included_settings(id)
expect(theme.baked_js_tests_with_digest).to eq([nil, nil])
end

it "includes theme's migrations theme fields" do
theme.set_field(
target: :migrations,
type: :js,
name: "0001-some-migration",
value: "export default function migrate(settings) { return settings; }",
)

theme.save!

content, _digest = theme.baked_js_tests_with_digest

expect(content).to include("function migrate(settings)")
end

it "digest does not change when settings are changed" do
content, digest = theme.baked_js_tests_with_digest
expect(content).to be_present
Expand Down
8 changes: 0 additions & 8 deletions spec/requests/theme_javascripts_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,5 @@ def update_digest_and_get(digest)
expect(response.status).to eq(200)
expect(response.body).to include("assert.ok(343434);")
end

it "includes inline sourcemap" do
ThemeJavascriptCompiler.enable_terser!
content, digest = component.baked_js_tests_with_digest
get "/theme-javascripts/tests/#{component.id}-#{digest}.js"
expect(response.status).to eq(200)
expect(response.body).to include("//# sourceMappingURL=data:application/json;base64,")
end
end
end

0 comments on commit 22614ca

Please sign in to comment.