piotrwitek / typesafe-actions

Typesafe utilities for "action-creators" in Redux / Flux Architecture
https://codesandbox.io/s/github/piotrwitek/typesafe-actions/tree/master/codesandbox
MIT License
2.41k stars 98 forks source link

Creating actions causing some problems #43

Closed abdurahmanus closed 6 years ago

abdurahmanus commented 6 years ago

I have 2 use cases 1) create action without payload, with meta const someAction = createAction('something', resolve => () => resolve(undefined, 'some meta info')) which gives me { type: 'something' }, but i expect {type: 'something', meta: string } 2) create action with payload and meta

const someOtherAction = createStandardAction('somethingElse').map((payload?: number) => ({
  meta: 'some meta info',
  payload: payload!,
}))

in this case I have to mark payload parameter as optional and after tell that it's actualy not, using ! operator to get correct action type ({type: 'somethingElse', payload: number, meta: string})

actualy example from docs

export const add = createStandardAction(ADD).map(
  ({ title }: { title: string }) => ({
    payload: { title, id: cuid(), completed: false } as Todo,
  })
);

doesn't work in my project (I use typescript v2.8.3, redux v4, typesafe-actions v2.0.3) with this error message: [ts] Argument of type '({ title }: { title: string; }) => { payload: { title: string; id: any; completed: boolean; }; }' is not assignable to parameter of type '(payload?: { title: string; } | undefined, meta?: void | undefined) => { payload: { title: string...'. Types of parameters '__0' and 'payload' are incompatible. Type '{ title: string; } | undefined' is not assignable to type '{ title: string; }'. Type 'undefined' is not assignable to type '{ title: string; }'.

because payload param is optional

abdurahmanus commented 6 years ago

also it would be helpful if we could create actions using createAsyncActions helper specifying the type of meta

piotrwitek commented 6 years ago

Ad 1) this is a valid point, adding support for this case is easy, will need to update validation checks Ad 2) not sure what you want here, it's how TS works, nothing actionable or related to the library

Regarding types issue it is related to react-redux@6 types, not in this library, please use react-redux@5 until there is a working solution: https://github.com/piotrwitek/typesafe-actions/issues/42

To extend createAsyncActions API we need a proposal

abdurahmanus commented 6 years ago

I've tried react-redux types v5 this code

const ADD = 'ADD'

interface Todo {
  title: string;
  id: number;
  completed: boolean;
}

export const add = createStandardAction(ADD).map(
  ({ title }: { title: string }) => ({
    payload: { title, id: 12, completed: false } as Todo,
  })
);

still don't work

[ts] Argument of type '({ title }: { title: string; }) => { payload: Todo; }' is not assignable to parameter of type '(payload?: { title: string; } | undefined, meta?: void | undefined) => { payload: Todo; }'. Types of parameters '__0' and 'payload' are incompatible. Type '{ title: string; } | undefined' is not assignable to type '{ title: string; }'. Type 'undefined' is not assignable to type '{ title: string; }'.

abdurahmanus commented 6 years ago

What I want at 2) is to avoid dancing with payload type. Like this:

const someOtherAction = createStandardAction('somethingElse').map((payload: number) => ({
  meta: 'some meta info',
  payload,
}))

Don't sure if it possible

piotrwitek commented 6 years ago

Ad 1) working fine here: https://github.com/piotrwitek/typesafe-actions-todo-app/blob/master/src/features/todos/actions.ts

Ad 2) map is for custom logic, better to use this in your case:

const increment = createStandardAction('INCREMENT')<void, MetaType>();
increment(undefined, 'meta info')
abdurahmanus commented 6 years ago

interface MetaType { s: string; } const increment = createStandardAction('INCREMENT')<void, MetaType>();

gives me {type: 'INCREMENT'} without meta at all

abdurahmanus commented 6 years ago

Maybe problems with types in my case because of different settings in tsconfig.json { "compilerOptions": { "jsx": "react", "module": "commonjs", "moduleResolution": "node", "outDir": "ClientApp/dist/", "sourceMap": true, "strict": true, "target": "es5", "lib": [ "es6", "dom" ], "noEmitOnError": true, "baseUrl": "./", "paths": { ... } }, "include": [ "ClientApp/*/" ] }

abdurahmanus commented 6 years ago

"strictFunctionTypes": false, removes some ts warnings/errors, but it's not a solution

piotrwitek commented 6 years ago

"strictFunctionTypes": false, removes some ts warnings/errors, but it's not a solution

Ok this looks like something we have to investigate, thanks. https://github.com/piotrwitek/typesafe-actions/issues/44

EDIT: From deeper analysis looks like use-case 2 issue is the symptom of strictFunctionTypes issue, so should be resolved with #44

piotrwitek commented 6 years ago

Fix for use-case 1 issue tracked here: https://github.com/piotrwitek/typesafe-actions/issues/47

piotrwitek commented 6 years ago

Hi @abdurahmanus From v2.0.4 both of your use-cases should work correctly: https://github.com/piotrwitek/typesafe-actions/pull/48

// case 1)
const someAction = createAction('something', resolve => () => resolve(undefined, 'some meta info')); 
someAction() // { type: 'something'; meta: string; }

// case 2)
const someOtherAction = createStandardAction('somethingElse').map(() => ({
  meta: 'some meta info',
}))
someOtherAction() // { type: 'somethingElse'; meta: string; }

API documentation was updated with all the above examples of "meta only" cases.

Already released a new version to npm, enjoy!

PS: I'm closing this as all the mentioned cases are supported now, and the issue with strictFunctionTypes is already tracked in #44, so there's no need for duplication