Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Destructuring or spreading createSelector() args as arrays TypeScript error #340

Closed
AlexeiDarmin opened this issue Apr 19, 2018 · 9 comments

Comments

@AlexeiDarmin
Copy link

AlexeiDarmin commented Apr 19, 2018

Hi, I'm trying to create selectorFactoryFactories however am having trouble with the types and was wondering if there's something I'm missing, or if it's a bug, or if it's impossible. Any and all help would be appreciated.

Edit* The destructuring method works perfectly during runtime, the issue is with the type errors being thrown.

The below case works as expected.

createSelector([
    regularSelectorA,
    selectorFactory('B'),
    selectorFactory('C'),
  ],
  (a, b, c ) => return { a, b, c })

However when I destructure part of the array or the complete array, typescript complains.

createSelector(...[
    regularSelectorA,
    selectorFactory('B'),
    selectorFactory('C'),
  ],
  (a, b, c ) => return { a, b, c })

In the above case typescript errors with expected 2-13 arguments but got 1 or more

createSelector([
    regularSelectorA,
    ...selectorFactoryFactory('B', 'C')
  ],
  (a, b, c ) => return { a, b, c })

In the above typescript also errors with

Argument of type '(OutputSelector<SharedRootState, ReadonlyRecord...' is not assignable to parameter of type '[ParametricSelector<SharedRootState, {}, ReadonlyRecord"...'. 

Property '0' is missing in type '(OutputSelector<SharedRootState, ReadonlyRecord<BaseEntity

The definition for selectorFactory and selectorFactoryFactory and regularSelector

function regularSelector(state: RootState) { return state.slice }

const selectorFactory= (attribute) => {
  return createSelector(regularSelector, function (storeState) {
    return storeState && storeState[attribute];
  });
}

const selectorFactoryFactory = (...attributes) => {
    return attributes.map(att => selectorFactory(att)
}
@ellbee ellbee closed this as completed Jun 22, 2018
@ellbee ellbee reopened this Jun 22, 2018
@ellbee ellbee changed the title Destructuring or spreading createSelector() args as arrays type error Destructuring or spreading createSelector() args as arrays TypeScript error Jun 22, 2018
@blackwood
Copy link

blackwood commented Jun 22, 2018

This is a funky issue. I was able to find a workaround for your last scenario, but this doesn't help if the number of values returned from selectorFactoryFactory is variable:

export type PS = ParametricSelector<any, {}, any>;

const selectorArray = concat<PS>(regularSelectorA, selectorFactoryFactory('B', 'C'));
// or if you want to use spread
const selectorArray = [regularSelectorA, ...selectorFactoryFactory('B', 'C')] as PS[];

createSelector(
  selectorArray as [PS, PS, PS],
  (a, b, c) => ({ a, b, c }));

@blackwood
Copy link

blackwood commented Jun 22, 2018

But I also think your second case is not a reselect type definition issue, it's a language issue, see this example:

const add2 = (a: number, b: number) => a + b;
add2(...[2, 3]);
// [ts] Expected 2 arguments, but got 0 or more.

http://www.typescriptlang.org/play/#src=const%20add2%20%3D%20(a%3A%20number%2C%20b%3A%20number)%20%3D%3E%20a%20%2B%20b%3B%0D%0Aadd2(...%5B2%2C%203%5D)%3B

microsoft/TypeScript#5296

side note, here is an extremely weird way of fixing this, but if you're using any that sort of defeats the point of typing:

const add2 = (a: number, b: number) => a + b;
(add2 as any)(...[2, 3]);

@blackwood
Copy link

Looks like there's already even been some prior thinking on this?
#143

@OliverJAsh
Copy link

OliverJAsh commented Jun 22, 2018

const add2 = (a: number, b: number) => a + b;
add2(...[2, 3]);
// [ts] Expected 2 arguments, but got 0 or more.

I think the issue with spreading an array into a function's arguments (above example) is that TypeScript sees the type of [2, 3] as an array (type number[]), which can have any length, so when you spread it, there's no way for TypeScript to know the array has the correct length to fulfll the function's arguments.

You can annotate [2, 3] as a tuple ([number, number]), in which case TypeScript would be able to know whether the tuple length fulfils the function's arguments, but unfortunately it doesn't seem to (microsoft/TypeScript#24241):

declare const createSelector: (selectors: number[], fn: Function) => void;

createSelector([1,2,3], () => {});
// type error
createSelector(...[1,2,3], () => {});
const tuple: [number, number, number] = [1, 2, 3];
// type error
createSelector(...tuple, () => {});
createSelector([1,...[2,3]], () => {})

As for spreading into the array, I can't reproduce the problem. Here is the code I tried:

// no error
createSelector([selectorFactory('a'), ...selectorFactoryFactory('b', 'c')], (a, b, c) => ({ a, b, c }))

@AlexeiDarmin
Copy link
Author

AlexeiDarmin commented Jun 25, 2018

I'm happy to see this ticket getting some love, thanks everyone!

@OliverJAsh You hit the nail on the head with:

I think the issue with spreading an array into a function's arguments (above example) is that TypeScript sees the type of [2, 3] as an array (type number[]), which can have any length, so when you spread it, there's no way for TypeScript to know the array has the correct length to fulfll the function's arguments.

Reselect also requires a minimum of two args for the create selector function, so either we'd have to tweak the definitions to allow 0 or more, or find a clever way to decorate what we're passing that is readable as 1 or more

/* one selector */
export function createSelector<S, R1, T>(
  selectors: [Selector<S, R1>],
  combiner: (res: R1) => T,
): OutputSelector<S, T, (res: R1) => T>;
export function createSelector<S, P, R1, T>(
  selectors: [ParametricSelector<S, P, R1>],
  combiner: (res: R1) => T,
): OutputParametricSelector<S, P, T, (res: R1) => T>;

I don't think Typing the input as selector[] is possible since it loses the relationship between the selector's R1 and the parametric selector's R1 .

In the below example an error isn't being thrown because the createSelector has a simplified definition via the number[], which works for the example but I'm not sure how to make it work for Reselect.

declare const createSelector: (selectors: number[], fn: Function) => void;

@AlexeiDarmin
Copy link
Author

If variadic types are added to TypeScript, that could be one way of going about this, however it's been under discussion since 2015 so I wouldn't expect it any time soon.
microsoft/TypeScript#5453

@AlexeiDarmin
Copy link
Author

Microsoft announced TypeScript 3 yesterday which allows for generic typing of the spread operator.

We may be able to simplify the Type file for reselect since instead of doing overloads like

// TODO (billg): 5 overloads should *probably* be enough for anybody?
function call<T1, T2, T3, T4, R>(fn: (param1: T1, param2: T2, param3: T3, param4: T4) => R, param1: T1, param2: T2, param3: T3, param4: T4): R
function call<T1, T2, T3, R>(fn: (param1: T1, param2: T2, param3: T3) => R, param1: T1, param2: T2, param3: T3): R
function call<T1, T2, R>(fn: (param1: T1, param2: T2) => R, param1: T1, param2: T2): R
function call<T1, R>(fn: (param1: T1) => R, param1: T1): R;
function call<R>(fn: () => R, param1: T1): R;
function call(fn: (...args: any[]) => any, ...args: any[]) {
    return fn(...args);
}

We would be able to do something like
function call<TS extends any[], R>(fn: (...args: TS) => R, ...args: TS): R { return fn(...args); }

And this would remove the typescript cap of 13 selectors for reselect.

@gustavopch
Copy link

Any news on the cap of 13 selectors now that we have TypeScript 3?

@markerikson
Copy link
Contributor

Should be fixed by #486 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants