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

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

Open
Vanuan opened this issue Feb 12, 2020 · 7 comments

Comments

@Vanuan
Copy link

Vanuan commented Feb 12, 2020

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
Copy link
Author

Vanuan commented Feb 12, 2020

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
Copy link
Author

Vanuan commented Feb 12, 2020

See #82

@Vanuan
Copy link
Author

Vanuan commented Feb 12, 2020

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
Copy link
Author

Vanuan commented Feb 13, 2020

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
Copy link
Author

Vanuan commented Feb 13, 2020

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
Copy link
Author

Vanuan commented Feb 13, 2020

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
Copy link
Author

Vanuan commented Feb 13, 2020

Oh, this looks even better:
reduxjs/redux-toolkit#138

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

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

1 participant