From 9dcb557cded72bd2a19f1f234ba1f0acb1e1f899 Mon Sep 17 00:00:00 2001 From: Luke Horvat Date: Fri, 21 Jun 2024 20:56:47 +0200 Subject: [PATCH 1/7] Fix timeout-dependent return types of .add() --- source/index.ts | 21 +++++++++++++++++---- source/options.ts | 20 ++++++++++++++++++++ test-d/index.test-d.ts | 11 ++++++++++- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/source/index.ts b/source/index.ts index faf2c9e..ff03c41 100644 --- a/source/index.ts +++ b/source/index.ts @@ -2,7 +2,7 @@ import {EventEmitter} from 'eventemitter3'; import pTimeout, {TimeoutError} from 'p-timeout'; import {type Queue, type RunFunction} from './queue.js'; import PriorityQueue from './priority-queue.js'; -import {type QueueAddOptions, type Options, type TaskOptions} from './options.js'; +import {type QueueAddOptions, type Options, type TaskOptions, type WithoutTimeout, type WithoutTimeoutThrow, type WithTimeoutThrow} from './options.js'; type Task = | ((options: TaskOptions) => PromiseLike) @@ -231,9 +231,22 @@ export default class PQueue(function_: Task, options: {throwOnTimeout: true} & Exclude): Promise; - async add(function_: Task, options?: Partial): Promise; - async add(function_: Task, options: Partial = {}): Promise { + async add( + function_: Task, + options?: WithoutTimeout> + ): Promise; + async add( + function_: Task, + options: WithTimeoutThrow> + ): Promise; + async add( + function_: Task, + options: WithoutTimeoutThrow> + ): Promise; + async add( + function_: Task, + options: Partial = {}, + ): Promise { options = { timeout: this.timeout, throwOnTimeout: this.#throwOnTimeout, diff --git a/source/options.ts b/source/options.ts index 8e8bf7b..c536e62 100644 --- a/source/options.ts +++ b/source/options.ts @@ -109,3 +109,23 @@ export type TaskOptions = { */ readonly signal?: AbortSignal; }; + +/** +Helper type to target the case where timeout=number. +*/ +export type WithTimeout = Options & {timeout: number}; + +/** +Helper type to target the case where timeout=undefined. +*/ +export type WithoutTimeout = Omit | {timeout: undefined}; + +/** +Helper type to target the case where timeout=number and throwOnTimeout=true. +*/ +export type WithTimeoutThrow = WithTimeout & {throwOnTimeout: true}; + +/** +Helper type to target the case where timeout=number and throwOnTimeout=false/undefined. +*/ +export type WithoutTimeoutThrow = WithTimeout & {throwOnTimeout?: false}; diff --git a/test-d/index.test-d.ts b/test-d/index.test-d.ts index 67641a7..ee129ba 100644 --- a/test-d/index.test-d.ts +++ b/test-d/index.test-d.ts @@ -3,5 +3,14 @@ import PQueue from '../source/index.js'; const queue = new PQueue(); -expectType>(queue.add(async () => '🦄')); +expectType>(queue.add(async () => '🦄')); +expectType>(queue.add(async () => '🦄', {})); +expectType>(queue.add(async () => '🦄', {throwOnTimeout: undefined})); +expectType>(queue.add(async () => '🦄', {throwOnTimeout: false})); expectType>(queue.add(async () => '🦄', {throwOnTimeout: true})); +expectType>(queue.add(async () => '🦄', {timeout: undefined})); +expectType>(queue.add(async () => '🦄', {timeout: 1, throwOnTimeout: true})); +expectType>(queue.add(async () => '🦄', {timeout: 1})); +expectType>(queue.add(async () => '🦄', {timeout: 1, throwOnTimeout: undefined})); +expectType>(queue.add(async () => '🦄', {timeout: 1, throwOnTimeout: false})); +expectType>(queue.add(async () => '🦄', {priority: 1})); From c1d076bb536fa9146146be1a8dbd2d4bfc62533a Mon Sep 17 00:00:00 2001 From: Luke Horvat Date: Fri, 21 Jun 2024 21:22:06 +0200 Subject: [PATCH 2/7] Fix timeout-dependent return types of .addAll() --- source/index.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/source/index.ts b/source/index.ts index ff03c41..3b21ba4 100644 --- a/source/index.ts +++ b/source/index.ts @@ -300,15 +300,19 @@ export default class PQueue( functions: ReadonlyArray>, - options?: {throwOnTimeout: true} & Partial>, + options?: WithoutTimeout>, ): Promise; async addAll( functions: ReadonlyArray>, - options?: Partial, + options: WithTimeoutThrow> + ): Promise; + async addAll( + functions: ReadonlyArray>, + options: WithoutTimeoutThrow>, ): Promise>; async addAll( functions: ReadonlyArray>, - options?: Partial, + options: Partial = {}, ): Promise> { return Promise.all(functions.map(async function_ => this.add(function_, options))); } From 16791ca4af9a8dac3ab07ba9a9710ee74fac307f Mon Sep 17 00:00:00 2001 From: Luke Horvat Date: Fri, 21 Jun 2024 21:36:34 +0200 Subject: [PATCH 3/7] Add clarity to readme about timeout default value --- readme.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index bf148e1..c8ecd0a 100644 --- a/readme.md +++ b/readme.md @@ -63,7 +63,8 @@ Concurrency limit. ##### timeout -Type: `number` +Type: `number`\ +Default: `undefined` (i.e. no timeout is enforced) Per-operation timeout in milliseconds. Operations fulfill once `timeout` elapses if they haven't already. @@ -72,7 +73,7 @@ Per-operation timeout in milliseconds. Operations fulfill once `timeout` elapses Type: `boolean`\ Default: `false` -Whether or not a timeout is considered an exception. +Whether or not a timeout is considered an exception. This option only has an effect when `timeout` is defined. ##### autoStart From 0d8fcb64f7784ecc9e161ab59f45a47566b36a7e Mon Sep 17 00:00:00 2001 From: Luke Horvat Date: Fri, 21 Jun 2024 21:38:07 +0200 Subject: [PATCH 4/7] Remove completed todo --- source/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/source/index.ts b/source/index.ts index 3b21ba4..7519a26 100644 --- a/source/index.ts +++ b/source/index.ts @@ -50,7 +50,6 @@ export default class PQueue) { super(); From 520dc1b79a55026e2d4e28e28aa867902b1c1c86 Mon Sep 17 00:00:00 2001 From: Luke Horvat Date: Fri, 21 Jun 2024 22:08:54 +0200 Subject: [PATCH 5/7] Warn in console when timeout=undefined and throwOnTimeout=true --- source/index.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source/index.ts b/source/index.ts index 7519a26..3eb8ad6 100644 --- a/source/index.ts +++ b/source/index.ts @@ -252,6 +252,10 @@ export default class PQueue { this.#queue.enqueue(async () => { this.#pending++; From 8f20196dedb4e24c65f7a4881e21e0b89c29940d Mon Sep 17 00:00:00 2001 From: Luke Horvat Date: Sat, 22 Jun 2024 14:36:18 +0200 Subject: [PATCH 6/7] Revert todo removal --- source/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/source/index.ts b/source/index.ts index 3eb8ad6..852d7c5 100644 --- a/source/index.ts +++ b/source/index.ts @@ -50,6 +50,7 @@ export default class PQueue) { super(); From 1dc92948800df206ab0ff438b45d84b36289c57d Mon Sep 17 00:00:00 2001 From: Luke Horvat Date: Sat, 22 Jun 2024 14:59:39 +0200 Subject: [PATCH 7/7] Further clarify timeout void behavior in readme --- readme.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/readme.md b/readme.md index c8ecd0a..df37cd8 100644 --- a/readme.md +++ b/readme.md @@ -73,7 +73,9 @@ Per-operation timeout in milliseconds. Operations fulfill once `timeout` elapses Type: `boolean`\ Default: `false` -Whether or not a timeout is considered an exception. This option only has an effect when `timeout` is defined. +Whether or not a timeout is considered an exception. When `false`, promises will resolve without a value on timeout. + +Obviously, this option only has an effect when `timeout` is defined. ##### autoStart