reduxjs / reselect

Selector library for Redux
MIT License
19.04k stars 670 forks source link

TypeScript typings for reselect? #55

Closed cveld closed 8 years ago

cveld commented 9 years ago

Do you provide TypeScript typings for reselect?

ellbee commented 9 years ago

I would be very happy if someone decided to create Typescript definitions. We did make some changes for the 1.0.0 release that should make them possible (#27).

faassen commented 9 years ago

It would also be interesting to see Flow types as well. I'm not sure whether the state of Flow is far enough along to cover all the ES6 we use. I wonder whether it's possible to generate one from the other.

inakianduaga commented 8 years ago

Need this as well, right now I'm debugging errors since 1/2 hour in my code due to incorrect manual types I added when composing selectors. If the compiler would know then it would be a ton of help (plus would reduce a lot of the manual effort needed to propagate the types). I would like to help exept that I'm pretty sure I don't know enough about the typing system to write the types definitions.

nizsheanez commented 8 years ago

for myself i created just most stupid definition:

declare module Reselect {
    function createSelector(...args: any[]): any;
}

declare module "reselect" {
    export = Reselect;
}
frankwallis commented 8 years ago

How about:

declare module "reselect" {
    type Selector<TInput, TOutput> = (arg1: TInput, arg2?: any) => TOutput;
    export function createSelector<TInput, TOutput, T1>(selector1: Selector<TInput, T1>, combiner: (arg1: T1) => TOutput): Selector<TInput, TOutput>;
    export function createSelector<TInput, TOutput, T1, T2>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, combiner: (arg1: T1, arg2: T2) => TOutput): Selector<TInput, TOutput>;
    export function createSelector<TInput, TOutput, T1, T2, T3>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, selector3: Selector<TInput, T3>, combiner: (arg1: T1, arg2: T2, arg3: T3) => TOutput): Selector<TInput, TOutput>;
    /* etc */
}
geon commented 8 years ago

@frankwallis , I just added your type definition to a project with some 40+ selectors. It seems to work pretty well.

For anyone who don't like to type a lot, here's some more of the "etc" part:

declare module Reselect {

