Skip to content

Sets error to true when payloadCreator return an Error object#43

Closed
kpaxqin wants to merge 1 commit into
redux-utilities:masterfrom
kpaxqin:kpaxqin-fsa-error
Closed

Sets error to true when payloadCreator return an Error object#43
kpaxqin wants to merge 1 commit into
redux-utilities:masterfrom
kpaxqin:kpaxqin-fsa-error

Conversation

@kpaxqin
Copy link
Copy Markdown

@kpaxqin kpaxqin commented Nov 19, 2015

No description provided.

@kpaxqin kpaxqin changed the title sets error to true when payloadCreator return an Error object Sets error to true when payloadCreator return an Error object Nov 19, 2015
@lukewestby
Copy link
Copy Markdown
Contributor

To me it makes more sense to check the resulting payload for an error than to check that the arg passed in was an error. +1 from me.

@danny-andrews
Copy link
Copy Markdown
Contributor

I personally think that if you pass an error in as the payload, it should bypass your custom payloadCreator logic and just set the payload equal to the passed-in error. Otherwise, you have to add an error check in each of your action creators to achieve the same effect.
Example:

const createCoolAction = createAction(COOL_ACTION_TYPE, spec => {
  const {zipCode, id} = spec;
  return {body: {zipCode, id}};
});

So, if I do something like, createCoolAction(new Error('Something bad happened')), the payload will be set to {body: {zipCode: undefined, id: undefined}} instead of just the error object itself. To fix this, I could add an error check, but I don't want to have to do this in every one of my action creators.

@timche
Copy link
Copy Markdown
Member

timche commented Aug 31, 2016

@yangmillstheory I think we can close this one because it is already in master, am I right?

@globalchubby
Copy link
Copy Markdown
Contributor

I think this is a bit different; what I see in master is https://github.com/acdlite/redux-actions/blob/master/src/createAction.js#L9.

This is saying that the payloadCreator is allowed to return an Error, but we bypass the payloadCreator entirely in master and just check the first action creator argument.

@globalchubby
Copy link
Copy Markdown
Contributor

Closing this as stale. Also, this duplicates the newer #43.

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

Successfully merging this pull request may close these issues.

5 participants