Fix service worker installation failure on Firefox#7755
Fix service worker installation failure on Firefox#7755pranjalisr wants to merge 7 commits intowebpack:mainfrom
Conversation
|
@pranjalisr is attempting to deploy a commit to the OpenJS Foundation Team on Vercel. A member of the Team first needs to authorize it. |
|
@alexander-akait Just following up to kindly request a review of this PR when you have availability. |
|
@evenstensberg Can you look at this and test, I think we can merge it |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Tested in Firefox using production build (yarn build + yarn serve-dist). Service worker installs successfully and no longer aborts on install. 
@evenstensberg Can you please check this? If this sounds Ok then I will push the code changes. |
ovflowd
left a comment
ovflowd
left a comment
There was a problem hiding this comment.
Appreciate the contribution. Code is not ideal, made a few suggestions :)
|
Made changes and re-ran locally on Firefox. It's running 
|
src/sw.js
Outdated
| return url.href; | ||
| }); | ||
|
|
||
| const allManifestURLs = [...manifest, ...otherManifest].map( |
There was a problem hiding this comment.
doubt it is a problem but lets filter out so we dont have duplicate urls before we access the cache
There was a problem hiding this comment.
What do you suggest? Should I make changes in allManifestURLs ?
There was a problem hiding this comment.
yes just filter out dupes, easy to do if you convert it to a set and back to an array.
There was a problem hiding this comment.
Made code changes as suggested. Checked locally, working fine on firefox browser.
alexander-akait
left a comment
alexander-akait
left a comment
There was a problem hiding this comment.
Please rebase
/cc @evenstensberg please take a look
|
I'll review once rebased |
|
I will rebase within 24hrs, travelling today so don't have access to laptop for today |
pranjalisr
force-pushed
the
fix/firefox-sw-install-clean
branch
from
dbfa1b0 to
4fe19fc
Compare
|
@evenstensberg please review. Working fine locally on firefox browser |
| return caches.match("/app-shell/index.html"); | ||
| default: | ||
| return Response.error(); | ||
| return fetch(event.request); |
There was a problem hiding this comment.
I dont think this is smart. Lets say something is wrong with your computer and it doesnt have a good network. You will always re-fetch until it suceeds. will be very expensive an circular-loop.
4 participants
Summary
Fixes #7738
Fix SW install failure in Firefox by replacing cache.addAll() with Promise.allSettled() to prevent installation from failing due to individual precache errors. Also deduplicates manifest URLs before caching.
What kind of change does this PR introduce?
Code change
Did you add tests for your changes?
No.
Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
No documentation changes are required.