    type Selector<TInput, TOutput> = (state: TInput, props?: any) => TOutput;
    function createSelector<TInput, TOutput, T1>(selector1: Selector<TInput, T1>, combiner: (arg1: T1) => TOutput): Selector<TInput, TOutput>;
    function createSelector<TInput, TOutput, T1, T2>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, combiner: (arg1: T1, arg2: T2) => TOutput): Selector<TInput, TOutput>;
    function createSelector<TInput, TOutput, T1, T2, T3>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, selector3: Selector<TInput, T3>, combiner: (arg1: T1, arg2: T2, arg3: T3) => TOutput): Selector<TInput, TOutput>;
    function createSelector<TInput, TOutput, T1, T2, T3, T4>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, selector3: Selector<TInput, T3>, selector4: Selector<TInput, T4>, combiner: (arg1: T1, arg2: T2, arg3: T3, arg4: T4) => TOutput): Selector<TInput, TOutput>;
    function createSelector<TInput, TOutput, T1, T2, T3, T4, T5>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, selector3: Selector<TInput, T3>, selector4: Selector<TInput, T4>, selector5: Selector<TInput, T5>, combiner: (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5) => TOutput): Selector<TInput, TOutput>;
    function createSelector<TInput, TOutput, T1, T2, T3, T4, T5, T6>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, selector3: Selector<TInput, T3>, selector4: Selector<TInput, T4>, selector5: Selector<TInput, T5>, selector6: Selector<TInput, T6>, combiner: (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, arg6: T6) => TOutput): Selector<TInput, TOutput>;
    function createSelector<TInput, TOutput, T1, T2, T3, T4, T5, T6, T7>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, selector3: Selector<TInput, T3>, selector4: Selector<TInput, T4>, selector5: Selector<TInput, T5>, selector6: Selector<TInput, T6>, selector7: Selector<TInput, T7>, combiner: (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, arg6: T6, arg7: T7) => TOutput): Selector<TInput, TOutput>;
    function createSelector<TInput, TOutput, T1, T2, T3, T4, T5, T6, T7, T8>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, selector3: Selector<TInput, T3>, selector4: Selector<TInput, T4>, selector5: Selector<TInput, T5>, selector6: Selector<TInput, T6>, selector7: Selector<TInput, T7>, selector8: Selector<TInput, T8>, combiner: (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, arg6: T6, arg7: T7, arg8: T8) => TOutput): Selector<TInput, TOutput>;
    function createSelector<TInput, TOutput, T1, T2, T3, T4, T5, T6, T7, T8, T9>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, selector3: Selector<TInput, T3>, selector4: Selector<TInput, T4>, selector5: Selector<TInput, T5>, selector6: Selector<TInput, T6>, selector7: Selector<TInput, T7>, selector8: Selector<TInput, T8>, selector9: Selector<TInput, T9>, combiner: (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, arg6: T6, arg7: T7, arg8: T8, arg9: T9) => TOutput): Selector<TInput, TOutput>;
    function createSelector<TInput, TOutput, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, selector3: Selector<TInput, T3>, selector4: Selector<TInput, T4>, selector5: Selector<TInput, T5>, selector6: Selector<TInput, T6>, selector7: Selector<TInput, T7>, selector8: Selector<TInput, T8>, selector9: Selector<TInput, T9>, selector10: Selector<TInput, T10>, combiner: (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, arg6: T6, arg7: T7, arg8: T8, arg9: T9, arg10: T10) => TOutput): Selector<TInput, TOutput>;
    function createSelector<TInput, TOutput, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11>(selector1: Selector<TInput, T1>, selector2: Selector<TInput, T2>, selector3: Selector<TInput, T3>, selector4: Selector<TInput, T4>, selector5: Selector<TInput, T5>, selector6: Selector<TInput, T6>, selector7: Selector<TInput, T7>, selector8: Selector<TInput, T8>, selector9: Selector<TInput, T9>, selector10: Selector<TInput, T10>, selector11: Selector<TInput, T11>, combiner: (arg1: T1, arg2: T2, arg3: T3, arg4: T4, arg5: T5, arg6: T6, arg7: T7, arg8: T8, arg9: T9, arg10: T10, arg11: T11) => TOutput): Selector<TInput, TOutput>;

    function defaultMemoize(func:Function, equalityCheck:any): any;

    function createSelectorCreator(memoize:Function, ...memoizeOptions:any[]): any;

    function createStructuredSelector(inputSelectors:any, selectorCreator:any): any;

}

declare module "reselect" {
    export = Reselect
}
frankwallis commented 8 years ago

Yes they're working pretty well for me too, and providing type-inference in the 'combiner' function which is really useful. Thanks for filling out more of the overloads, I've submitted a PR to DefinitelyTyped.

inakianduaga commented 8 years ago

@frankwallis works nicely, thanks a bunch!

ellbee commented 8 years ago

Hey, this is great, thanks everyone! I'll add a link to here from the readme.

Do you think you could let me know if they accept the PR to DefinitelyTyped please? I'll update the readme to point there instead if that happens.

frankwallis commented 8 years ago

Ok will do - I'm not sure on the best way to get it merged but it may help if the PR has been 'approved' by a member of the reselect project...

Until it is merged if you are using tsd you can manually put the PR commit hash in tsd.json to install this declaration file:

    "reselect/reselect.d.ts": {
      "commit": "7ee8c031f981bfb4fd1dd511d3edf598ab96e5bb"
    },
frankwallis commented 8 years ago

That PR has now been merged.

ellbee commented 8 years ago

Thanks

ianks commented 8 years ago

Would you guys be willing to include the typings in the NPM package? The main benefit being that the typings can be managed directly in this repo, and users do not have to go to a third-party source to get typings for this lib. I would be more than willing to implement this.

https://github.com/Microsoft/TypeScript/wiki/Typings-for-npm-packages

ellbee commented 8 years ago

@ianks Yes, please do!

geon commented 8 years ago

After having used the typing for a while, I noticed it isn't playing well with connectfrom react-redux.

Since the props argument in the Selector<> function is any, all component wrappers created by connect are inferred to have the props be any. That sucks.

So I'm experimenting with this instead:


    type Selector<TInput, TProps, TOutput> = (state: TInput, props?: TProps) => TOutput;

    function createSelector<TInput, TProps, TOutput, T1>(selector1: Selector<TInput, TProps, T1>, combiner: (arg1: T1) => TOutput): Selector<TInput, TProps, TOutput>;
    function createSelector<TInput, TProps, TOutput, T1, T2>(selector1: Selector<TInput, TProps, T1>, selector2: Selector<TInput, TProps, T2>, combiner: (arg1: T1, arg2: T2) => TOutput): Selector<TInput, TProps, TOutput>;

Seems to work fine. Any comments?

ellbee commented 8 years ago

Thanks for the suggestion. I don't know Typescript well enough to be confident in commenting.

@ianks @frankwallis @geon: do any of you want commit access to Reselect to take care of Typescript issues?

ianks commented 8 years ago

@geon that looks good. can you create a minimal example case for this (just to show that that types are retained from connect)?

geon commented 8 years ago

@ianks

Here's a semi-realistic example of what our code looks like:

delete_button_component.ts


export type DeleteButtonProps = DeleteButtonStateProps & DeleteButtonDispatchProps;

export type DeleteButtonStateProps = {
    disabled: boolean
}

export type DeleteButtonDispatchProps = {
    onClick: event => void
}

export function deleteButton({
    disabled,
    onClick
}: DeleteButtonProps) {

    return DOM.button({
        disabled,
        onClick
    }, 'Delete');
}

delete_button_container.ts


type DeleteButtonContainerProps = {
    itemId: string
}

const mapStateToProps = createSelector<
    // NOTE! 3 types for the input/output instead of 2 as before.
    RootState, DeleteButtonContainerProps, DeleteButtonStateProps,
    boolean
> (
    (state, props) => !!state.items[props.itemId],
    (itemExists: boolean) => ({
        disabled: !itemExists
    })
);

function mapDispatchToProps (dispatch: ReactRedux.Dispatch, props: DeleteButtonContainerProps): DeleteButtonDispatchProps {

    return {
        onClick: event => dispatch(actions.deleteItem(props.itemId))
    };
}

export const DeleteButton = connect(mapStateToProps, mapDispatchToProps)(deleteButton);

Now, calling the container with anything else than DeleteButtonContainerProps will not compile.

Using it:


function render () {

    return DOM.div({}
        SomeItemView({
            foo: 'foo',
            bar: 'bar',
            itemId: theItemId
        }),
        DeleteButton({
            itemId: theItemId
        })
    );
}

We don't use jsx, so I don't really know how to write a more idiomatic example. Seems like nobody else use the DOM api manually. :P

This code has obviously never run, so I'm sure there are some bugs in it. But I think you get the gist.

Without the props typing I suggested, DeleteButton would accept any extra and missing props.

geon commented 8 years ago

@ellbee , I'm not sure I should have commit access. I still feel like a react noob. I've only been using it since christmas.

frankwallis commented 8 years ago

I need to just be sure that it does not become necessary to specify the type parameters when calling createSelector, currently I am using it like this (simplified):

interface GlobalState {
   products: { [id: number] : Product; }
   clients: { [id: string] : Client; }
}

interface MyComponentProps {
   clientId: string;
}
const clientRepoSelector = (state: GlobalState) => state.clients;
const clientIdSelector = (state: GlobalState, props: MyComponentProps) => props.clientId;

const mapStateToProps = createSelector(
   clientRepoSelector,
   clientIdSelector,
   (clientRepo, clientId) => clientRepo[clientId]
);

So I specify the shape of the global state and the inputs to the functions and then get type inference in the 'combiner' function and the resulting selectors. The type inference feeds into other selectors when you start combining them.

I would be quite unhappy if I had to start specifying all the type parameters whenever I called createSelector, so I would like to check that first.

Can I also ask if you are using connect as a decorator or as a plain function as I think that can also affect the type of the resulting component.

geon commented 8 years ago

I would be quite unhappy if I had to start specifying all the type parameters

Yeah. For some reason, we didn't get that to work earlier. Possible because we had bad typing in other places. (We migrated to ts with a lot of js already written.) I should try again.

