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 = "