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

Add error property support when payload is instance of Error #52

Closed sylvanaar closed 5 years ago

sylvanaar commented 6 years ago

I have become a little more familiar with the API's, and I really like them, they are super flexible and encourage use of FSA which is great.

I have been able to create an error action using createStandardAction but it wasn't clear how to do it with the other 2 API's.

The way our action creators were written previously, the meta value was the first parameter - so we would like to continue that, so we are writing them using action, but looking at action it isn't clear how to add the error: true property for an error action. (Actually now we are using createAction because we want getType)

Maybe action could be extended to have a 3rd parameter that just let you mix in whatever properties you wanted - or perhaps less flexible but better for FSA's would be to have an error parameter that let you set that property.

   fetchDataError: (widgetKey: string, error: Error) =>
        createStandardAction(FETCH_DATA_FAILURE).map((payload: Error, meta: string) => ({
            payload,
            meta: { widgetKey: meta },
            error: true as true
        }))(error, widgetKey)

Using the API in this way though breaks getType

I haven't tried the Async API's yet though. They might be useful here, but i still need the parameter ordering.

piotrwitek commented 6 years ago

Hi, thanks for the feedback and suggestions.

I like the solution to add 3rd optional param to the creator function, allowing to put whatever to payload, this would be nice generic solution allowing to handle many custom use-cases without cluttering API surface.

I'm probably going to create an experimental branch for it tomorrow.

piotrwitek commented 6 years ago

alternative would be to add implicit mechanism to automatically add error: true property when payload is instanceof Error

const getTodos = createAction('GET_TODOS', resolve => {
  return (meta: string, error: Error) => resolve(error, meta); // { payload: Error, meta: string, error: true }
});
sylvanaar commented 6 years ago

@piotrwitek I think the auto property might be good for createStandardAction, but not the other 2 unless you are 100% committed to FSA. Even though it suits my use case perfectly, it may not suit everyone. Also perhaps not all error actions really use types derived from Error (Mine do, but who knows). Just some thoughts.

I like the generic 3rd param best too. I think it will provide the other 2 API's the same flexibility afforded by createStandardAction's map

IssueHuntBot commented 6 years ago

@boostio funded this issue with $20. Visit this issue on Issuehunt

sjsyrek commented 5 years ago

Hello! I made a PR to address this issue, at least in part: https://github.com/piotrwitek/typesafe-actions/pull/108

The idea is to allow an error parameter when using createAction instead of just payload and meta.

piotrwitek commented 5 years ago

CC @sjsyrek @sylvanaar

This feature request was already solved with addition of new createActionWithType API (althought it isn't public yet)

NOTE: I'm planning to rename createActionWithType to createCustomAction and publish it with 3.1.0 release #111

This API allow a consumer to create a custom action object shape:

Example of solving OP custom use-case:

const getTodos = createaCustomAction('FETCH_DATA_FAILURE', type => {
  return (widgetKey: string, error: Error) => ({
    type,
    payload: error,
    meta: { widgetKey },
    error: true
 });
});
// Result: { type: 'FETCH_DATA_FAILURE' payload: Error, meta: { widgetKey: string }, error: true }
IssueHuntBot commented 5 years ago

@piotrwitek has rewarded $14.00 to @piotrwitek. See it on IssueHunt