-
Notifications
You must be signed in to change notification settings - Fork 267
[WS-2111]: Clean up sw.js #13689
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
base: latest
Are you sure you want to change the base?
[WS-2111]: Clean up sw.js #13689
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Bumps the service worker version and adjusts caching/fetch handling to improve offline stability and align cacheable asset routes with the current hosting/layout.
Changes:
- Bumped SW version to
v0.3.4. - Updated
CACHEABLE_FILESentries for Reverb and Frosted Promo to match current asset locations. - Changed fetch handler fallbacks to return a
503Responseinstead of throwing / falling through.
| const version = 'v0.3.3'; | ||
| const version = 'v0.3.4'; | ||
| // Update cache name when changing caching logic / changes in offlinepage.tsx | ||
| const cacheName = 'simorghCache_v3'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrade cacheName as well
| /^https:\/\/static(?:\.test)?\.files\.bbci\.co\.uk\/ws\/(?:simorgh-assets|simorgh1-preview-assets|simorgh2-preview-assets)\/public\/static\/js\/reverb\/reverb-3.10.2.js$/, | ||
| 'https://mybbc-analytics.files.bbci.co.uk/reverb-client-js/reverb-3.10.2.js', | ||
| // Smart Tag | ||
| 'https://mybbc-analytics.files.bbci.co.uk/reverb-client-js/smarttag-5.29.4.min.js', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not properly cached as per Toby's findings
Refer :https://bbc-tpg.slack.com/archives/C08JHPZFCTT/p1769509652307959?thread_ts=1769509469.732389&cid=C08JHPZFCTT.
So fix for this is WIP?
| const cached = await cache.match(event.request); | ||
| if (cached) return cached; | ||
| return fetch(event.request); | ||
| return new Response('', { status: 503 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if we should keep the fetch(event.request); 🤔 Is it possible to end up in the situation where isPWADeviceOffline = true, but the user is actually back online and the request is not a in navigation mode (i.e. isNavigationMode = false?
Probably we could attempt to do a request and catch error if needed?
fetch(event.request).catch(err => {
return new Response('', { status: 503 });
})| } | ||
| // fallback to browser default behavior | ||
| throw err; | ||
| return new Response('', { status: 503 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would that be a default fallback browser's behavior? I think we might lose error context if we just return 503 with an empty string 🤔
Edit: Although that would only happen in the navigation mode, not all requests.
Resolves JIRA: https://bbc.atlassian.net/browse/WS-2111
Summary
This PR improves the service worker’s stability and caching, ensuring:
Code changes
Fetch error handling
Cache cleanup
Frosted Promo
https://static.files.bbci.co.uk/ws/simorgh-assets/public/_next/static/chunks/frosted_promo.*\.jsReverb script
https://mybbc-analytics.files.bbci.co.uk/reverb-client-js/reverb-3.10.2.jsMoment.js
Testing
Useful Links