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

ServiceWorker static routing API #1701

Merged
merged 59 commits into from
Feb 22, 2024

Conversation

yoshisatoyanagisawa
Copy link
Contributor

@yoshisatoyanagisawa yoshisatoyanagisawa commented Jan 15, 2024

This API allows developers to configure the routing, and allows them to offload simple things ServiceWorkers do. If the condition matches, the navigation happens without starting ServiceWorkers or executing JavaScript, which allows web pages to avoid performance penalties due to ServiceWorker interceptions.

Explainer: https://github.com/WICG/service-worker-static-routing-api
TAG review: w3ctag/design-reviews#863


💥 Error: 503 Service Unavailable 💥

PR Preview failed to build. (Last tried on Feb 22, 2024, 12:13 AM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 HTML Diff Service - The HTML Diff Service is used to create HTML diffs of the spec changes suggested in a pull request.

🔗 [Related URL]([object Object])

<html><body><h1>503 Service Unavailable</h1>
No server is available to handle this request.
</body></html>

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

I used to work on the specification update to support the
ServiceWorker static routing API
(https://github.com/WICG/service-worker-static-routing-api)
w3c#1686

However, I accidentally closed it by force-sync to the ServiceWorker
specification's repository HEAD.

This CL is for reviving it.
Just trying to add workflows rule to run CI in static_routing_api repository.
Considering the future update of the condition, we should mark the case
handled as one of conditions.  Also, we need to make it extensible.
It was flipped by mistake.
sisidovski and others added 4 commits February 21, 2024 10:45
Co-authored-by: Marijn Kruisselbrink <[email protected]>
… unset` phrase (#14)

* Remove |raceResponseMap|

* Re-format {{ServiceWorkerGlobalScope}} part and move `It is initially unset` phrase
Upon the review request, this CL makes |activeWorker| not modified
in the Setup ServiceWorkerGlobalScope algorithm.  It is the caller's
responsibility to update the active service worker's global scope.

With this change, an effemeral ServiceWorkerGlobalScope can be used
for a cache source in the static routing API.
docs/index.bs Outdated Show resolved Hide resolved
docs/index.bs Outdated
:: |workerRealm|, a [=relevant realm=] of the [=service worker/global object=]
:: |reservedClient|, a [=request/reserved client=]
:: |preloadResponse|, a [=promise=]
:: |raceResponse|, a [=race response=]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add "or null", since sometimes you pass in null (and handle null)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

docs/index.bs Outdated
: Output
:: a [=/response=] or null

1. If |request|'s [=request/reserved client=] is null, return null.
Copy link
Collaborator

@mkruisselbrink mkruisselbrink Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe add a Note explaining that this check ensures that we're only dealing with non-subresrouce requests, and thus looking up a registration using the request URL is correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I was wrong. We don't want to limit this algorithm for non-subresource requests. What we'd like to achieve here is to get [=active worker=] from [=/request=], regardless of non-subresource requests or subresource requests.

Updated to get the active worker from subreosurce requests, but perhaps we can just use |request|'s [=request/client=]'s [=active service worker=] for both subresource and non-subresource requests?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you have now looks reasonable to me. I think using request's client's active service worker for non-subresource requests would be incorrect. Maybe at some point we should abstract out a "give me the right registration for this request" algorithm to be shared between Handle Fetch and this algorithm, but for now what you have seems fine too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the clarification!

* Remove |raceResponseMap|

* Re-format {{ServiceWorkerGlobalScope}} part and move `It is initially unset` phrase

* Fix |url| in Lookup Race Response

* Update race handling, lookup race response algorithm
Copy link
Collaborator

@mkruisselbrink mkruisselbrink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience, I think this addressed all my concerns.

@mkruisselbrink mkruisselbrink merged commit 8d4b9df into w3c:main Feb 22, 2024
2 checks passed
github-actions bot added a commit that referenced this pull request Feb 22, 2024
SHA: 8d4b9df
Reason: push, by mkruisselbrink

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@yoshisatoyanagisawa yoshisatoyanagisawa deleted the static_routing_api branch February 22, 2024 08:40
tidoust added a commit to tidoust/ServiceWorker that referenced this pull request Feb 22, 2024
Following w3c#1701, the `install` event now uses the `InstallEvent` interface.
github-actions bot added a commit to asleekgeek/ServiceWorker that referenced this pull request Feb 22, 2024
SHA: 8d4b9df
Reason: push, by pull[bot]

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
mkruisselbrink pushed a commit that referenced this pull request Feb 22, 2024
Following #1701, the `install` event now uses the `InstallEvent` interface.
sisidovski pushed a commit to whatwg/urlpattern that referenced this pull request Feb 29, 2024
In w3c/ServiceWorker#1701, the ServiceWorker static routing API uses the match algorithm to match the request URL. However, the original match algorithm accepts a URLPatternInput object, and it does not accept a URL struct. To allow the ServiceWorker static routing API to use the match algorithm, we need to make the match algorithm to accept the URL struct.

Fixes: #218.
yoshisatoyanagisawa added a commit to yoshisatoyanagisawa/ServiceWorker that referenced this pull request Mar 4, 2024
Updates in the URLPattern specification was requested during the
w3c#1701 review.

The issues for the request has been resolved:
- whatwg/urlpattern#217
- whatwg/urlpattern#218

However, during the specification updates, the algorithm names are also
updated, and adjustment is needed.
mkruisselbrink pushed a commit that referenced this pull request Mar 13, 2024
…tion (#1707)

* Update link texts upon updates in the URLPattern specification

Updates in the URLPattern specification was requested during the
#1701 review.

The issues for the request has been resolved:
- whatwg/urlpattern#217
- whatwg/urlpattern#218

However, during the specification updates, the algorithm names are also
updated, and adjustment is needed.


Co-authored-by: Domenic Denicola <[email protected]>
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.

5 participants