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

Switch to Variadic Tuple Types for reducers and selectors #2715

Closed
alex-okrushko opened this issue Sep 14, 2020 · 6 comments
Closed

Switch to Variadic Tuple Types for reducers and selectors #2715

alex-okrushko opened this issue Sep 14, 2020 · 6 comments

Comments

@alex-okrushko
Copy link
Member

Describe any alternatives/workarounds you're currently using

Angular release 10.1 added TS 4.0 support (https://github.com/angular/angular/blob/master/CHANGELOG.md#1010-2020-09-02) so we'd want to get that as well. This will enable us to use Variadic Tuple Types.

Other information:

If accepted, I would be willing to submit a PR for this feature

[x] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

@david-shortman
Copy link
Contributor

I'm interested in helping add the types. Would want to know how I should go about this since the repo doesn't currently use 4.0+ yet.

@markostanimirovic
Copy link
Member

Issue with switching createSelector to variadic tuples: microsoft/TypeScript#41709

@david-shortman
Copy link
Contributor

david-shortman commented Jan 30, 2021

Here's at first shot at defining them.

export function createSelector<State, S extends any[], Result>(
  selectors: [...{ [i in keyof S]: Selector<State, S[i]> }],
  projector: (...args: [...S]) => Result
): MemoizedSelector<State, Result>;
export function createSelector< State, S extends any[], Result>(
  ...args: [
    ...{ [i in keyof S]: Selector<State, S[i]> },
    (...args: [...S]) => Result
  ]
): MemoizedSelector<State, Result>;
export function createSelector<State, Props, S extends any[], Result>(
  selectors: [...{ [i in keyof S]: SelectorWithProps<State, Props, S[i]> }],
  projector: (...args: [...S]) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
export function createSelector<State, Props, S extends any[], Result>(
  ...args: [
    ...{ [i in keyof S]: SelectorWithProps<State, Props, S[i]> },
    (...args: [...S, Props]) => Result
  ]
): MemoizedSelectorWithProps<State, Props, Result>;

But I'm hitting a problem where Typescript is not able to infer State properly, resulting in this error:

(in `router-store/src/router_selectors.ts)

Argument of type '(state: V) => RouterReducerState<any>' is not assignable to parameter of type 'Selector<unknown, RouterReducerState<any>>'. 
  Types of parameters 'state' and 'state' are incompatible.
    Type 'unknown' is not assignable to type 'V'.
      'V' could be instantiated with an arbitrary type which could be unrelated to 'unknown'.

As you can see, the type of createSelector has unknown as the type for state. Not sure why.
Screen Shot 2021-01-30 at 17 44 36

@david-shortman
Copy link
Contributor

david-shortman commented Jan 31, 2021

So the problem I was having in #2905 is that typescript doesn't seem to interpret variadic tuple type with strict order. It just creates a basic union type for all elements in the array.
This is problematic for createReducer as we want to have a function signature for a function that takes in a variadic number of selectors, followed by a single function whose parameters are all the return types of the reducers.

Instead, Typescript came up with a definition like this for the following:

/* derived:
export function createSelector<Selector<any, any>[], any>(
   ...args: Selector<any, any> | ((...args: any[]) => any)[]):
    MemoizedSelector<any, any, DefaultProjectorFn<any>>
*/
const selector = createSelector(
        incrementOne,
        incrementTwo,
        (state: any, props: any) => props.value,
        projectFn
      );

A very simple example of my expectations vs reality:

playground

type NumberArray = number[];
type StringArray = string[];
// I'd like a type that is a tuple of a variadic number of numbers followed by a variadic number of string
type Combined = [...numbers: NumberArray, ...strings: StringArray];

function thing(...args: Combined) {
  return 3;
}

thing('5', 5); // this should result in an error, right?

@david-shortman
Copy link
Contributor

...And now I see a comment from Alex in #2894 mentioning microsoft/TypeScript#41544 and that might be the fix needed to get the kind of type desired for createReducer

@david-shortman
Copy link
Contributor

Can we make a checklist of what still needs to be converted? Happy to help get items done.

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

3 participants