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

Service Worker Scope Pattern Matching explainer #417

Closed
3 of 5 tasks
wanderview opened this issue Sep 4, 2019 · 28 comments
Closed
3 of 5 tasks

Service Worker Scope Pattern Matching explainer #417

wanderview opened this issue Sep 4, 2019 · 28 comments

Comments

@wanderview
Copy link

wanderview commented Sep 4, 2019

こんにちはTAG!

I'm requesting a TAG review of:

Further details:

We recommend the explainer to be in Markdown. On top of the usual information expected in the explainer, it is strongly recommended to add:

  • Links to major pieces of multi-stakeholder review or discussion of this specification:
  • Links to major unresolved issues or opposition with this specification:

You should also know that...

This is an early explainer review. I anticipate changes based on Service Worker WG discussion at TPAC 2019.

We'd prefer the TAG provide feedback as (please select one):

  • open issues in our GitHub repo for each point of feedback
  • open a single issue in our GitHub repo for the entire review
  • leave review feedback as a comment in this issue and @-notify [github usernames]

Please preview the issue and check that the links work before submitting. In particular, if anything links to a URL which requires authentication (e.g. Google document), please make sure anyone with the link can access the document.

¹ For background, see our explanation of how to write a good explainer.

@kenchris
Copy link

Looking at this with @cynthia and we are both OK with this. Looks quite useful and as I stated before this might even make sense for Web NFC.

