-
Notifications
You must be signed in to change notification settings - Fork 259
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 #399
base: main
Are you sure you want to change the base?
Add service worker #399
Conversation
7af65c8
to
be38e38
Compare
be38e38
to
23f00c1
Compare
Looks good to me, though I've not used serviceWorkers for caching before so I'm unware of any pitfalls of them. |
Well there's a gotcha regarding cache invalidation. With cache first strategy at least we entirely skip network fetch if it's found in the local cache. Might not be much of an issue for most cases but some ppl may start wondering why their new image isn't being displayed when they updated some image in xyz website. Perhaps we can add functionality to clear the cache from UI. |
Good idea. Another thought is what happens when the user tries to hard refresh ( Do we know if the service worker gets reset? |
OK in the GitHub hosted page it seems to bypass the cache from service worker, and the page fails to load if completely offline. But as an extension the images are loading properly but weirdly enough I don't see any network request being made. |
Also do you know if chrome removed the functionality to view cache from web inspector for extensions? I can't seem to be able to access those. |
I can try running it myself tomorrow (left work late today) and see what i can figure out. |
Sorry about the delay, was busy with other work. I added the option to clear entire cache and service workers when user clears their data. This should take care of any cache related issue that might arise. |
Using this PR for service worker implementation as per #334. As I have scrapped #339 due to it being too ugly approach.
As suggested by @metruzanca here's modular approach for creating service workers. I have added it as an entry point for webpack builds rather than direct copy of a single js file.
Registration of service workers is done at page load from
initialBackground.js
rather than by themanifest.json
. This way it's more portable, and doesn't break as was the case when trying to use with manifest v3 in firefox. Although firefox still disallows service workers for browser extensions, the only difference will be simply service worker registration failing and proceeding as normal.As an added benefit this also works when the page is not run as an extension. Which means for service worker unsupported browsers (in extensions only) like firefox, users can still gain the features available to service workers by simply settin the remote url as new tab page (which works even when offline).
PS: I did remove
[content-hash]
from webpack build file for js as I am not sure how to use a static name to load frominitialBackground.js