tomayac
commented
5 months ago Fully approve of the intent of the PR. However, I don't think Promise.allSettled() needs polyfilling looking at browser support. Looking at devices that didn't make the Safari 13 cut, this are iPhone 5S, iPhone 6 and 6 Plus, the original iPad Air, the iPad mini 2 and iPad mini 3, and the 6th-generation iPod Touch. For Chromebooks, we'd be locking out Chromebooks stuck on Chrome <76. @tropicadri, what do you think?
Fixes https://github.com/GoogleChrome/workbox/issues/3171
StrategyHandler.doneWaiting throws on the first rejected promise that it encounters. This means that subsequent rejected promises may result in unhandled rejection errors.
Promise.allSettled is a natural solution. It does not appear to be supported on all browsers that workbox uses. (However, I'm having trouble finding a clear statement of what browsers workbox supports, so I could be wrong.) I tried installing promise.allsettled from NPM but had trouble getting the default import from that module to work correctly with Rollup, and I noticed that the workbox service worker libraries tend to avoid external dependencies in general, so I created a local version instead. I put it under workbox-core's _private directory, following the example of other utility code such as Deferred. If I should handle this differently, please let me know.
This is a rebase of #3172 against the v7 branch.