Can I also ask if you are using connect as a decorator or as a plain function

We only use it as a function, since 99% of our components are stateless functions, not classes.

I suppose there really should be a few test cases to ensure the typings work in all the intended situations.

geon commented 8 years ago

@frankwallis

I tried using createSelector without explicit type parameters. It seems it will work fine, as long as the "primitive" selectors in the arguments are explicitly typed, just like in your example.

Modified example from above (that actually compiles, yay!):


const mapStateToProps = Reselect.createSelector(
    (state: RootState, props: DeleteButtonContainerProps) => !!state.items[props.itemId],
    (itemExists: boolean) => ({
        disabled: !itemExists
    })
);

I suppose it would be impossible to not type it explicitly somewhere. In our case, we have a lot ot "primitive" selectors, so it is nice to be able to type them in createSelector. A trade-off.

frankwallis commented 8 years ago

So I have given this a try, I did get some compiler errors because I am using the Selector type as the input to some of my selector creation functions, and these now fail because they are missing a type parameter e.g.

export function createListPropsSelector<TItem>(listStoreSelector: Selector<GlobalState, ListStore>, repositorySelector: Selector<GlobalState, Repository<TItem>>): Selector<GlobalState, ListProps<TItem>>;

Unfortunately it is not possible to specify default values for generic parameters, but hopefully that will be possible soon (see here). If it were possible it sounds like they would have to be at the end of the parameter list so I think that the TProps param should be moved to the end.

I have this working and backwards compatible as follows:

    type PropsSelector<TInput, TOutput, TProps> = (state: TInput, props?: TProps) => TOutput;
    type Selector<TInput, TOutput> = PropsSelector<TInput, TOutput, any>;

    function createSelector<TInput, TOutput, T1, TProps>(selector1: PropsSelector<TInput, T1, TProps>, combiner: (arg1: T1) => TOutput): PropsSelector<TInput, TOutput, TProps>;
    function createSelector<TInput, TOutput, T1, T2, TProps>(selector1: PropsSelector<TInput, T1, TProps>, selector2: PropsSelector<TInput, T2, TProps>, combiner: (arg1: T1, arg2: T2) => TOutput): PropsSelector<TInput, TOutput, TProps>;
    function createSelector<TInput, TOutput, T1, T2, T3, TProps>(selector1: PropsSelector<TInput, T1, TProps>, selector2: PropsSelector<TInput, T2, TProps>, selector3: PropsSelector<TInput, T3, TProps>, combiner: (arg1: T1, arg2: T2, arg3: T3) => TOutput): PropsSelector<TInput, TOutput, TProps>;

When default generic parameters are available it will be possible to unify PropsSelector and Selector

If people are already calling createSelector with the type-parameters then they will get errors due to the missing TProps type parameter, so for back-compatibility I think it is necessary to keep the existing declarations as overrides as well, ie

    function createSelector<TInput, TOutput, T1, TProps>(selector1: PropsSelector<TInput, T1, TProps>, combiner: (arg1: T1) => TOutput): PropsSelector<TInput, TOutput, TProps>;
    function createSelector<TInput, TOutput, T1>(selector1: Selector<TInput, T1>, combiner: (arg1: T1) => TOutput): Selector<TInput, TOutput>;
geon commented 8 years ago

I added tests and props typing in a pull-request. Please review: https://github.com/reactjs/reselect/pull/115

vjau commented 8 years ago

For some reason, i have the typescript compiler complaining that the reselect module is unknown. I get this error : Error:(1, 30) TS2307:Cannot find module 'reselect'. with code

import {createSelector} from 'reselect';
threehams commented 8 years ago

@vjau Can you open a new issue with information on TypeScript version and contents of tsconfig.json? TS2307 can happen for hundreds of different reasons.

screenpunch commented 8 years ago

I had this error when "module" in my tsconfig.json was set "es6", try importing using

const { createSelector } = require('reselect');

ianks commented 8 years ago

@screenpunch you do not get the typed version if you load it that way.

aikoven commented 7 years ago

To everyone interested, please help review updated typings: https://github.com/reactjs/reselect/pull/192