ioredis
ioredis copied to clipboard
Loosen callback typings
In @types/ioredis, callbacks are defined as (err: Error | null, res: T) => void, whereas in ioredis v5, they are (err?: Error | null, res?: T) => void.
The intention was to indicate the res can be absent when there is an error. However, given TypeScript doesn't provide a way to achieve "if there is no error, then res should be fulfilled", developers have to check res in every callback even if they have already checked err:
return new Promise((resolve, reject) => {
redis.get('foo', (err, res) => {
if (err) reject(err);
// Can't use `else` here
if (typeof res !== 'undefined') resolve(res);
});
});
I feel it's a bit inconvenient, and apparently, people have different opinions on it: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43583.
I lean towards keeping in line with the Node.js way: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/884d68c/types/node/fs.d.ts#L2416 though no strong opinion.
Open to any suggestions! 😄
Why not model the type exactly after ist usage?
export type Callback<T = any> = (err: Error, result: never) => void | (err: undefined, result: T) => void;
By the way, I believe that the more "TypeScript-y" default type for T would be unknown, resulting in
export type Callback<T = unknown> = (err: Error, result: never) => void | (err: undefined, result: T) => void;
@fabrykowski Don't think that works. TypeScript asks to give the parameters types.
My bad – I didn't check it with a compiler 😬
You can use tuples in rest parameters to achieve the goal:
export type Callback<T = unknown> = (...args: [err: Error, result: never] | [err: undefined, result: T]) => void;
In that case seems result will never be never 😢
If we check for the existence of the error, then result does become never.
Unfortunately if we don't check, the type of result is never | T, which TypeScript simplifies to T.
Of course result can be set to undefined:
export type Callback<T = unknown> = (...args: [err: Error, result: undefined] | [err: undefined, result: T]) => void;
playground We lose some of the semantics, but gain a functioning definition 😅
@fabrykowski Oh that's awesome! I didn't know that was possible. Looks like it's only supported from TypeScript 4.6 but seems to be a good progressive enhancement.