With that in mind, I was wondering whether there were any plans to do something similar for mimetypes (we do in Web NFC) and sometimes multiple mimetypes support are used say for json, like application/json text/json application/ld+json or you might want to get all images like image/*.

FYI @reillyeon

@kenchris kenchris added Missing: venue Progress: in progress Review type: CG early review An early review of general direction from a Community Group and removed Progress: unreviewed labels Sep 12, 2019
@cynthia
Copy link
Member

cynthia commented Sep 12, 2019

This also looks useful for non-browser JS as well (and has no web dependencies) - curious if this could be a universal feature. @littledan know anyone that might be able to weigh in on if this makes sense?

@dbaron
Copy link
Member

dbaron commented Sep 19, 2019

I think one of the things we need to talk about here is multiple specs that have different concepts of scope, and different way to express scope.

I'm particularly concerned about the scope expression in service workers and the scope of web app manifests. That comes up in two cases:

  • ability to make declarative equivalents (in app manifests) of script features (in service workers), or vice-versa, and
  • decisions about whether a feature belongs more in an app manifest or a service worker being biased by the scope-choosing needs of the feature's users rather than more fundamental aspects of which one is the right place for the feature.

It seems if app manifests and service workers have the same capabilities for expressing scopes, we'd avoid these problems.

@dbaron
Copy link
Member

dbaron commented Sep 19, 2019

(I'd note there is a bit in the explainer about Web App Manifests, but it's not clear whether it's saying that this is moving towards providing the same capabilities for both. I've certainly been told that they currently have substantially different capabilities.)

@wanderview
Copy link
Author

wanderview commented Sep 19, 2019

It seems if app manifests and service workers have the same capabilities for expressing scopes, we'd avoid these problems.

AIUI the manifest spec tries to keep their scopes behavior aligned with service worker scopes behavior. @mgiuca has told me he might do the spec work to try to factor out the scope behavior into a common place that both service workers and manifests can reference. That has not happened yet.

So my implicit going in assumption is that we want to keep them the same.

That being said, there is currently a mismatch between service workers and manifest in terms of matching search parameters. Service workers prefix matches against the search field, but there are parts of the manifest integration with android that cannot match against search parameters. There are discussions on going about what to do about that. We could try deprecating the service worker behavior, but we would need to determine if sites are using it, etc.

For the scopes pattern matching proposal one of the outcomes from the TPAC face-to-face is that we're going to punt on the search parameter bits I have in the explainer. I believe the rest of the explainer is supportable in manifest and our intent would be to keep them aligned.

Does that address your concern?

Edit: See existing issue discussion about manifest/sw scope search parameters behavior here in whatwg/urlpattern#5.

@mgiuca
Copy link

mgiuca commented Sep 19, 2019

@mgiuca has told me he might do the spec work to try to factor out the scope behavior into a common place that both service workers and manifests can reference. That has not happened yet.

I think it would be pretty easy; we'd just need a home for it. (Doesn't make sense to live in Service Workers or Manifest, IMO.) Where could it go? HTML? URL? Probably the same place where URLPattern would end up... where do you think that should live?

I hadn't realised until recently that there was a mismatch (I didn't realise that SW scope matched query strings). The unified algorithm would need to be parameterized by whether it matches query strings or not. The discussion on deprecating the matching of query strings is on w3c/ServiceWorker#1469.

I believe the rest of the explainer is supportable in manifest and our intent would be to keep them aligned.

We also need to make sure that every use case can be done by passing a JSON-like object to the URLPattern constructor (no other JS code or use of dynamic values). This came up in this discussion where it was suggested that certain use cases could be taken care of by use of the URL constructor, but you can't do those things from a declarative context like Manifest.

@dbaron
Copy link
Member

dbaron commented Sep 19, 2019

That sounds like it mostly addresses my concern (modulo the bits about the query string), and certainly sounds better than what I heard in an earlier side conversation.

@torgo torgo added this to the 2019-10-23-telecon milestone Oct 9, 2019
@kenchris
Copy link

kenchris commented Oct 9, 2019

Hi @wanderview can you add a comment here when further progress on this feature happens? Or do you have any ETA so that we can adjust our milestones?

@wanderview
Copy link
Author

I plan to work on this in Q4, but may not have significant new progress for your 10-23 telecon.

My next steps are to incorporate the TPAC feedback into the explainer:

w3c/ServiceWorker#1468 (comment)

I've also started digging into the features offered by the popular path-to-regexp library used in express. I'm not sure if I will try to incorporate findings from that in the explainer or rather just start a separate discussion around it.

Hope that helps. Thanks for the feedback.

@kenchris kenchris added the Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review label Dec 4, 2019
@kenchris
Copy link

kenchris commented Dec 4, 2019

Adding pending external feedback as work is still ongoing. Please ping us again when you have something new for us to look at, thanks!

@littledan
Copy link

cc @xtuc for feedback on ServiceWorker in CloudFlare Workers

@xtuc
Copy link

xtuc commented Dec 11, 2019

Cloudflare uses its own routing layer to handle the service-worker scope registration.
The URLPattern object looks interesting to provide routing in the service-worker.

@kenchris
Copy link

kenchris commented Mar 4, 2020

Hi @wanderview are you still pursuing this? Could you give us any update

@kenchris kenchris added this to the 2020-04-06-week milestone Mar 24, 2020
@kenchris
Copy link

kenchris commented Apr 7, 2020

Closing for now, please re-open when you have something new, so that it gets back on our radar. Thanks!

@kenchris kenchris closed this as completed Apr 7, 2020
@wanderview
Copy link
Author

I've been working on this effort some more recently and have an updated proposal.

The revamped explainer is in the same place:

https://github.com/wanderview/service-worker-scope-pattern-matching/blob/master/explainer.md

I've tried to keep the explainer focused on use cases. If you are interested in more detail, however, I also have a chromium design document here:

https://docs.google.com/document/d/17L6b3zlTHtyxQvOAvbK55gQOi5rrJLERwjt_sKXpzqc/edit?usp=sharing

My current plan for moving forward is to:

  1. Update the service worker WG in a video call to reaffirm consensus (organizing in Virtual Call: pattern matching scopes w3c/ServiceWorker#1535)
  2. Prototype in chromium.
  3. Write specs based on detail learned from prototyping.

Any additional feedback from the TAG on this updated proposal would be most welcome.

One particular question I am trying to figure out is where to spec the URLPattern. I'll probably split that off into a WICG repo unless service worker WG wants to take it.

Should I file a new issue or would you like to continue here? (I don't have permissions to re-open the issue myself.)

Thanks!

@wanderview
Copy link
Author

Sorry, I forgot to mention the main differences in the proposal:

  1. Pattern syntax is aligned with the popular path-to-regexp library.
  2. List of patterns are introduced (per TPAC feedback last year).
  3. Query parameter matching was dropped (per TPAC feedback last year).

@wanderview
Copy link
Author

@kenchris ping for question above. Should I open a new issue or can you re-open this one? Thanks!

@kenchris
Copy link

Sure, let me reopen. @plinss do we need to do anything with milestones etc so that this gets on our radar?

@kenchris kenchris reopened this Sep 16, 2020
@kenchris
Copy link

Is this in the Service Worker WG venue?

@kenchris kenchris added Progress: in progress Review type: later review and removed Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review Progress: stalled Missing: venue Review type: CG early review An early review of general direction from a Community Group labels Sep 16, 2020
@wanderview
Copy link
Author

Is this in the Service Worker WG venue?

Yes. I might move URLPattern out to WICG since I'm unsure if that belongs in SW WG itself.

By the way, this is still pre-implementation and pre-spec-definition. The proposal has just been updated based on previous feedback and should have fewer unknowns in it.

@cynthia
Copy link
Member

cynthia commented Sep 24, 2020

@kenchris and I discussed this today. The API surface and use cases look great! Some questions below.

Given this example:

const pattern = new URLPattern({ pathname: '/api/:product' });
if (let result = pattern.exec(url)) {
  routeTo(result.pathname.groups['product']);
}

And the input URL is /api/foo?bar=baz#yay , is the query and hash accessible from the result object?
Additionally, does the webapp manifest usage of pattern matching have the same restrictions as service workers?

@kenchris
Copy link

kenchris commented Sep 24, 2020

The library mentions that it doesn't support queries etc:

image

@kenchris
Copy link

Thank you for bringing this to the TAG!

@wanderview
Copy link
Author

And the input URL is /api/foo?bar=baz#yay , is the query and hash accessible from the result object?

We could certainly expose the original matched URL as a property on the result dictionary. Code using URLPattern could also do that themselves. I don't feel too strongly either way and I'm willing to follow API input from developers once we have something they can test out.

Additionally, does the webapp manifest usage of pattern matching have the same restrictions as service workers?

I expect that to be the case, yes. I've been attempting to keep manifest constraints in mind.

@wanderview
Copy link
Author

And thank you for the review!

@cynthia
Copy link
Member

cynthia commented Sep 24, 2020

I don't feel too strongly either way and I'm willing to follow API input from developers once we have something they can test out.

Given the functionality, I'm sure developers will find a way to make new use cases for this. If it's not technically challenging, it would be good to see the this in scope.

@wanderview
Copy link
Author

FYI, we are making a small enhancement to URLPattern to add the option to perform case-insensitive matching. This was a web developer community request. Please see:

https://groups.google.com/a/chromium.org/g/blink-dev/c/hNUoDg94-_8/m/-Cnv2qyMAAAJ
whatwg/urlpattern#148

Note, since this is a small change I thought it might be better to comment here instead of creating a new issue. If you would prefer a new issue, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants