Skip to content
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

Closed

Conversation

fauresebast
Copy link

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

@brenogazzola
Copy link
Collaborator

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?

@ylecuyer
Copy link

ylecuyer commented Sep 7, 2023

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: ylecuyer@55ea5f3

@fauresebast fauresebast force-pushed the custom_headers_configuration branch 2 times, most recently from e0f7efd to 5bb5f80 Compare September 11, 2023 11:43
@dhh
Copy link
Member

dhh commented Sep 24, 2023

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.

@fauresebast fauresebast force-pushed the custom_headers_configuration branch 2 times, most recently from c7749f7 to c93a478 Compare October 4, 2023 11:55
@fauresebast fauresebast changed the title Add custom HTTP headers configuration for assets Add service-worker-allowed header by default Oct 4, 2023
@fauresebast
Copy link
Author

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.

@brenogazzola
Copy link
Collaborator

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?

@fauresebast fauresebast force-pushed the custom_headers_configuration branch 2 times, most recently from 0103f67 to 25df59d Compare January 10, 2024 12:07
@fauresebast
Copy link
Author

@brenogazzola done

@dhh
Copy link
Member

dhh commented Jan 10, 2024

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

@dhh
Copy link
Member

dhh commented May 15, 2024

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

@dhh dhh closed this May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants