From 22614ca85b99f9b47447a1170357e7c0042fc455 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 16 Jan 2024 09:50:44 +0800 Subject: [PATCH] DEV: Compile theme migrations javascript files when running theme qunit (#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. --- app/models/theme.rb | 27 ++++++++++++++----- lib/theme_javascript_compiler.rb | 14 ++++++---- spec/models/theme_spec.rb | 17 ++++++++++++ .../theme_javascripts_controller_spec.rb | 8 ------ 4 files changed, 46 insertions(+), 20 deletions(-) diff --git a/app/models/theme.rb b/app/models/theme.rb index 94c89d7f6d9..6fd63b03940 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -913,21 +913,27 @@ class Theme < ActiveRecord::Base 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 @@ -942,6 +948,13 @@ class Theme < ActiveRecord::Base 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});" diff --git a/lib/theme_javascript_compiler.rb b/lib/theme_javascript_compiler.rb index 334a638fa7d..1a9d91f42e6 100644 --- a/lib/theme_javascript_compiler.rb +++ b/lib/theme_javascript_compiler.rb @@ -18,24 +18,27 @@ class ThemeJavascriptCompiler @@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 @@ -92,7 +95,7 @@ class ThemeJavascriptCompiler 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" @@ -168,9 +171,9 @@ class ThemeJavascriptCompiler # 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" @@ -238,6 +241,7 @@ class ThemeJavascriptCompiler 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} diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 955f89dc263..43e225079bd 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -916,12 +916,14 @@ HTML 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 @@ -930,6 +932,21 @@ HTML 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 diff --git a/spec/requests/theme_javascripts_controller_spec.rb b/spec/requests/theme_javascripts_controller_spec.rb index 093c33fb9b7..e221a305cbf 100644 --- a/spec/requests/theme_javascripts_controller_spec.rb +++ b/spec/requests/theme_javascripts_controller_spec.rb @@ -229,13 +229,5 @@ RSpec.describe ThemeJavascriptsController do 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