sindresorhus / type-fest

A collection of essential TypeScript types
Creative Commons Zero v1.0 Universal
14k stars 531 forks source link

Require parameters in functions #484

Open fregante opened 1 year ago

fregante commented 1 year ago

TypeScript is too loose when accepting functions:

type Fn = (a: string) => string;

const fn: Fn = () => '🥲'
// This is fine.

https://www.typescriptlang.org/play?#code/C4TwDgpgBAYgdlAvFAFAQwFxQM7AE4CWcA5gJRIB8O+RxA3AFAMDGA9nLlAGZxbxKpyiKgHJAfBuAnXZFA

Do you think there's a way to enforce the usage of the a parameter?

I have a situation where a function should only be used if a is required; if a isn't used, it's pointless.

A native example would be this: There's no point in using .some() if you're not going to use the array value inside the callback.

[0,12,345].some(() => true)

Upvote & Fund

Fund with Polar

sindresorhus commented 1 year ago

I immediately thought it was a bug until I tried older TS versions and it seems it has always behaved like this... I tried some hacks, but could not get it to require the function parameter.

fregante commented 1 year ago

Yeah, it does make sense most of the times as a default because you don't always need the parameter, but sometimes you do.

NiGhTTraX commented 1 year ago

A function type is a 2 way contract: I give the function some data, it gives me a return value. If an implementation of that contract can give me the same return value without using any of the data, then fine by me 👍

I have a situation where a function should only be used if a is required; if a isn't used, it's pointless.

The callback in Array.some has the following definition: (value: T, index: number, array: T[]) => boolean. (value) => !!value is a valid callback, and it doesn't use the 2 last arguments, so it's fair to say that a callback ignoring all 3 is also valid. () => true could mean "opt out of filtering" for someone, while () => false could be used inside a test 🤷

fregante commented 1 year ago

I'm not arguing that they should all be required, in fact I mentioned that the current default makes sense in most situations.

Tests are full of exceptions so I'm not going to consider that excuse valid.

sindresorhus commented 1 year ago

There is currently not a way in TypeScript to indicate that a callback parameter must be present.

NiGhTTraX commented 1 year ago

you don't always need the parameter, but sometimes you do

And that's up to the implementation of the type to decide. You cannot force a function to read its parameters, because what happens inside the function is an implementation detail.

Maybe you can share what you're trying to do, where you think reading the parameters is important? Regardless, TS doesn't support what you're asking for, see the 2nd link posted by Sindre above.

fregante commented 1 year ago

Basically this: https://github.com/n1ru4l/use-async-effect

This hook is only useful because it provides isMounted to the callback. If you don't use it, it's indistinguishable from a plain useEffect hook.

NiGhTTraX commented 1 year ago

If you don't use it, it's indistinguishable from a plain useEffect hook.

This sounds like a benefit to me. I can use the same hook for both sync and async effects, and it provides me the means to implement the latter.

fregante commented 1 year ago

Yes, but also the specific hook is not only useful, it's mandatory. If you have an await call and then setState, React will complain when the state is set after the component has been unmounted. This means every single await should theoretically be followed by isMounted (even though it's based on a deprecated practice, which isn't related to this issue, it's an example)

ggrandi commented 1 year ago

The only way I know to make sure that a callback has at least one argument is by using a generic like this (based on Array\<number>.some):

declare const some:
    <Callback extends (arg: number, index: number, arr: number[]) => boolean>
        // the parameters of the callback have to have a 0 property
        (fn: Parameters<Callback> extends { 0: unknown } ? Callback : "requires at least one parameter") => boolean

some((a) => !!a)
//@ts-expect-error
some(() => false)
NiGhTTraX commented 1 year ago

Consider inversion of control then — instead of relying on the callback to call/use the parameters you pass in, move that responsibility into the consumer. useAsyncEffect does this by having the callback return a generator, and then taking care of the isMounted logic when consuming it.

skarab42 commented 1 year ago

Inspired by the solution proposed by @ggrandi, I made a proto for a new type.

ClampParametersLength - Playground

type SomeCallback = (arg: number, index: number, arr: number[]) => boolean;

declare function some<Callback extends SomeCallback>(fn: ClampParametersLength<Callback, SomeCallback>): boolean

some(() => false) // Fail -> Argument of type '() => false' is not assignable to parameter of type '"expected 3 parameters, received 0"'.
some((a) => false) // Fail
some((a, b, c) => true) // Pass
some((a, b, c, d) => false) // Fail

EDIT: Fixed playground link