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 ServiceWorker #2357

Merged
merged 4 commits into from
Nov 29, 2024
Merged

Add ServiceWorker #2357

merged 4 commits into from
Nov 29, 2024

Conversation

autonome
Copy link
Collaborator

@autonome autonome commented Nov 28, 2024

Large feature, since that's how most of the references treat it, and we can break out smaller pieces later.

Things like Cache are instrumental to the core functionality, whereas WindowClient might make sense to break out.

Computed status to line up with Caniuse.

@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Nov 28, 2024
@autonome autonome marked this pull request as ready for review November 28, 2024 12:06
Copy link
Contributor

@captainbrosset captainbrosset left a comment

Choose a reason for hiding this comment

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

Not approving just yet because I think others should review too. But I don't really have any major comments other than:

  • A suggestion for the description.
  • A suggestion for pinning the compat data on keys that I think make more sense.
  • An agreement that Client and WindowClient make sense here, I've never seen them be used in other contexts than in service workers.

features/service-workers.yml Outdated Show resolved Hide resolved
features/service-workers.yml Outdated Show resolved Hide resolved
autonome and others added 2 commits November 28, 2024 20:53
Co-authored-by: Patrick Brosset <patrickbrosset@gmail.com>
Co-authored-by: Patrick Brosset <patrickbrosset@gmail.com>
Copy link
Contributor

@captainbrosset captainbrosset left a comment

Choose a reason for hiding this comment

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

I'm approving this because I don't think there's a subset of the keys here that form a separate feature. And if so, we could always extract them later.

I did a bit of googling about the Cache API, and every page I could find talks about it in the scope of service workers. In fact, search for "cache API" -service basically only returns results from other libraries, languages, or frameworks. This comforts me in thinking that this feature is the right home for the Cache and CacheStorage APIs.

I see you've asked the serviceworker group for feedback, which is great.

@autonome
Copy link
Collaborator Author

Thanks @captainbrosset, if we get feedback from the spec folks, I will file followup issues and address there.

@autonome autonome merged commit 8cf2e60 into web-platform-dx:main Nov 29, 2024
3 checks passed
@autonome autonome deleted the sw branch November 29, 2024 13:28
@autonome
Copy link
Collaborator Author

autonome commented Dec 9, 2024

Capturing the feedback from the ServiceWorker standards folks:

Not sure how to read that pull request, but I would definitely not group the "Caches" API with the rest of service workers. While it was designed to be convenient to be used in conjunction with service worker fetch events nothing about it limits it to that use case, and really the only reason it's in the same spec is because nobody has done the work to split it off, not because it logically belongs here (i.e. see w3c/ServiceWorker#879).

Sounds like no action at this time, but once that work happens that the cache parts can be split out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature definition Creating or defining new features or groups of features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants