propshaft icon indicating copy to clipboard operation
propshaft copied to clipboard

Add service-worker-allowed header by default

Open fauresebast opened this issue 1 year ago • 7 comments

If you need to custom you response header not only for standard web requests but also for your assets, these changes allow you to specify them in your environment file

fauresebast avatar Sep 05 '23 13:09 fauresebast

Hey @fauresebast, what would be the use case for having custom headers in asset responses and what headers would those be? Maybe we could add some of those as defaults?

brenogazzola avatar Sep 05 '23 16:09 brenogazzola

Hello @brenogazzola, I found this to be necessary when working with service workers. On development setup, when delivered via propshaft, they will be under /assets/ path but if you register the service worker from / you'll get an error as the scope doesn't match.

Uncaught (in promise) DOMException: Failed to register a ServiceWorker for scope ('http://localhost:3000/') with script ('http://localhost:3000/assets/sw-v0000001.digested.js'): The path of the provided scope ('/') is not under the max scope allowed ('/assets/'). Adjust the scope, move the Service Worker script, or use the Service-Worker-Allowed HTTP header to allow the scope.

On production this isn't an issue as you can configure nginx to add the Service-Worker-Allowed header or if you serve static assets via rails you can use config.public_file_server.headers

I have a similar change on a fork to bypass this limitation: https://github.com/ylecuyer/propshaft/commit/55ea5f346c63fe0e09559796e6da6c257cdc8a65

ylecuyer avatar Sep 07 '23 12:09 ylecuyer

I've hit that service worker issue as well, but it seems a bit much to send that specific header with every asset request, no? Although maybe it's not a big deal.

Also, I think we should probably just configure this header by default in an exposed configuration point, so service workers can be served by Propshaft out of the box.

dhh avatar Sep 24 '23 16:09 dhh

We chose to follow your idea:

Also, I think we should probably just configure this header by default in an exposed configuration point, so service workers can be served by Propshaft out of the box.

And made the changes.

fauresebast avatar Oct 04 '23 11:10 fauresebast

Would other users want to set something other than /? I think David meant something like config.assets.prefix, not a boolean that adds or not the fixed / path?

brenogazzola avatar Dec 11 '23 18:12 brenogazzola

@brenogazzola done

fauresebast avatar Jan 10 '24 12:01 fauresebast

Hmm, I don't love this being service worker specific. Propshaft shouldn't know anything about specific files, types, or whatever. If we need to add/change headers, it needs to be a generic interface.

As it specifically pertains to the server-worker file at the root, we've gone this way for Rails 8: https://github.com/rails/rails/pull/50528

dhh avatar Jan 10 '24 13:01 dhh

We've added default PWA files that are routed through a controller in Rails now. I think that's a better solution.

dhh avatar May 15 '24 23:05 dhh