Improve the message error when attaching onCancel after the promise s…

…ettled (#31)

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

.github/workflows/main.yml

@@ -10,13 +10,13 @@ jobs:
       fail-fast: false
       matrix:
         node-version:
+          - 18
+          - 16
           - 14
           - 12
-          - 10
-          - 8
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-node@v1
+      - uses: actions/checkout@v3
+      - uses: actions/setup-node@v3
         with:
           node-version: ${{ matrix.node-version }}
       - run: npm install

index.d.ts

@@ -16,58 +16,12 @@ Accepts a function that is called when the promise is canceled.
 You're not required to call this function. You can call this function multiple times to add multiple cancel handlers.
 */
 export interface OnCancelFunction {
-	shouldReject: boolean;
 	(cancelHandler: () => void): void;
+	shouldReject: boolean;
 }
 
+// eslint-disable-next-line @typescript-eslint/naming-convention
 export default class PCancelable<ValueType> extends Promise<ValueType> {
-	/**
-	Whether the promise is canceled.
-	*/
-	readonly isCanceled: boolean;
-
-	/**
-	Cancel the promise and optionally provide a reason.
-
-	The cancellation is synchronous. Calling it after the promise has settled or multiple times does nothing.
-
-	@param reason - The cancellation reason to reject the promise with.
-	*/
-	cancel: (reason?: string) => void;
-
-	/**
-	Create a promise that can be canceled.
-
-	Can be constructed in the same was as a [`Promise` constructor](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Promise), but with an appended `onCancel` parameter in `executor`. `PCancelable` is a subclass of `Promise`.
-
-	Cancelling will reject the promise with `CancelError`. To avoid that, set `onCancel.shouldReject` to `false`.
-
-	@example
-	```
-	import PCancelable from 'p-cancelable';
-
-	const cancelablePromise = new PCancelable((resolve, reject, onCancel) => {
-		const job = new Job();
-
-		onCancel.shouldReject = false;
-		onCancel(() => {
-			job.stop();
-		});
-
-		job.on('finish', resolve);
-	});
-
-	cancelablePromise.cancel(); // Doesn't throw an error
-	```
-	*/
-	constructor(
-		executor: (
-			resolve: (value?: ValueType | PromiseLike<ValueType>) => void,
-			reject: (reason?: unknown) => void,
-			onCancel: OnCancelFunction
-		) => void
-	);
-
 	/**
 	Convenience method to make your promise-returning or async function cancelable.
 
@@ -145,7 +99,7 @@ export default class PCancelable<ValueType> extends Promise<ValueType> {
 		Agument3Type,
 		Agument4Type,
 		Agument5Type,
-		ReturnType
+		ReturnType,
 	>(
 		userFn: (
 			argument1: Agument1Type,
@@ -165,4 +119,51 @@ export default class PCancelable<ValueType> extends Promise<ValueType> {
 	static fn<ReturnType>(
 		userFn: (...arguments: unknown[]) => PromiseLike<ReturnType>
 	): (...arguments: unknown[]) => PCancelable<ReturnType>;
+
+	/**
+	Whether the promise is canceled.
+	*/
+	readonly isCanceled: boolean;
+
+	/**
+	Cancel the promise and optionally provide a reason.
+
+	The cancellation is synchronous. Calling it after the promise has settled or multiple times does nothing.
+
+	@param reason - The cancellation reason to reject the promise with.
+	*/
+	cancel: (reason?: string) => void;
+
+	/**
+	Create a promise that can be canceled.
+
+	Can be constructed in the same was as a [`Promise` constructor](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Promise), but with an appended `onCancel` parameter in `executor`. `PCancelable` is a subclass of `Promise`.
+
+	Cancelling will reject the promise with `CancelError`. To avoid that, set `onCancel.shouldReject` to `false`.
+
+	@example
+	```
+	import PCancelable from 'p-cancelable';
+
+	const cancelablePromise = new PCancelable((resolve, reject, onCancel) => {
+		const job = new Job();
+
+		onCancel.shouldReject = false;
+		onCancel(() => {
+			job.stop();
+		});
+
+		job.on('finish', resolve);
+	});
+
+	cancelablePromise.cancel(); // Doesn't throw an error
+	```
+	*/
+	constructor(
+		executor: (
+			resolve: (value?: ValueType | PromiseLike<ValueType>) => void,
+			reject: (reason?: unknown) => void,
+			onCancel: OnCancelFunction
+		) => void
+	);
 }

index.js

@@ -9,43 +9,48 @@ export class CancelError extends Error {
 	}
 }
 
