-
Notifications
You must be signed in to change notification settings - Fork 99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add service-worker-allowed header by default #153
Conversation
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? |
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.
On production this isn't an issue as you can configure nginx to add the I have a similar change on a fork to bypass this limitation: ylecuyer@55ea5f3 |
e0f7efd
to
5bb5f80
Compare
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. |
c7749f7
to
c93a478
Compare
We chose to follow your idea:
And made the changes. |
c93a478
to
2ce922a
Compare
Would other users want to set something other than |
0103f67
to
25df59d
Compare
25df59d
to
993376d
Compare
@brenogazzola done |
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: rails/rails#50528 |
We've added default PWA files that are routed through a controller in Rails now. I think that's a better solution. |
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