From dcbd99d6daca6633ef773a2e7bfdd5f96aea0d33 Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Wed, 1 Feb 2023 10:12:59 -0500 Subject: [PATCH 1/5] Fix typos --- packages/workbox-strategies/src/StrategyHandler.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/workbox-strategies/src/StrategyHandler.ts b/packages/workbox-strategies/src/StrategyHandler.ts index 88609255a..e9323c6f6 100644 --- a/packages/workbox-strategies/src/StrategyHandler.ts +++ b/packages/workbox-strategies/src/StrategyHandler.ts @@ -29,7 +29,7 @@ function toRequest(input: RequestInfo) { } /** - * A class created every time a Strategy instance instance calls + * A class created every time a Strategy instance calls * {@link workbox-strategies.Strategy~handle} or * {@link workbox-strategies.Strategy~handleAll} that wraps all fetch and * cache actions around plugin callbacks and keeps track of when the strategy @@ -534,7 +534,7 @@ class StrategyHandler { /** * Adds a promise to the * [extend lifetime promises]{@link https://w3c.github.io/ServiceWorker/#extendableevent-extend-lifetime-promises} - * of the event event associated with the request being handled (usually a + * of the event associated with the request being handled (usually a * `FetchEvent`). * * Note: you can await @@ -556,7 +556,7 @@ class StrategyHandler { * * Note: any work done after `doneWaiting()` settles should be manually * passed to an event's `waitUntil()` method (not this handler's - * `waitUntil()` method), otherwise the service worker thread my be killed + * `waitUntil()` method), otherwise the service worker thread may be killed * prior to your work completing. */ async doneWaiting(): Promise { From 36edccf2a023893ce9612d18264eeb2397b06979 Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Wed, 1 Feb 2023 13:54:16 -0500 Subject: [PATCH 2/5] Fix potential unhandled rejections in StrategyHandler.doneWaiting doneWaiting throws on the first rejected promise that it encounters. This means that subsequent rejected promises may result in unhandled rejection errors. Fixes #3171 --- .../workbox-core/src/_private/allSettled.ts | 40 +++++++++++++++++++ .../workbox-strategies/src/StrategyHandler.ts | 14 +++++-- 2 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 packages/workbox-core/src/_private/allSettled.ts diff --git a/packages/workbox-core/src/_private/allSettled.ts b/packages/workbox-core/src/_private/allSettled.ts new file mode 100644 index 000000000..d6b863d3b --- /dev/null +++ b/packages/workbox-core/src/_private/allSettled.ts @@ -0,0 +1,40 @@ +export interface PromiseResolution { + status: 'fulfilled'; + value: T; +} + +export interface PromiseRejection { + status: 'rejected'; + reason: unknown; +} + +export type PromiseResult = PromiseResolution | PromiseRejection; + +/** + * Promise.allSettled polyfill based on + * + * https://github.com/es-shims/Promise.allSettled/blob/main/implementation.js + * + * which is (c) 2019 Jordan Harband and used under the terms of the MIT license. + */ +export function allSettled( + iterable: Iterable>, +): Promise[]> { + const values = Array.from(iterable); + return Promise.all( + values.map(function (item) { + const onFulfill = function (value: T) { + return {status: 'fulfilled' as const, value: value}; + }; + const onReject = function (reason: unknown) { + return {status: 'rejected' as const, reason: reason}; + }; + const itemPromise = Promise.resolve(item); + try { + return itemPromise.then(onFulfill, onReject); + } catch (e) { + return Promise.reject(e); + } + }), + ); +} diff --git a/packages/workbox-strategies/src/StrategyHandler.ts b/packages/workbox-strategies/src/StrategyHandler.ts index e9323c6f6..6c1d7864d 100644 --- a/packages/workbox-strategies/src/StrategyHandler.ts +++ b/packages/workbox-strategies/src/StrategyHandler.ts @@ -6,6 +6,10 @@ https://opensource.org/licenses/MIT. */ +import { + allSettled, + PromiseRejection, +} from 'workbox-core/_private/allSettled.js'; import {assert} from 'workbox-core/_private/assert.js'; import {cacheMatchIgnoreParams} from 'workbox-core/_private/cacheMatchIgnoreParams.js'; import {Deferred} from 'workbox-core/_private/Deferred.js'; @@ -560,9 +564,13 @@ class StrategyHandler { * prior to your work completing. */ async doneWaiting(): Promise { - let promise; - while ((promise = this._extendLifetimePromises.shift())) { - await promise; + while (this._extendLifetimePromises.length) { + const promises = this._extendLifetimePromises.splice(0); + const result = await allSettled(promises); + const firstRejection = result.find((i) => i.status === 'rejected'); + if (firstRejection) { + throw (firstRejection as PromiseRejection).reason; + } } } From 1e868abc33bda6c796f6e1c2f04e70f0660f39ad Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Wed, 1 Feb 2023 14:29:52 -0500 Subject: [PATCH 3/5] Fix test failures --- packages/workbox-core/src/_private.ts | 2 ++ packages/workbox-core/src/_private/allSettled.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/packages/workbox-core/src/_private.ts b/packages/workbox-core/src/_private.ts index 455b7cd08..4721f190d 100644 --- a/packages/workbox-core/src/_private.ts +++ b/packages/workbox-core/src/_private.ts @@ -7,6 +7,7 @@ */ // We either expose defaults or we expose every named export. +import {allSettled} from './_private/allSettled.js'; import {assert} from './_private/assert.js'; import {cacheNames} from './_private/cacheNames.js'; import {cacheMatchIgnoreParams} from './_private/cacheMatchIgnoreParams.js'; @@ -25,6 +26,7 @@ import {WorkboxError} from './_private/WorkboxError.js'; import './_version.js'; export { + allSettled, assert, cacheMatchIgnoreParams, cacheNames, diff --git a/packages/workbox-core/src/_private/allSettled.ts b/packages/workbox-core/src/_private/allSettled.ts index d6b863d3b..80dfa5900 100644 --- a/packages/workbox-core/src/_private/allSettled.ts +++ b/packages/workbox-core/src/_private/allSettled.ts @@ -1,3 +1,5 @@ +import '../_version.js'; + export interface PromiseResolution { status: 'fulfilled'; value: T; From f5c41d4052e41afd721c49b0a1f59b7e9e18dad2 Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Sat, 27 Apr 2024 20:51:58 -0400 Subject: [PATCH 4/5] Fix test failures --- packages/workbox-core/src/_private/allSettled.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/workbox-core/src/_private/allSettled.ts b/packages/workbox-core/src/_private/allSettled.ts index 80dfa5900..29ee7e7b8 100644 --- a/packages/workbox-core/src/_private/allSettled.ts +++ b/packages/workbox-core/src/_private/allSettled.ts @@ -18,8 +18,10 @@ export type PromiseResult = PromiseResolution | PromiseRejection; * https://github.com/es-shims/Promise.allSettled/blob/main/implementation.js * * which is (c) 2019 Jordan Harband and used under the terms of the MIT license. + * + * @private */ -export function allSettled( +function allSettled( iterable: Iterable>, ): Promise[]> { const values = Array.from(iterable); @@ -40,3 +42,5 @@ export function allSettled( }), ); } + +export {allSettled}; From acd93917217fbacacfb704b64253231c11305748 Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Wed, 20 Nov 2024 11:43:44 -0500 Subject: [PATCH 5/5] Switch to native Promise.allSettled As discussed in code review, this is widely available. Update tsconfig.json to recognize it. --- packages/workbox-core/src/_private.ts | 2 - .../workbox-core/src/_private/allSettled.ts | 46 ------------------- .../workbox-strategies/src/StrategyHandler.ts | 8 +--- tsconfig.json | 2 +- 4 files changed, 3 insertions(+), 55 deletions(-) delete mode 100644 packages/workbox-core/src/_private/allSettled.ts diff --git a/packages/workbox-core/src/_private.ts b/packages/workbox-core/src/_private.ts index 4721f190d..455b7cd08 100644 --- a/packages/workbox-core/src/_private.ts +++ b/packages/workbox-core/src/_private.ts @@ -7,7 +7,6 @@ */ // We either expose defaults or we expose every named export. -import {allSettled} from './_private/allSettled.js'; import {assert} from './_private/assert.js'; import {cacheNames} from './_private/cacheNames.js'; import {cacheMatchIgnoreParams} from './_private/cacheMatchIgnoreParams.js'; @@ -26,7 +25,6 @@ import {WorkboxError} from './_private/WorkboxError.js'; import './_version.js'; export { - allSettled, assert, cacheMatchIgnoreParams, cacheNames, diff --git a/packages/workbox-core/src/_private/allSettled.ts b/packages/workbox-core/src/_private/allSettled.ts deleted file mode 100644 index 29ee7e7b8..000000000 --- a/packages/workbox-core/src/_private/allSettled.ts +++ /dev/null @@ -1,46 +0,0 @@ -import '../_version.js'; - -export interface PromiseResolution { - status: 'fulfilled'; - value: T; -} - -export interface PromiseRejection { - status: 'rejected'; - reason: unknown; -} - -export type PromiseResult = PromiseResolution | PromiseRejection; - -/** - * Promise.allSettled polyfill based on - * - * https://github.com/es-shims/Promise.allSettled/blob/main/implementation.js - * - * which is (c) 2019 Jordan Harband and used under the terms of the MIT license. - * - * @private - */ -function allSettled( - iterable: Iterable>, -): Promise[]> { - const values = Array.from(iterable); - return Promise.all( - values.map(function (item) { - const onFulfill = function (value: T) { - return {status: 'fulfilled' as const, value: value}; - }; - const onReject = function (reason: unknown) { - return {status: 'rejected' as const, reason: reason}; - }; - const itemPromise = Promise.resolve(item); - try { - return itemPromise.then(onFulfill, onReject); - } catch (e) { - return Promise.reject(e); - } - }), - ); -} - -export {allSettled}; diff --git a/packages/workbox-strategies/src/StrategyHandler.ts b/packages/workbox-strategies/src/StrategyHandler.ts index 6c1d7864d..56c60bcae 100644 --- a/packages/workbox-strategies/src/StrategyHandler.ts +++ b/packages/workbox-strategies/src/StrategyHandler.ts @@ -6,10 +6,6 @@ https://opensource.org/licenses/MIT. */ -import { - allSettled, - PromiseRejection, -} from 'workbox-core/_private/allSettled.js'; import {assert} from 'workbox-core/_private/assert.js'; import {cacheMatchIgnoreParams} from 'workbox-core/_private/cacheMatchIgnoreParams.js'; import {Deferred} from 'workbox-core/_private/Deferred.js'; @@ -566,10 +562,10 @@ class StrategyHandler { async doneWaiting(): Promise { while (this._extendLifetimePromises.length) { const promises = this._extendLifetimePromises.splice(0); - const result = await allSettled(promises); + const result = await Promise.allSettled(promises); const firstRejection = result.find((i) => i.status === 'rejected'); if (firstRejection) { - throw (firstRejection as PromiseRejection).reason; + throw (firstRejection as PromiseRejectedResult).reason; } } } diff --git a/tsconfig.json b/tsconfig.json index 9ca662e37..37bf717ed 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "composite": true, "declaration": true, - "lib": ["es2017", "webworker"], + "lib": ["es2020", "webworker"], "module": "esnext", "moduleResolution": "node", "noFallthroughCasesInSwitch": true,