+const promiseState = Object.freeze({
+	pending: Symbol('pending'),
+	canceled: Symbol('canceled'),
+	resolved: Symbol('resolved'),
+	rejected: Symbol('rejected'),
+});
+
 // TODO: Use private class fields when ESLint 8 is out.
 
 export default class PCancelable {
 	static fn(userFunction) {
-		return (...arguments_) => {
-			return new PCancelable((resolve, reject, onCancel) => {
-				arguments_.push(onCancel);
-				// eslint-disable-next-line promise/prefer-await-to-then
-				userFunction(...arguments_).then(resolve, reject);
-			});
-		};
+		return (...arguments_) => new PCancelable((resolve, reject, onCancel) => {
+			arguments_.push(onCancel);
+			userFunction(...arguments_).then(resolve, reject);
+		});
 	}
 
 	constructor(executor) {
 		this._cancelHandlers = [];
-		this._isPending = true;
-		this._isCanceled = false;
 		this._rejectOnCancel = true;
+		this._state = promiseState.pending;
 
 		this._promise = new Promise((resolve, reject) => {
 			this._reject = reject;
 
 			const onResolve = value => {
-				if (!this._isCanceled || !onCancel.shouldReject) {
-					this._isPending = false;
+				if (this._state !== promiseState.canceled || !onCancel.shouldReject) {
 					resolve(value);
+					this._state = promiseState.resolved;
 				}
 			};
 
 			const onReject = error => {
-				this._isPending = false;
-				reject(error);
+				if (this._state !== promiseState.canceled || !onCancel.shouldReject) {
+					reject(error);
+					this._state = promiseState.rejected;
+				}
 			};
 
 			const onCancel = handler => {
-				if (!this._isPending) {
-					throw new Error('The `onCancel` handler was attached after the promise settled.');
+				if (this._state !== promiseState.pending) {
+					throw new Error(`The \`onCancel\` handler was attached after the promise ${this._state.description}.`);
 				}
 
 				this._cancelHandlers.push(handler);
@@ -56,35 +61,32 @@ export default class PCancelable {
 					get: () => this._rejectOnCancel,
 					set: boolean => {
 						this._rejectOnCancel = boolean;
-					}
-				}
+					},
+				},
 			});
 
 			executor(onResolve, onReject, onCancel);
 		});
 	}
 
 	then(onFulfilled, onRejected) {
-		// eslint-disable-next-line promise/prefer-await-to-then
 		return this._promise.then(onFulfilled, onRejected);
 	}
 
 	catch(onRejected) {
-		// eslint-disable-next-line promise/prefer-await-to-then
 		return this._promise.catch(onRejected);
 	}
 
 	finally(onFinally) {
-		// eslint-disable-next-line promise/prefer-await-to-then
 		return this._promise.finally(onFinally);
 	}
 
 	cancel(reason) {
-		if (!this._isPending || this._isCanceled) {
+		if (this._state !== promiseState.pending) {
 			return;
 		}
 
-		this._isCanceled = true;
+		this._state = promiseState.canceled;
 
 		if (this._cancelHandlers.length > 0) {
 			try {
@@ -103,7 +105,7 @@ export default class PCancelable {
 	}
 
 	get isCanceled() {
-		return this._isCanceled;
+		return this._state === promiseState.canceled;
 	}
 }
 

index.test-d.ts

@@ -12,7 +12,7 @@ const cancelablePromise: PCancelable<number> = new PCancelable(
 		expectType<OnCancelFunction>(onCancel);
 		onCancel(() => 'foo');
 		onCancel.shouldReject = false;
-	}
+	},
 );
 
 cancelablePromise.cancel();
@@ -27,27 +27,25 @@ const function0 = PCancelable.fn(async onCancel => {
 expectType<() => PCancelable<number>>(function0);
 
 const function1 = PCancelable.fn(
-	async (_parameter1: string, _onCancel: OnCancelFunction) => Promise.resolve(10)
+	async (_parameter1: string, _onCancel: OnCancelFunction) => Promise.resolve(10),
 );
 expectType<(parameter1: string) => PCancelable<number>>(function1);
 
 const function2 = PCancelable.fn(
 	async (_parameter1: string, _parameter2: boolean, _onCancel: OnCancelFunction) =>
-		Promise.resolve(10)
+		Promise.resolve(10),
 );
 expectType<(_parameter1: string, _parameter2: boolean) => PCancelable<number>>(
-	function2
+	function2,
 );
 
 const function3 = PCancelable.fn(
 	async (
 		_parameter1: string,
 		_parameter2: boolean,
 		_parameter3: number,
-		_onCancel: OnCancelFunction
-	) => {
-		return Promise.resolve(10);
-	}
+		_onCancel: OnCancelFunction,
+	) => Promise.resolve(10),
 );
 expectType<
 (
@@ -63,10 +61,8 @@ const function4 = PCancelable.fn(
 		_parameter2: boolean,
 		_parameter3: number,
 		_parameter4: symbol,
-		_onCancel: OnCancelFunction
-	) => {
-		return Promise.resolve(10);
-	}
+		_onCancel: OnCancelFunction,
+	) => Promise.resolve(10),
 );
 expectType<
 (
@@ -84,10 +80,8 @@ const function5 = PCancelable.fn(
 		_parameter3: number,
 		_parameter4: symbol,
 		_parameter5: null,
-		_onCancel: OnCancelFunction
-	) => {
-		return Promise.resolve(10);
-	}
+		_onCancel: OnCancelFunction,
+	) => Promise.resolve(10),
 );
 expectType<
 (

package.json

@@ -45,6 +45,6 @@
 		"ava": "^3.15.0",
 		"delay": "^5.0.0",
 		"tsd": "^0.16.0",
-		"xo": "^0.40.1"
+		"xo": "^0.46.0"
 	}
 }