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.
This commit is contained in:
David Taylor 2022-02-14 12:47:56 +00:00 committed by GitHub
parent 6ab4d26d84
commit 07893779df
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 29 additions and 4 deletions

View File

@ -203,8 +203,12 @@ class StaticController < ApplicationController
path = File.expand_path(Rails.root + "public/assets/#{Rails.application.assets_manifest.assets['service-worker.js']}") path = File.expand_path(Rails.root + "public/assets/#{Rails.application.assets_manifest.assets['service-worker.js']}")
response.headers["Last-Modified"] = File.ctime(path).httpdate response.headers["Last-Modified"] = File.ctime(path).httpdate
end 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( render(
plain: Rails.application.assets_manifest.find_sources('service-worker.js').first, plain: content,
content_type: 'application/javascript' content_type: 'application/javascript'
) )
end end

View File

@ -890,16 +890,14 @@ Discourse::Application.routes.draw do
resources :drafts, only: [:index, :create, :show, :destroy] 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'] if service_worker_asset = Rails.application.assets_manifest.assets['service-worker.js']
# https://developers.google.com/web/fundamentals/codelabs/debugging-service-workers/ # https://developers.google.com/web/fundamentals/codelabs/debugging-service-workers/
# Normally the browser will wait until a user closes all tabs that contain the # Normally the browser will wait until a user closes all tabs that contain the
# current site before updating to a new Service Worker. # current site before updating to a new Service Worker.
# Support the old Service Worker path to avoid routing error filling up the # Support the old Service Worker path to avoid routing error filling up the
# logs. # logs.
get "/service-worker.js" => "static#service_worker_asset", format: :js
get service_worker_asset => "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 end
get "cdn_asset/:site/*path" => "static#cdn_asset", format: false, constraints: { format: /.*/ } get "cdn_asset/:site/*path" => "static#cdn_asset", format: false, constraints: { format: /.*/ }

View File

@ -404,4 +404,27 @@ describe StaticController do
end end
end 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 end