From 07893779dfe1500bbff3e34b8837e9b8acdb6918 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 14 Feb 2022 12:47:56 +0000 Subject: [PATCH] DEV: Correct service-worker sourceMappingURL (#15916) We serve `service-worker.js` in an unusual way, which means that the sourcemap is not available on an adjacent path. This means that the browser fails to fetch the map, and shows an error in the console. This commit re-writes the source map reference in the static_controller to be an absolute link to the asset (including the appropriate CDN, if enabled), and adds a spec for the behavior. It's important to do this at runtime, rather than JS precompile time, so that changes to CDN configuration do not require re-compilation to take effect. --- app/controllers/static_controller.rb | 6 +++++- config/routes.rb | 4 +--- spec/requests/static_controller_spec.rb | 23 +++++++++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/app/controllers/static_controller.rb b/app/controllers/static_controller.rb index 6db0adabf81..5a509932230 100644 --- a/app/controllers/static_controller.rb +++ b/app/controllers/static_controller.rb @@ -203,8 +203,12 @@ class StaticController < ApplicationController path = File.expand_path(Rails.root + "public/assets/#{Rails.application.assets_manifest.assets['service-worker.js']}") response.headers["Last-Modified"] = File.ctime(path).httpdate end + content = Rails.application.assets_manifest.find_sources('service-worker.js').first + content = content.sub( + /^\/\/# sourceMappingURL=(service-worker-.+\.map)$/ + ) { "//# sourceMappingURL=#{helpers.script_asset_path(Regexp.last_match(1))}" } render( - plain: Rails.application.assets_manifest.find_sources('service-worker.js').first, + plain: content, content_type: 'application/javascript' ) end diff --git a/config/routes.rb b/config/routes.rb index 641a9057655..f2733e9abe5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -890,16 +890,14 @@ Discourse::Application.routes.draw do resources :drafts, only: [:index, :create, :show, :destroy] + get "/service-worker.js" => "static#service_worker_asset", format: :js if service_worker_asset = Rails.application.assets_manifest.assets['service-worker.js'] # https://developers.google.com/web/fundamentals/codelabs/debugging-service-workers/ # Normally the browser will wait until a user closes all tabs that contain the # current site before updating to a new Service Worker. # Support the old Service Worker path to avoid routing error filling up the # logs. - get "/service-worker.js" => "static#service_worker_asset", format: :js get service_worker_asset => "static#service_worker_asset", format: :js - elsif Rails.env.development? - get "/service-worker.js" => "static#service_worker_asset", format: :js end get "cdn_asset/:site/*path" => "static#cdn_asset", format: false, constraints: { format: /.*/ } diff --git a/spec/requests/static_controller_spec.rb b/spec/requests/static_controller_spec.rb index 406a05a3d16..df20b714fab 100644 --- a/spec/requests/static_controller_spec.rb +++ b/spec/requests/static_controller_spec.rb @@ -404,4 +404,27 @@ describe StaticController do end end end + + describe "#service_worker_asset" do + it "works" do + get "/service-worker.js" + expect(response.status).to eq(200) + expect(response.content_type).to start_with("application/javascript") + expect(response.body).to include("workbox") + end + + it "replaces sourcemap URL" do + Rails.application.assets_manifest.stubs(:find_sources).with("service-worker.js").returns([ + <<~JS + someFakeServiceWorkerSource(); + //# sourceMappingURL=service-worker-abcde.js.map + JS + ]) + + get "/service-worker.js" + expect(response.status).to eq(200) + expect(response.content_type).to start_with("application/javascript") + expect(response.body).to include("sourceMappingURL=/assets/service-worker-abcde.js.map") + end + end end