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

[✨] Create a unique file name for each version of service-worker.js #6311

Closed
DustinJSilk opened this issue May 14, 2024 · 16 comments
Closed
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: enhancement New feature or request

Comments

@DustinJSilk
Copy link
Contributor

DustinJSilk commented May 14, 2024

Is your feature request related to a problem?

The service-worker.js file includes a list of bundles to prefetch. If the service-worker.js file is cached by the browser or a CDN, the user may receive an outdated list of files to prefetch resulting in a large amount of unnecessary network requests which may return 404 or it could prefetch unused files. It also means that the correct files arent being prefetched which reduces performance.

Describe the solution you'd like

Create a unique file name for each service-worker.js file so that if a new version is released, the service worker setup will install the new version.

Describe alternatives you've considered

We could ensure we return a Cache-Control header with the service-worker.js file so that it isnt cached by browsers or CDNs, but then the page will always download a new copy even when it isnt needed.

Additional context

No response

@DustinJSilk DustinJSilk added STATUS-1: needs triage New issue which needs to be triaged TYPE: enhancement New feature or request labels May 14, 2024
@DustinJSilk
Copy link
Contributor Author

This seems related to #6302 and also some of the issues here #3777.

@PatrickJS
Copy link
Member

Where are you deploying your qwik app? Aws, cloudflare, vercel, gcp, other?

@DustinJSilk
Copy link
Contributor Author

Cloud Run on GCP. I have a CDN in place that caches all static files.

@PatrickJS
Copy link
Member

service-worker.js spec ignores caching policy for this reason. We have another version of service-worker for MFE that we haven't documented by should be more agnostic so won't require updating

@DustinJSilk
Copy link
Contributor Author

I see, it looks like the below adds a Cache-Control header to the service-worker.js, which causes it to cache in the CDN which means it doesnt update in the browser until the CDN entry expires.

const { router, notFound, staticFile } = createQwikCity({
  // ...
  static: {
    cacheControl: 'public, max-age=31557600',
  },
)}

@PatrickJS
Copy link
Member

yeah, the cdn might be caching it but the browser won't follow the cache spec. on your cdn you can add a setting to ignore caching the service-worker.js or clear the cache for it on deploys

@DustinJSilk
Copy link
Contributor Author

DustinJSilk commented May 14, 2024

Unfortunately GCP CDN doesn't allow you to ignore a specific file, i'd need to either invalidate the entry with each deploy or remove the Cache-Control header from the service-worker.js file somehow.

EDIT

Looks like i can bypass the CDN if i add a custom header to the service-worker.js file, that should be the easiest solution on GCP

^ actually thats not possible in hindsight

@PatrickJS
Copy link
Member

PatrickJS commented May 14, 2024

@DustinJSilk ok @thejackshelton is making an example app but it should be as easy as
replacing
<ServiceWorkerRegister />
with

<PrefetchServiceWorker />

in your root.tsx

@DustinJSilk
Copy link
Contributor Author

Ok great! It looks like this will embed the symbol files in the HTML and pass it through to the service worker to then be prefetched.

Are there any draw backs in performance to using this approach as opposed to the regular service worker when it isnt cached by the CDN?

Thanks for your time!

@PatrickJS
Copy link
Member

no drawbacks 👍 this is the new way to do it which will also works for microfrontends

@PatrickJS
Copy link
Member

ok one thing about this (we need to fix) is that base is set to /build/ so assuming you didn't set base in your ssr.entry.tsx file then it should be fine. if it's different then you have to set the new value in the component

@DustinJSilk
Copy link
Contributor Author

ok cool, i'll get this setup and test it out on my staging environment today

@DustinJSilk
Copy link
Contributor Author

Ah actually, the PrefetchServiceWorker doesnt support a nonce property like the ServiceWorkerRegister component.

@PatrickJS
Copy link
Member

yeah we need to add that

@PatrickJS
Copy link
Member

PatrickJS commented May 14, 2024

@DustinJSilk if you need a fix right away before the next release you can do this in entry.ssr.tsx for any scripts you need. the performance during ssr won't be as great so there are trade-offs until we add nonce support

image

^ fix the typo in my screenshot

@DustinJSilk
Copy link
Contributor Author

Im closing this issue since it is solved by the fact that the service worker spec doesn't cache service worker files in the browser and the PrefetchServiceWorker doesn't change with each build so it is safe to cache in a CDN.

The only thing to note is that future changes to the PrefetchServiceWorker might require some users to evict their prefetch-service-worker.js file from their CDN to reflect changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants