From 476bd1d23704500206bcba1c2db1671d935e6958 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 11 May 2022 10:23:32 +0100 Subject: [PATCH] DEV: Fix production sourcemaps with Ember CLI (#16707) 22a7905f restructured how we load Ember CLI assets in production. Unfortunately, it also broke sourcemaps for those assets. This commit fixes that regression via a couple of changes: - It adds the necessary `.map` paths to `config.assets.precompile` - It swaps Sprockets' default `SourcemappingUrlProcessor` with an extended version which maintains relative URLs of maps --- config/application.rb | 3 ++ config/initializers/assets.rb | 1 + lib/discourse_sourcemapping_url_processor.rb | 15 +++++++++ lib/ember_cli.rb | 14 +++++++++ lib/tasks/assets.rake | 4 +-- ...course_sourcemapping_url_processor_spec.rb | 31 +++++++++++++++++++ 6 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 lib/discourse_sourcemapping_url_processor.rb create mode 100644 spec/lib/discourse_sourcemapping_url_processor_spec.rb diff --git a/config/application.rb b/config/application.rb index 6a60d8d95e4..48abdb168f8 100644 --- a/config/application.rb +++ b/config/application.rb @@ -176,6 +176,7 @@ module Discourse Sprockets.register_transformer 'text/x-handlebars', 'application/javascript', Ember::Handlebars::Template require 'discourse_js_processor' + require 'discourse_sourcemapping_url_processor' Sprockets.register_mime_type 'application/javascript', extensions: ['.js', '.es6', '.js.es6'], charset: :unicode Sprockets.register_postprocessor 'application/javascript', DiscourseJsProcessor @@ -184,6 +185,8 @@ module Discourse Discourse::Application.initializer :prepend_ember_assets do |app| # Needs to be in its own initializer so it runs after the append_assets_path initializer defined by Sprockets app.config.assets.paths.unshift "#{app.config.root}/app/assets/javascripts/discourse/dist/assets" + Sprockets.unregister_postprocessor 'application/javascript', Sprockets::Rails::SourcemappingUrlProcessor + Sprockets.register_postprocessor 'application/javascript', DiscourseSourcemappingUrlProcessor end end diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb index 306dd044ab0..69f6c4aa7be 100644 --- a/config/initializers/assets.rb +++ b/config/initializers/assets.rb @@ -61,6 +61,7 @@ if EmberCli.enabled? scripts/discourse-test-listen-boot scripts/discourse-boot } + Rails.application.config.assets.precompile += EmberCli::ASSETS.map { |name| name.sub('.js', '.map') } else Rails.application.config.assets.precompile += %w{ application.js diff --git a/lib/discourse_sourcemapping_url_processor.rb b/lib/discourse_sourcemapping_url_processor.rb new file mode 100644 index 00000000000..51c3ba21b2a --- /dev/null +++ b/lib/discourse_sourcemapping_url_processor.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +# This postprocessor rewrites `//sourceMappingURL=` comments to include the hashed filename of the map. +# As a side effect, the default implementation also replaces relative sourcemap URLs with absolute URLs, including the CDN domain. +# We want to preserve the relative nature of the URLs, so that compiled JS is portable across sites with differing CDN configurations. +class DiscourseSourcemappingUrlProcessor < Sprockets::Rails::SourcemappingUrlProcessor + def self.sourcemap_asset_path(sourcemap_logical_path, context:) + result = super(sourcemap_logical_path, context: context) + if File.basename(sourcemap_logical_path) === sourcemap_logical_path + # If the original sourcemap reference is relative, keep it relative + result = File.basename(result) + end + result + end +end diff --git a/lib/ember_cli.rb b/lib/ember_cli.rb index c153b1cb54b..de6d5ef310c 100644 --- a/lib/ember_cli.rb +++ b/lib/ember_cli.rb @@ -1,6 +1,15 @@ # frozen_string_literal: true module EmberCli + ASSETS = %w( + discourse.js + admin.js + ember_jquery.js + pretty-text-bundle.js + start-discourse.js + vendor.js + ) + ALIASES ||= { "application" => "discourse", "discourse/tests/test-support-rails" => "test-support", @@ -36,4 +45,9 @@ module EmberCli return name if !enabled? ALIASES[name] || name end + + def self.is_ember_cli_asset?(name) + return false if !enabled? + ASSETS.include?(name) || name.start_with?("chunk.") + end end diff --git a/lib/tasks/assets.rake b/lib/tasks/assets.rake index fec553c7fbb..55b153f28d9 100644 --- a/lib/tasks/assets.rake +++ b/lib/tasks/assets.rake @@ -54,7 +54,7 @@ task 'assets:precompile:before' do if EMBER_CLI # Add ember cli chunks Rails.configuration.assets.precompile.push( - *EmberCli.script_chunks.values.flatten + *EmberCli.script_chunks.values.flatten.flat_map { |name| ["#{name}.js", "#{name}.map"] } ) end end @@ -189,7 +189,7 @@ end def max_compress?(path, locales) return false if Rails.configuration.assets.skip_minification.include? path - return false if is_ember_cli_asset?(path) + return false if EmberCli.is_ember_cli_asset?(path) return true unless path.include? "locales/" path_locale = path.delete_prefix("locales/").delete_suffix(".js") diff --git a/spec/lib/discourse_sourcemapping_url_processor_spec.rb b/spec/lib/discourse_sourcemapping_url_processor_spec.rb new file mode 100644 index 00000000000..1c348c42ea7 --- /dev/null +++ b/spec/lib/discourse_sourcemapping_url_processor_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'discourse_sourcemapping_url_processor' + +describe DiscourseSourcemappingUrlProcessor do + def process(input) + env = Sprockets::Environment.new + env.context_class.class_eval do + def resolve(path, **kargs) + "/assets/mapped.js.map" + end + + def asset_path(path, options = {}) + "/assets/mapped-HEXGOESHERE.js.map" + end + end + + input = { environment: env, data: input, name: 'mapped', filename: 'mapped.js', metadata: {} } + DiscourseSourcemappingUrlProcessor.call(input)[:data] + end + + it 'maintains relative paths' do + output = process "var mapped;\n//# sourceMappingURL=mapped.js.map" + expect(output).to eq("var mapped;\n//# sourceMappingURL=mapped-HEXGOESHERE.js.map\n//!\n") + end + + it 'uses default behaviour for non-adjacent relative paths' do + output = process "var mapped;\n//# sourceMappingURL=/assets/mapped.js.map" + expect(output).to eq("var mapped;\n//# sourceMappingURL=/assets/mapped-HEXGOESHERE.js.map\n//!\n") + end +end