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

Should InstallEvent.addRoutes reject if the the install event is not active? #1743

Open
youennf opened this issue Jan 21, 2025 · 1 comment

Comments

@youennf
Copy link

youennf commented Jan 21, 2025

Currently, it is possible to call addRoutes while the service worker is no longer installing.
I am wondering what the rationale for this decision is.

In that specific case, there is no guarantee that a route will be stored persistently (the service worker could have been stopped just after addRoutes is called).
When run again, the service worker has no way to know which of these after-installing service worker routes will actually be applied.

This is probably an edge case, but I am wondering whether it would be safer to call add lifetime promise within addRoutes.
This would have two effects:

  1. Guarantee that addRoutes promise is rejected if the service worker is not installing.
  2. Guarantee that the service worker will go to installed state after the route is properly stored (without developers having to explicitly call waitUntil).
@yoshisatoyanagisawa
Copy link
Collaborator

tl;dr I should say it is a specification bug. Thanks for finding it.

We intend to limit the addRoutes() to be called within the install handler execution. That is why we made addRoutes() as a member of InstallEvent (not a ServiceWorker global scope etc). However, if there is a path to call it outside of the install event execution, it is a specification bug. (And, my colleague pointed out that there is a way to call addRoutes() by storing it in the global scope and use it outside, which is out of what I thought during the API implementation)

Thanks for the suggestion on add lifetime promise. I could find the usage in FetchEvent.respondWith(r).
I am trying to fix it with #1744

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

No branches or pull requests

2 participants