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

Better typings for connect #160

Open
yannick-cw opened this issue Jun 28, 2019 · 2 comments
Open

Better typings for connect #160

yannick-cw opened this issue Jun 28, 2019 · 2 comments

Comments

@yannick-cw
Copy link

yannick-cw commented Jun 28, 2019

Hey,
using the current connect in typescript seems to give not as much type safety as possible. I came ob with these types/changes to make it more safe for us, as we had it more than once that we actually broke things without these types.

export const safeConnect = <T, S, K, I>(
  pickFromState: (keyof K) & (keyof I) & string | Array<(keyof K) & (keyof I) & string> | StateMapper<T, K, I>,
  actions?: PropsActions<I, K> | Array<PropsActions<I, K>> | SafeActionCreator<K, I>,
): ((Child: ComponentConstructor<T & I, S> | AnyComponent<T & I, S>) => ComponentConstructor<T, S>) =>
  connect<T, S, K, I>(
    pickFromState,
    Array.isArray(actions) ? actions.reduce((prev, current) => ({ ...prev, ...current }), {}) : actions,
  );

export type SafeActionCreator<K, I> = (store: Store<K>) => PropsActions<I, K>;

type PropsActions<I extends Record<string, any>, K> = {
  [P in keyof Partial<I>]: I[P] extends ((...args: any[]) => void)
    ? (s: State, ...a: Parameters<I[P]>) => Promise<Partial<K>> | Partial<K> | void
    : never
};

This ensures a few things, maybe an example shows that better:

export interface Store {
  counter: number,
}

export const counterAction = {
  setCounter: (_: State, n: number) => ({ counter: n }),
};

export interface ComponentConnectedProps {
  counter: number;
  setCounter: (count: number) => void
}

// usage
export const ConnectedComponent = safeConnect<{}, {}, Store, ComponentConnectedProps>(
  ['counter'], // or 'counter'
  [counterAction], // or counterAction
)(Component);
  1. pickFromState parameter from safeConnect
  • needs to be a key of the Store and the ComponentConnectedProps
  1. actions parameter from safeConnect
  • key of the action object needs to be in the ComponentConnectedProps
  • function parameter (without the State) needs to match the one in ComponentConnectedProps
  • return type of the action function needs to be Partial of the state

The one thing it can not ensure, is that all of the items from ComponentConnectedProps are there.

Is there interest to get some version of this upstream?

@yannick-cw
Copy link
Author

I think it would also make sense to make the StateMapper<T, K, I> Partial on I. As otherwise you can not use it with actions, if I am not mistaken

@sjinks
Copy link

sjinks commented Nov 15, 2019

I use another approach — I provide another signature for connect() in custom.d.ts. Something like this:

declare module 'unistore/preact' {
    import { AnyComponent, ComponentConstructor } from 'preact';
    import { ActionCreator, StateMapper, Store, ActionMap, ActionFn } from 'unistore';

    type TupleTail<T extends any[]> = T['length'] extends 0
        ? never
        : ((...tail: T) => void) extends (head: any, ...tail: infer I) => void
        ? I
        : never;

    type MakeBoundAction<K, F extends (...args: any) => ReturnType<ActionFn<K>>> = (
        ...args: TupleTail<Parameters<F>>
    ) => void;
    type BoundActionFn<K> = MakeBoundAction<K, ActionFn<K>>;

    export type ActionBinder<K, T extends object> = {
        [P in keyof T]: BoundActionFn<T[P]>;
    };

    export function connect<T, S, K, I, A extends ActionMap<K>>(
        mapStateToProps: string | string[] | StateMapper<T, K, I>,
        actions: A | ((store: Store<K>) => A),
    ): (
        Child: ComponentConstructor<T & I & ActionBinder<K, A>, S> | AnyComponent<T & I & ActionBinder<K, A>, S>,
    ) => ComponentConstructor<T | (T & I & ActionBinder<K, A>), S>;
}

Benefits:

  1. This separates injected props from action props, and enables type checking of actions: with the current typings we can pass function someAction(x: number): void {} as an action, and the compiler will not complain, although the signature of the function is incorrect
  2. There is only one source of truth for action props: with the current typings you have to check that props defined in the component interface match those passed to connect()
  3. This takes into account that connect() passed actions bound to the state to the component.
  4. mapStateToProps() does not have to care about action props, and it is OK to use StateMapper<T, K, I> instead of StateMapper<T, K, Partial<I>> (which may leave some props undefined)

Drawbacks:

  1. One extra type parameter to connect()
  2. Action props have to inherit from ActionMap<K>

How it works: most of the magic happens in MakeBoundAction: what it does is converts ActionFn<K> (which is (state: K, ...args: any): Partial<K> | Promise<Partial<K>> | void) into (...args: any): void by stripping state: K parameter. It also accounts for the case when the action does not have state parameter in its signature. ActionBinder, using this helpers, converts ActionMap (which is basically a Record<string, ActionFn<K>>) into Record<string, BoundActionFn<K>>. This is necessary to make sure that the connected component is passed action props of the correct type.

Example:

interface AppState {
    property: string;
}

function someAction(state: AppState, param: string): void {}

// Our component
interface OwnProps {
    someProperty: string;
}

interface InjectedProps {
    injectedProperty: string;
}

interface ActionProps extends ActionMap<AppState> {
    someAction: typeof someAction;
}

type Props = OwnProps & InjectedProps & ActionBinder<AppState, ActionProps>;
type State = { /* ... */ };

class MyComponent extends Component<Props, State> { /* ... */ }

function mapStateToProps(state: AppState): InjectedProps { /* ... */ }

export const Extended = connect<OwnProps, State, AppState, InjectedProps, ActionProps>(
    mapStateToProps,
    {
        someAction,
    }
)(MyComponent);

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

No branches or pull requests

2 participants