aikoven / typescript-fsa

Type-safe action creator utilities
MIT License
607 stars 28 forks source link

Bug: can't create an action without payload. 'ActionCreator<void>' is not assignable to '() => void' #81

Open Vanuan opened 4 years ago

Vanuan commented 4 years ago

Here's a rough example:

// actions.ts
import actionCreatorFactory from 'typescript-fsa';
const actionCreator = actionCreatorFactory('tasks');
export const allDone = actionCreator('MARK_ALL_DONE');
export const markDone = actionCreator<number>('MARK_DONE');

// Tasks.tsx
interface ITasksProps = {
  allDone: () => void;
  markDone: (taskId: number) => void;
}
...

// TasksContainer.tsx
import Tasks from './Tasks'
import { allDone, markDone } from './actions';
...
const mapDispatchToProps = {
  allDone,
  markDone,
};

connect(null, mapDispatchToProps)(Tasks);

Here's an error:

Type 'ActionCreator<void>' is not assignable to type '() => void'.  TS2345

To fix it, I have to change this:

  allDone: () => void;

To this:

  allDone: (payload: void) => void;

Which is stupid.

Vanuan commented 4 years ago

I think it can be fixed like this:

  export interface ActionCreatorFactory {
      <Payload = undefined>(type: string, commonMeta?: Meta, isError?: boolean): ActionCreator<Payload>;
  ...
  }

  interface ActionCreatorWithoutPayload<Payload> {
       ...
      (): AnyAction;
  };

  interface ActionCreatorWithPayload<Payload> {
       ...
      (payload: Payload, meta?: Meta): Action<Payload>;
  };

  export declare type ActionCreator<Payload = undefined> =
    Payload extends undefined 
    ? ActionCreatorWithoutPayload<Payload>
    : Payload extends undefined | any 
      ? ActionCreatorWithoutPayload<Payload> & ActionCreatorWithPayload<Payload>
      : ActionCreatorWithPayload<Payload>;
Vanuan commented 4 years ago

See #82

Vanuan commented 4 years ago

In addition, we don't want payload to be present:

export type Action<Payload> = Payload extends void ? {
    type: string;
    error?: boolean;
    meta?: Meta;
} : {
    type: string;
    payload: Payload;
    error?: boolean;
    meta?: Meta;
}
Vanuan commented 4 years ago

The issue is that you can't use void as a parameter type in generic:

function testAsyncGenericStrictError<P, R, E>(params: P, result: R, error: E) {
    const async = actionCreator.async<P, R, E>('ASYNC');

    //  you can't do this because you don't know whether P is void or not
    const started = async.started();

There's no way to verify whether parameter value is void or not. The only place where void is allowed is the return value. So using void as a generic type to signify that a parameter can be omitted is wrong.

    (...a: (Payload extends void
            ? [] | [undefined] | [undefined, Meta]
            : [Payload] | [Payload, Meta])): Action<Payload>;
Vanuan commented 4 years ago

So it means the default value should be changed to something else that can be type guarded:

  <Payload = void>(
    type: string,
    commonMeta?: Meta,
    isError?: boolean,
  ): ActionCreator<Payload>;

Should be changed to

  <Payload = undefined>(
    type: string,
    commonMeta?: Meta,
    isError?: boolean,
  ): ActionCreator<Payload>;

undefined much better serves the purpose of a missing value

    (...a: (Payload extends undefined
            ? [] | [undefined] | [undefined, Meta]
            : [Payload] | [Payload, Meta])): Action<Payload>;
Vanuan commented 4 years ago

Here's how similar issue fixed in redux-toolkit:

https://github.com/reduxjs/redux-toolkit/pull/133/files#diff-438bbd0288b07c3e7ae7f40f89f7f451R9-R11

export type SliceActionCreator<P> = P extends void
  ? () => PayloadAction<void>
  : (payload: P) => PayloadAction<P>
Vanuan commented 4 years ago

Oh, this looks even better: https://github.com/reduxjs/redux-toolkit/pull/138

export type SliceActionCreator<P> = [P] extends [void]
  ? () => PayloadAction<void>    
  : (payload: P) => PayloadAction<P>