Typescript: Selector type does not handle additional arguments

[dahannes]

Do you want to request a feature or report a bug?

bug

What is the current behaviour?

Passing types to the example with multiple arguments from the docs leads to type errors.

What is the expected behaviour?

No type errors.

Steps to Reproduce the Problem

Assign ts types to createCachedSelector, like:

const getPieceOfData = createCachedSelector<StateType, StateType, ItemIdType, DataTypeType, CombinerReturnType>(
  state => state, // selector1: ok
  (state, itemId) => itemId, // selector2: is not assignable to type Selector because it has more than one argument.
  (state, itemId, dataType) => dataType, // selector3: same issue as selector2
  (state, itemId, dataType) => expensiveComputation(state, itemId, dataType)
)(
  (state, itemId, dataType) => dataType // Use dataType as cacheKey
);

Manually changing the Selector type from export type Selector<S, R> = (state: S) => R; to export type Selector<S, R> = (state: S ,...args: any[]) => R; would fix the issue.

[toomuchdesign]

Hi @dahannes, thanks for reporting.

I had a look to the typing declaration file and the proposed solution. I'll open PR to try it out.

This library's typings were written from reselect's typings and they are still similar. Is this bug reproducible with reselect, too?

Greetings!

[toomuchdesign]

I see that the typing error is related to the generic types used in the example: <StateType, StateType, ItemIdType, DataTypeType, CombinerReturnType>

And the error pops up when using reselect, too.

What's the reason for having them in the selector declaration? It seems to me that they are not actually used.

[dahannes]

@toomuchdesign Thanks for getting back to me. I have not used reselect before but I see that it behaves the same in that regard.

Maybe I am understanding sth wrong, but I would like to pass the types to the createCachedSelector function once and not repeat the typings on every line. If I do so I get implicit any's for all arguments.

That could be fixed by changing the type as originally proposed to export type Selector<S, R> = (state: S ,...args: any[]) => R;

Anyhow that's actually not an ideal solution since it will just make the arguments explicit any's. In the end I would like TS to know exactly which arguments need to be passed to a selector function.

With the current typings it only expects state when it should actually show the types of the 3 arguments of the resolver function and the return type.

I will see if I can come up with a solution for it during the week.

[xsburg]

Hi @dahannes, @toomuchdesign

I can clearly see what you are trying to achieve here but unfortunately, the limitations of current typings do not allow us to do it.

First of all, the root cause of the error you see (implicit any) is that the typings do not support a dynamic number of arguments in the selector function. Only the first two arguments (see generic arguments S and P) are strongly typed while the rest are assumed as any. Because of that you have to explicitly set the types when using more arguments.

It could be fixed by using the new feature of TypeScript 3.0 called Generic rest parameters, so that we could do something like this: createCachedSelector<[State, string, number, boolean] /* Selector arguments */, [string, number, boolean] /* Resolver arguments */, boolean /* Return type */>. This approach would be ideal, but I've come across an issue during it's implementation and still looking for a solution. See my commit above, any help is much appreciated.

Alternatively, you can avoid explicit typing by using helper functions, for example:

const selectArgument1 = <T>(arg1: T): T => arg1;
const selectArgument2 = <T>(arg1: any, arg2: T): T => arg2;
const selectArgument3 = <T>(arg1: any, arg2: any, arg3: T): T => arg3;
const selectArgument4 = <T>(arg1: any, arg2: any, arg3: any, arg4: T): T => arg4;

const selector = createCachedSelector(
    selectArgument2,
    selectArgument3,
    selectArgument4,
    (arg1: string, arg2: number, arg3: boolean) => compute(arg1, arg2, arg3)
)(selectArgument2);

So in general, following FP style to compose a selector rather than create a new one will reduce typings. I've successfully used such approach in the past and it helps make some of the most annoying cases less annoying.

Another solution would be to combine all the extra arguments into only one extra argument props, which is strong typed and also can be typed explicitly in generic arguments (see generic parameter P).

[dahannes]

Thanks @xsburg, the forked typings work pretty well already. Only one problem remains: When I now call the selector function I get the correct types, but not the argument names. Instead it just requests args_0, args_1, etc.

But that's probably the best we can get atm I guess.

[xsburg]

@dahannes, that's one of the issues and also the reason why I normally redefine the type of the resulting selector explicitly, so that I have meaningful names.

Sadly, the more concerning problem is that you cannot specify selector functions without unnecessary arguments with these new typings:

const selector = createCachedSelector(
      (state: State, arg1: string) => arg1, // all OK
      (state: State, arg1: string, arg2: number) => arg2, // Error, type inference assumed the selector signature to be "(state: State, arg1: string)" and doesn't allow "arg2".
      (state: State, arg1: string, arg2: number, arg3: boolean) => arg3, // Same problem
      (arg1, arg2, arg3) => compute(arg1, arg2, arg3)
    )((state: State) => state.foo);

So basically, TS wrongly guesses the signature of the selector based on the first selector function, which is wrong in this case, and as a result, this quite common scenario falls apart. It's kind of a breaking change and might be very annoying, so I'm not sure if we should move forward with these new typings. At least until the problem is resolved and the typings are at least no worse than what we have now. Perhaps, another upcoming feature, which is "Concat/Split of tuples", will help us here.

On another hand, you are free to use these typings locally in your projects while they are still not merged, if this problem is unimportant.

[tboulis]

Hello! Any updates on this? Is there a workaround?