From be3d6a56ceb6c41918df6ded81c9881bcb1f6407 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 18 Oct 2022 18:20:10 +0100 Subject: [PATCH] DEV: Introduce minification and source maps for Theme JS (#18646) Theme javascript is now minified using Terser, just like our core/plugin JS bundles. This reduces the amount of data sent over the network. This commit also introduces sourcemaps for theme JS. Browser developer tools will now be able show each source file separately when browsing, and also in backtraces. For theme test JS, the sourcemap is inlined for simplicity. Network load is not a concern for tests. --- .../javascripts/discourse-js-processor.js | 28 +++++- .../theme_javascripts_controller.rb | 62 ++++++++---- app/models/javascript_cache.rb | 1 + app/models/theme.rb | 14 ++- app/models/theme_field.rb | 14 +-- config/routes.rb | 1 + ...0550_add_source_map_to_javascript_cache.rb | 7 ++ lib/discourse_js_processor.rb | 18 +++- lib/theme_javascript_compiler.rb | 96 ++++++++++++++++--- spec/lib/discourse_js_processor_spec.rb | 26 +++++ spec/lib/theme_javascript_compiler_spec.rb | 68 +++++++++---- spec/models/theme_field_spec.rb | 22 +++++ spec/models/theme_spec.rb | 3 + .../theme_javascripts_controller_spec.rb | 45 +++++++++ 14 files changed, 338 insertions(+), 67 deletions(-) create mode 100644 db/migrate/20221018100550_add_source_map_to_javascript_cache.rb diff --git a/app/assets/javascripts/discourse-js-processor.js b/app/assets/javascripts/discourse-js-processor.js index 7050fe2ac77..0e038ec87fa 100644 --- a/app/assets/javascripts/discourse-js-processor.js +++ b/app/assets/javascripts/discourse-js-processor.js @@ -1,4 +1,4 @@ -/* global Babel:true */ +/* global Babel:true Terser:true */ // This is executed in mini_racer to provide the JS logic for lib/discourse_js_processor.rb @@ -110,3 +110,29 @@ exports.transpile = function ( throw error; } }; + +// mini_racer doesn't have native support for getting the result of an async operation. +// To work around that, we provide a getMinifyResult which can be used to fetch the result +// in a followup method call. +let lastMinifyError, lastMinifyResult; + +exports.minify = async function (sources, options) { + lastMinifyError = lastMinifyResult = null; + try { + lastMinifyResult = await Terser.minify(sources, options); + } catch (e) { + lastMinifyError = e; + } +}; + +exports.getMinifyResult = function () { + const error = lastMinifyError; + const result = lastMinifyResult; + + lastMinifyError = lastMinifyResult = null; + + if (error) { + throw error; + } + return result; +}; diff --git a/app/controllers/theme_javascripts_controller.rb b/app/controllers/theme_javascripts_controller.rb index 77ad53f05cb..418a076b825 100644 --- a/app/controllers/theme_javascripts_controller.rb +++ b/app/controllers/theme_javascripts_controller.rb @@ -9,10 +9,10 @@ class ThemeJavascriptsController < ApplicationController :preload_json, :redirect_to_login_if_required, :verify_authenticity_token, - only: [:show, :show_tests] + only: [:show, :show_map, :show_tests] ) - before_action :is_asset_path, :no_cookies, :apply_cdn_headers, only: [:show, :show_tests] + before_action :is_asset_path, :no_cookies, :apply_cdn_headers, only: [:show, :show_map, :show_tests] def show raise Discourse::NotFound unless last_modified.present? @@ -21,18 +21,29 @@ class ThemeJavascriptsController < ApplicationController # Security: safe due to route constraint cache_file = "#{DISK_CACHE_PATH}/#{params[:digest]}.js" - unless File.exist?(cache_file) - content = query.pluck_first(:content) - raise Discourse::NotFound if content.nil? - - FileUtils.mkdir_p(DISK_CACHE_PATH) - File.write(cache_file, content) + write_if_not_cached(cache_file) do + content, has_source_map = query.pluck_first(:content, "source_map IS NOT NULL") + if has_source_map + content += "\n//# sourceMappingURL=#{params[:digest]}.map?__ws=#{Discourse.current_hostname}\n" + end + content end - # this is only required for NGINX X-SendFile it seems - response.headers["Content-Length"] = File.size(cache_file).to_s - set_cache_control_headers - send_file(cache_file, disposition: :inline) + serve_file(cache_file) + end + + def show_map + raise Discourse::NotFound unless last_modified.present? + return render body: nil, status: 304 if not_modified? + + # Security: safe due to route constraint + cache_file = "#{DISK_CACHE_PATH}/#{params[:digest]}.map" + + write_if_not_cached(cache_file) do + query.pluck_first(:source_map) + end + + serve_file(cache_file) end def show_tests @@ -48,14 +59,11 @@ class ThemeJavascriptsController < ApplicationController @cache_file = "#{TESTS_DISK_CACHE_PATH}/#{digest}.js" return render body: nil, status: 304 if not_modified? - if !File.exist?(@cache_file) - FileUtils.mkdir_p(TESTS_DISK_CACHE_PATH) - File.write(@cache_file, content) + write_if_not_cached(@cache_file) do + content end - response.headers["Content-Length"] = File.size(@cache_file).to_s - set_cache_control_headers - send_file(@cache_file, disposition: :inline) + serve_file @cache_file end private @@ -94,4 +102,22 @@ class ThemeJavascriptsController < ApplicationController immutable_for(1.year) end end + + def write_if_not_cached(cache_file) + unless File.exist?(cache_file) + content = yield + raise Discourse::NotFound if content.nil? + + FileUtils.mkdir_p(File.dirname(cache_file)) + File.write(cache_file, content) + end + end + + def serve_file(cache_file) + # this is only required for NGINX X-SendFile it seems + response.headers["Content-Length"] = File.size(cache_file).to_s + set_cache_control_headers + type = cache_file.end_with?(".map") ? "application/json" : "text/javascript" + send_file(cache_file, type: type, disposition: :inline) + end end diff --git a/app/models/javascript_cache.rb b/app/models/javascript_cache.rb index 85a31c94c4e..794a4ef612d 100644 --- a/app/models/javascript_cache.rb +++ b/app/models/javascript_cache.rb @@ -41,6 +41,7 @@ end # created_at :datetime not null # updated_at :datetime not null # theme_id :bigint +# source_map :text # # Indexes # diff --git a/app/models/theme.rb b/app/models/theme.rb index 1f9f23e548e..662dc1c890e 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -6,7 +6,7 @@ require 'json_schemer' class Theme < ActiveRecord::Base include GlobalPath - BASE_COMPILER_VERSION = 64 + BASE_COMPILER_VERSION = 65 attr_accessor :child_components @@ -127,7 +127,7 @@ class Theme < ActiveRecord::Base settings_hash = build_settings_hash js_compiler.prepend_settings(settings_hash) if settings_hash.present? javascript_cache || build_javascript_cache - javascript_cache.update!(content: js_compiler.content) + javascript_cache.update!(content: js_compiler.content, source_map: js_compiler.source_map) else javascript_cache&.destroy! end @@ -717,13 +717,17 @@ class Theme < ActiveRecord::Base compiler = ThemeJavascriptCompiler.new(id, name) compiler.append_tree(tests_tree, for_tests: true) - content = compiler.content - - content = <<~JS + content + 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 + content += "\n//# sourceMappingURL=data:application/json;base64,#{Base64.strict_encode64(compiler.source_map)}\n" + end + [content, Digest::SHA1.hexdigest(content)] end diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index 90d9aa9db89..475e3541565 100644 --- a/app/models/theme_field.rb +++ b/app/models/theme_field.rb @@ -95,6 +95,7 @@ class ThemeField < ActiveRecord::Base js_compiler.append_ember_template("discourse/templates/#{name}", hbs_template) end rescue ThemeJavascriptCompiler::CompileError => ex + js_compiler.append_js_error("discourse/templates/#{name}", ex.message) errors << ex.message end @@ -128,25 +129,23 @@ class ThemeField < ActiveRecord::Base js_compiler.append_module(js, "discourse/initializers/#{initializer_name}", include_variables: true) rescue ThemeJavascriptCompiler::CompileError => ex + js_compiler.append_js_error("discourse/initializers/#{initializer_name}", ex.message) errors << ex.message end node.remove end - doc.css('script').each do |node| + doc.css('script').each_with_index do |node, index| next unless inline_javascript?(node) - js_compiler.append_raw_script(node.inner_html) + js_compiler.append_raw_script("_html/#{Theme.targets[self.target_id]}/#{name}_#{index + 1}.js", node.inner_html) node.remove end - errors.each do |error| - js_compiler.append_js_error(error) - end - settings_hash = theme.build_settings_hash - js_compiler.prepend_settings(settings_hash) if js_compiler.content.present? && settings_hash.present? + js_compiler.prepend_settings(settings_hash) if js_compiler.has_content? && settings_hash.present? javascript_cache.content = js_compiler.content + javascript_cache.source_map = js_compiler.source_map javascript_cache.save! doc.add_child("") if javascript_cache.content.present? @@ -239,6 +238,7 @@ class ThemeField < ActiveRecord::Base end javascript_cache.content = js_compiler.content + javascript_cache.source_map = js_compiler.source_map javascript_cache.save! doc = "" doc = "" if javascript_cache.content.present? diff --git a/config/routes.rb b/config/routes.rb index 0b1931aaeb9..ade3bfa2f38 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -561,6 +561,7 @@ Discourse::Application.routes.draw do get "stylesheets/:name.css" => "stylesheets#show", constraints: { name: /[-a-z0-9_]+/ } get "color-scheme-stylesheet/:id(/:theme_id)" => "stylesheets#color_scheme", constraints: { format: :json } get "theme-javascripts/:digest.js" => "theme_javascripts#show", constraints: { digest: /\h{40}/ } + get "theme-javascripts/:digest.map" => "theme_javascripts#show_map", constraints: { digest: /\h{40}/ } get "theme-javascripts/tests/:theme_id-:digest.js" => "theme_javascripts#show_tests" post "uploads/lookup-metadata" => "uploads#metadata" diff --git a/db/migrate/20221018100550_add_source_map_to_javascript_cache.rb b/db/migrate/20221018100550_add_source_map_to_javascript_cache.rb new file mode 100644 index 00000000000..1651e418bd3 --- /dev/null +++ b/db/migrate/20221018100550_add_source_map_to_javascript_cache.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddSourceMapToJavascriptCache < ActiveRecord::Migration[7.0] + def change + add_column :javascript_caches, :source_map, :text + end +end diff --git a/lib/discourse_js_processor.rb b/lib/discourse_js_processor.rb index 9f1d9c756bb..11f1ee27672 100644 --- a/lib/discourse_js_processor.rb +++ b/lib/discourse_js_processor.rb @@ -152,6 +152,10 @@ class DiscourseJsProcessor } JS + # Terser + load_file_in_context(ctx, "node_modules/source-map/dist/source-map.js") + load_file_in_context(ctx, "node_modules/terser/dist/bundle.min.js") + # Template Compiler load_file_in_context(ctx, "node_modules/ember-source/dist/ember-template-compiler.js") load_file_in_context(ctx, "node_modules/babel-plugin-ember-template-compilation/src/plugin.js", wrap_in_module: "babel-plugin-ember-template-compilation/index") @@ -197,6 +201,8 @@ class DiscourseJsProcessor ctx.eval <<~JS globalThis.compileRawTemplate = require('discourse-js-processor').compileRawTemplate; globalThis.transpile = require('discourse-js-processor').transpile; + globalThis.minify = require('discourse-js-processor').minify; + globalThis.getMinifyResult = require('discourse-js-processor').getMinifyResult; JS ctx @@ -219,9 +225,16 @@ class DiscourseJsProcessor @ctx end + # Call a method in the global scope of the v8 context. + # The `fetch_result_call` kwarg provides a workaround for the lack of mini_racer async + # result support. The first call can perform some async operation, and then `fetch_result_call` + # will be called to fetch the result. def self.v8_call(*args, **kwargs) + fetch_result_call = kwargs.delete(:fetch_result_call) mutex.synchronize do - v8.call(*args, **kwargs) + result = v8.call(*args, **kwargs) + result = v8.call(fetch_result_call) if fetch_result_call + result end rescue MiniRacer::RuntimeError => e message = e.message @@ -276,5 +289,8 @@ class DiscourseJsProcessor self.class.v8_call("compileRawTemplate", source, theme_id) end + def terser(tree, opts) + self.class.v8_call("minify", tree, opts, fetch_result_call: "getMinifyResult") + end end end diff --git a/lib/theme_javascript_compiler.rb b/lib/theme_javascript_compiler.rb index 998963ef55d..b278d322c98 100644 --- a/lib/theme_javascript_compiler.rb +++ b/lib/theme_javascript_compiler.rb @@ -7,16 +7,78 @@ class ThemeJavascriptCompiler class CompileError < StandardError end - attr_accessor :content + @@terser_disabled = false + def self.disable_terser! + raise "Tests only" if !Rails.env.test? + @@terser_disabled = true + end + + def self.enable_terser! + raise "Tests only" if !Rails.env.test? + @@terser_disabled = false + end def initialize(theme_id, theme_name) @theme_id = theme_id - @content = +"" + @output_tree = [] @theme_name = theme_name end + def compile! + if !@compiled + @compiled = true + @output_tree.freeze + output = if !has_content? + { "code" => "" } + elsif @@terser_disabled + { "code" => raw_content } + else + DiscourseJsProcessor::Transpiler.new.terser(@output_tree.to_h, terser_config) + end + @content = output["code"] + @source_map = output["map"] + end + [@content, @source_map] + rescue DiscourseJsProcessor::TranspileError => e + message = "[THEME #{@theme_id} '#{@theme_name}'] Compile error: #{e.message}" + @content = "console.error(#{message.to_json});\n" + [@content, @source_map] + end + + def terser_config + # Based on https://github.com/ember-cli/ember-cli-terser/blob/28df3d90a5/index.js#L12-L26 + { + sourceMap: { includeSources: true, root: "theme-#{@theme_id}/" }, + compress: { + negate_iife: false, + sequences: 30, + }, + output: { + semicolons: false, + }, + } + end + + def content + compile! + @content + end + + def source_map + compile! + @source_map + end + + def raw_content + @output_tree.map { |filename, source| source }.join("") + end + + def has_content? + @output_tree.present? + end + def prepend_settings(settings_hash) - @content.prepend <<~JS + @output_tree.prepend ['settings.js', <<~JS] (function() { if ('require' in window) { require("discourse/lib/theme-settings-store").registerSettings(#{@theme_id}, #{settings_hash.to_json}); @@ -90,16 +152,17 @@ class ThemeJavascriptCompiler elsif extension == "hbr" append_raw_template(module_name.sub("discourse/templates/", ""), content) else - append_js_error("unknown file extension '#{extension}' (#{filename})") + append_js_error(filename, "unknown file extension '#{extension}' (#{filename})") end rescue CompileError => e - append_js_error "#{e.message} (#{filename})" + append_js_error filename, "#{e.message} (#{filename})" end end def append_ember_template(name, hbs_template) - name = "/#{name}" if !name.start_with?("/") - module_name = "discourse/theme-#{@theme_id}#{name}" + module_name = name + module_name = "/#{module_name}" if !module_name.start_with?("/") + module_name = "discourse/theme-#{@theme_id}#{module_name}" # Some themes are colocating connector JS under `/connectors`. Move template to /templates to avoid module name clash if (match = COLOCATED_CONNECTOR_REGEX.match(module_name)) && !match[:prefix].end_with?("/templates") @@ -114,7 +177,7 @@ class ThemeJavascriptCompiler JS template_module = DiscourseJsProcessor.transpile(script, "", module_name, theme_id: @theme_id) - content << <<~JS + @output_tree << ["#{name}.js", <<~JS] if ('define' in window) { #{template_module} } @@ -130,8 +193,12 @@ class ThemeJavascriptCompiler def append_raw_template(name, hbs_template) compiled = DiscourseJsProcessor::Transpiler.new.compile_raw_template(hbs_template, theme_id: @theme_id) - @content << <<~JS + source_for_comment = hbs_template.gsub("*/", '*\/').indent(4, ' ') + @output_tree << ["#{name}.js", <<~JS] (function() { + /* + #{source_for_comment} + */ const addRawTemplate = requirejs('discourse-common/lib/raw-templates').addRawTemplate; const template = requirejs('discourse-common/lib/raw-handlebars').template(#{compiled}); addRawTemplate(#{raw_template_name(name)}, template); @@ -141,11 +208,12 @@ class ThemeJavascriptCompiler raise CompileError.new ex.message end - def append_raw_script(script) - @content << script + "\n" + def append_raw_script(filename, script) + @output_tree << [filename, script + "\n"] end def append_module(script, name, include_variables: true) + original_filename = name name = "discourse/theme-#{@theme_id}/#{name.gsub(/^discourse\//, '')}" # Some themes are colocating connector JS under `/templates/connectors`. Move out of templates to avoid module name clash @@ -155,7 +223,7 @@ class ThemeJavascriptCompiler script = "#{theme_settings}#{script}" if include_variables transpiler = DiscourseJsProcessor::Transpiler.new - @content << <<~JS + @output_tree << ["#{original_filename}.js", <<~JS] if ('define' in window) { #{transpiler.perform(script, "", name).strip} } @@ -164,9 +232,9 @@ class ThemeJavascriptCompiler raise CompileError.new ex.message end - def append_js_error(message) + def append_js_error(filename, message) message = "[THEME #{@theme_id} '#{@theme_name}'] Compile error: #{message}" - append_raw_script "console.error(#{message.to_json});" + append_raw_script filename, "console.error(#{message.to_json});" end private diff --git a/spec/lib/discourse_js_processor_spec.rb b/spec/lib/discourse_js_processor_spec.rb index 1fb2b64321f..1d87b5a85bd 100644 --- a/spec/lib/discourse_js_processor_spec.rb +++ b/spec/lib/discourse_js_processor_spec.rb @@ -185,4 +185,30 @@ RSpec.describe DiscourseJsProcessor do ) end end + + describe "Transpiler#terser" do + it "can minify code and provide sourcemaps" do + sources = { + "multiply.js" => "let multiply = (firstValue, secondValue) => firstValue * secondValue;", + "add.js" => "let add = (firstValue, secondValue) => firstValue + secondValue;" + } + + result = DiscourseJsProcessor::Transpiler.new.terser(sources, { sourceMap: { includeSources: true } }) + expect(result.keys).to contain_exactly("code", "map") + + begin + # Check the code still works + ctx = MiniRacer::Context.new + ctx.eval(result["code"]) + expect(ctx.eval("multiply(2, 3)")).to eq(6) + expect(ctx.eval("add(2, 3)")).to eq(5) + ensure + ctx.dispose + end + + map = JSON.parse(result["map"]) + expect(map["sources"]).to contain_exactly(*sources.keys) + expect(map["sourcesContent"]).to contain_exactly(*sources.values) + end + end end diff --git a/spec/lib/theme_javascript_compiler_spec.rb b/spec/lib/theme_javascript_compiler_spec.rb index 46fa810ee55..e530d3ef85e 100644 --- a/spec/lib/theme_javascript_compiler_spec.rb +++ b/spec/lib/theme_javascript_compiler_spec.rb @@ -8,28 +8,28 @@ RSpec.describe ThemeJavascriptCompiler do template = "

hello

" name = "/path/to/templates1" compiler.append_raw_template("#{name}.raw", template) - expect(compiler.content.to_s).to include("addRawTemplate(\"#{name}\"") + expect(compiler.raw_content.to_s).to include("addRawTemplate(\"#{name}\"") name = "/path/to/templates2" compiler.append_raw_template("#{name}.hbr", template) - expect(compiler.content.to_s).to include("addRawTemplate(\"#{name}\"") + expect(compiler.raw_content.to_s).to include("addRawTemplate(\"#{name}\"") name = "/path/to/templates3" compiler.append_raw_template("#{name}.hbs", template) - expect(compiler.content.to_s).to include("addRawTemplate(\"#{name}.hbs\"") + expect(compiler.raw_content.to_s).to include("addRawTemplate(\"#{name}.hbs\"") end end describe "#append_ember_template" do it 'maintains module names so that discourse-boot.js can correct them' do compiler.append_ember_template("/connectors/blah-1", "{{var}}") - expect(compiler.content.to_s).to include("define(\"discourse/theme-1/connectors/blah-1\", [\"exports\", \"@ember/template-factory\"]") + expect(compiler.raw_content.to_s).to include("define(\"discourse/theme-1/connectors/blah-1\", [\"exports\", \"@ember/template-factory\"]") compiler.append_ember_template("connectors/blah-2", "{{var}}") - expect(compiler.content.to_s).to include("define(\"discourse/theme-1/connectors/blah-2\", [\"exports\", \"@ember/template-factory\"]") + expect(compiler.raw_content.to_s).to include("define(\"discourse/theme-1/connectors/blah-2\", [\"exports\", \"@ember/template-factory\"]") compiler.append_ember_template("javascripts/connectors/blah-3", "{{var}}") - expect(compiler.content.to_s).to include("define(\"discourse/theme-1/javascripts/connectors/blah-3\", [\"exports\", \"@ember/template-factory\"]") + expect(compiler.raw_content.to_s).to include("define(\"discourse/theme-1/javascripts/connectors/blah-3\", [\"exports\", \"@ember/template-factory\"]") end end @@ -39,22 +39,22 @@ RSpec.describe ThemeJavascriptCompiler do compiler = ThemeJavascriptCompiler.new(1, 'marks') compiler.append_ember_template("connectors/outlet/blah-1", "{{var}}") compiler.append_module("console.log('test')", "connectors/outlet/blah-1") - expect(compiler.content.to_s).to include("discourse/theme-1/connectors/outlet/blah-1") - expect(compiler.content.to_s).to include("discourse/theme-1/templates/connectors/outlet/blah-1") + expect(compiler.raw_content.to_s).to include("discourse/theme-1/connectors/outlet/blah-1") + expect(compiler.raw_content.to_s).to include("discourse/theme-1/templates/connectors/outlet/blah-1") # Colocated under `/templates/connectors` compiler = ThemeJavascriptCompiler.new(1, 'marks') compiler.append_ember_template("templates/connectors/outlet/blah-1", "{{var}}") compiler.append_module("console.log('test')", "templates/connectors/outlet/blah-1") - expect(compiler.content.to_s).to include("discourse/theme-1/connectors/outlet/blah-1") - expect(compiler.content.to_s).to include("discourse/theme-1/templates/connectors/outlet/blah-1") + expect(compiler.raw_content.to_s).to include("discourse/theme-1/connectors/outlet/blah-1") + expect(compiler.raw_content.to_s).to include("discourse/theme-1/templates/connectors/outlet/blah-1") # Not colocated compiler = ThemeJavascriptCompiler.new(1, 'marks') compiler.append_ember_template("templates/connectors/outlet/blah-1", "{{var}}") compiler.append_module("console.log('test')", "connectors/outlet/blah-1") - expect(compiler.content.to_s).to include("discourse/theme-1/connectors/outlet/blah-1") - expect(compiler.content.to_s).to include("discourse/theme-1/templates/connectors/outlet/blah-1") + expect(compiler.raw_content.to_s).to include("discourse/theme-1/connectors/outlet/blah-1") + expect(compiler.raw_content.to_s).to include("discourse/theme-1/templates/connectors/outlet/blah-1") end end @@ -83,8 +83,8 @@ RSpec.describe ThemeJavascriptCompiler do "discourse/templates/components/mycomponent.hbs" => "{{my-component-template}}" } ) - expect(compiler.content).to include('define("discourse/theme-1/components/mycomponent"') - expect(compiler.content).to include('define("discourse/theme-1/discourse/templates/components/mycomponent"') + expect(compiler.raw_content).to include('define("discourse/theme-1/components/mycomponent"') + expect(compiler.raw_content).to include('define("discourse/theme-1/discourse/templates/components/mycomponent"') end it "handles colocated components" do @@ -97,8 +97,8 @@ RSpec.describe ThemeJavascriptCompiler do "discourse/components/mycomponent.hbs" => "{{my-component-template}}" } ) - expect(compiler.content).to include("__COLOCATED_TEMPLATE__ =") - expect(compiler.content).to include("setComponentTemplate") + expect(compiler.raw_content).to include("__COLOCATED_TEMPLATE__ =") + expect(compiler.raw_content).to include("setComponentTemplate") end it "prints error when default export missing" do @@ -111,8 +111,8 @@ RSpec.describe ThemeJavascriptCompiler do "discourse/components/mycomponent.hbs" => "{{my-component-template}}" } ) - expect(compiler.content).to include("__COLOCATED_TEMPLATE__ =") - expect(compiler.content).to include("throw new Error") + expect(compiler.raw_content).to include("__COLOCATED_TEMPLATE__ =") + expect(compiler.raw_content).to include("throw new Error") end it "handles template-only components" do @@ -121,9 +121,35 @@ RSpec.describe ThemeJavascriptCompiler do "discourse/components/mycomponent.hbs" => "{{my-component-template}}" } ) - expect(compiler.content).to include("__COLOCATED_TEMPLATE__ =") - expect(compiler.content).to include("setComponentTemplate") - expect(compiler.content).to include("@ember/component/template-only") + expect(compiler.raw_content).to include("__COLOCATED_TEMPLATE__ =") + expect(compiler.raw_content).to include("setComponentTemplate") + expect(compiler.raw_content).to include("@ember/component/template-only") + end + end + + describe "terser compilation" do + it "applies terser and provides sourcemaps" do + sources = { + "multiply.js" => "let multiply = (firstValue, secondValue) => firstValue * secondValue;", + "add.js" => "let add = (firstValue, secondValue) => firstValue + secondValue;" + } + + compiler.append_tree(sources) + + expect(compiler.content).to include("multiply") + expect(compiler.content).to include("add") + + map = JSON.parse(compiler.source_map) + expect(map["sources"]).to contain_exactly(*sources.keys) + expect(map["sourcesContent"].to_s).to include("let multiply") + expect(map["sourcesContent"].to_s).to include("let add") + expect(map["sourceRoot"]).to eq("theme-1/") + end + + it "handles invalid JS" do + compiler.append_raw_script("filename.js", "if(someCondition") + expect(compiler.content).to include('console.error("[THEME 1') + expect(compiler.content).to include('Unexpected token') end end end diff --git a/spec/models/theme_field_spec.rb b/spec/models/theme_field_spec.rb index 931bc45bf72..5a14bca2fa6 100644 --- a/spec/models/theme_field_spec.rb +++ b/spec/models/theme_field_spec.rb @@ -3,6 +3,8 @@ RSpec.describe ThemeField do fab!(:theme) { Fabricate(:theme) } + before { ThemeJavascriptCompiler.disable_terser! } + after { ThemeJavascriptCompiler.enable_terser! } describe "scope: find_by_theme_ids" do it "returns result in the specified order" do @@ -194,6 +196,26 @@ HTML expect(theme.javascript_cache.content).to include("define(\"discourse/theme-#{theme.id}/controllers/discovery-2\"") expect(theme.javascript_cache.content).to include("const settings =") expect(theme.javascript_cache.content).to include("[THEME #{theme.id} '#{theme.name}'] Compile error: unknown file extension 'blah' (discourse/controllers/discovery.blah)") + + # Check sourcemap + expect(theme.javascript_cache.source_map).to eq(nil) + ThemeJavascriptCompiler.enable_terser! + js_field.update(compiler_version: "0") + theme.save! + + expect(theme.javascript_cache.source_map).not_to eq(nil) + map = JSON.parse(theme.javascript_cache.source_map) + + expect(map["sources"]).to contain_exactly( + "discourse/controllers/discovery-2.js", + "discourse/controllers/discovery.blah", + "discourse/controllers/discovery.js", + "discourse/templates/discovery.js", + "discovery.js", + "other_discovery.js" + ) + expect(map["sourceRoot"]).to eq("theme-#{theme.id}/") + expect(map["sourcesContent"].length).to eq(6) end def create_upload_theme_field!(name) diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index cf2bc2514fb..8d513ee005a 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -5,6 +5,9 @@ RSpec.describe Theme do Theme.clear_cache! end + before { ThemeJavascriptCompiler.disable_terser! } + after { ThemeJavascriptCompiler.enable_terser! } + fab! :user do Fabricate(:user) end diff --git a/spec/requests/theme_javascripts_controller_spec.rb b/spec/requests/theme_javascripts_controller_spec.rb index aaa24753a43..0f2c58ce584 100644 --- a/spec/requests/theme_javascripts_controller_spec.rb +++ b/spec/requests/theme_javascripts_controller_spec.rb @@ -2,6 +2,9 @@ RSpec.describe ThemeJavascriptsController do include ActiveSupport::Testing::TimeHelpers + before { ThemeJavascriptCompiler.disable_terser! } + after { ThemeJavascriptCompiler.enable_terser! } + def clear_disk_cache if Dir.exist?(ThemeJavascriptsController::DISK_CACHE_PATH) `rm -rf #{ThemeJavascriptsController::DISK_CACHE_PATH}` @@ -55,6 +58,40 @@ RSpec.describe ThemeJavascriptsController do get "/theme-javascripts/#{javascript_cache.digest}.js" expect(response.status).to eq(404) end + + it "adds sourceMappingUrl if there is a source map" do + digest = SecureRandom.hex(20) + javascript_cache.update(digest: digest) + get "/theme-javascripts/#{digest}.js" + expect(response.status).to eq(200) + expect(response.body).to eq('console.log("hello");') + + digest = SecureRandom.hex(20) + javascript_cache.update(digest: digest, source_map: '{fakeSourceMap: true}') + get "/theme-javascripts/#{digest}.js" + expect(response.status).to eq(200) + expect(response.body).to eq <<~JS + console.log("hello"); + //# sourceMappingURL=#{digest}.map?__ws=test.localhost + JS + end + end + + describe "#show_map" do + it "returns a source map when present" do + get "/theme-javascripts/#{javascript_cache.digest}.map" + expect(response.status).to eq(404) + + digest = SecureRandom.hex(20) + javascript_cache.update(digest: digest, source_map: '{fakeSourceMap: true}') + get "/theme-javascripts/#{digest}.map" + expect(response.status).to eq(200) + expect(response.body).to eq("{fakeSourceMap: true}") + + javascript_cache.destroy + get "/theme-javascripts/#{digest}.map" + expect(response.status).to eq(404) + end end describe "#show_tests" do @@ -133,5 +170,13 @@ 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